Re: [RFC/PATCH net-next v2 4/5] flow_offload: add reoffload process to update hw_count

Hi Vlad,

On Fri, Oct 01, 2021 at 08:30:38PM +0300, Vlad Buslov wrote:

On Fri 01 Oct 2021 at 14:32, Simon Horman [off-list ref] wrote:

quoted

From: Baowen Zheng <redacted>

Add reoffload process to update hw_count when driver
is inserted or removed.

When reoffloading actions, we still offload the actions
that are added independent of filters.

As per comment on 2/4.

	Thanks for your review and sorry for the delay in responding.
	I believe that at this point we have addressed most of the points
	your raised and plan to post a v3 shortly.

	At this point I'd like to relay some responses from Baowen who
	has been working on addressing your review.

quoted

Change the lock usage to fix sleeping in invalid context.

What does this refer to? Looking at the code it is not clear to me which
lock usage is changed. Or is it just a change log from v1?

Sorry, this is an artifact of our development process that shouldn't have
been left here. Please ignore.

...

quoted

@@ -44,6 +45,9 @@ struct tc_action {
 	u8			hw_stats;
 	u8			used_hw_stats;
 	bool			used_hw_stats_valid;
+	bool                    add_separate; /* indicate if the action is created
+					       * independent of any flow
+					       */

This looks like a duplication of flags since this value is derived from
BIND flag. I understand that you need this because currently all flags
that are not visible to the userspace are cleared after action is
created, but maybe it would be better to refactor the code to preserve
all flags and to only apply TCA_ACT_FLAGS_USER_MASK when dumping to user
space.

Thanks, we've cleaned things up as you suggest for v3.

...