[patch net-next] flow_offload: use flow_action_for_each in flow_action_mixed_hw_stats_types_check()

[patch net-next] flow_offload: use flow_action_for_each in flow_action_mixed_hw_stats_types_check()

[Jiri Pirko]

Instead of manually iterating over entries, use flow_action_for_each
helper. Move the helper and wrap it to fit to 80 cols on the way.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/net/flow_offload.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
index 64807aa03cee..7b7bd9215156 100644
--- a/include/net/flow_offload.h
+++ b/include/net/flow_offload.h
@@ -256,6 +256,11 @@ static inline bool flow_offload_has_one_action(const struct flow_action *action)
 	return action->num_entries == 1;
 }
 
+#define flow_action_for_each(__i, __act, __actions)			\
+        for (__i = 0, __act = &(__actions)->entries[0];			\
+	     __i < (__actions)->num_entries;				\
+	     __act = &(__actions)->entries[++__i])
+
 static inline bool
 flow_action_mixed_hw_stats_types_check(const struct flow_action *action,
 				       struct netlink_ext_ack *extack)
@@ -267,7 +272,7 @@ flow_action_mixed_hw_stats_types_check(const struct flow_action *action,
 	if (flow_offload_has_one_action(action))
 		return true;
 
-	for (i = 0; i < action->num_entries; i++) {
+	flow_action_for_each(i, action_entry, action) {
 		action_entry = &action->entries[i];
 		if (i && action_entry->hw_stats_type != last_hw_stats_type) {
 			NL_SET_ERR_MSG_MOD(extack, "Mixing HW stats types for actions is not supported");
@@ -316,9 +321,6 @@ flow_action_basic_hw_stats_types_check(const struct flow_action *action,
 	return flow_action_hw_stats_types_check(action, extack, 0);
 }
 
-#define flow_action_for_each(__i, __act, __actions)			\
-        for (__i = 0, __act = &(__actions)->entries[0]; __i < (__actions)->num_entries; __act = &(__actions)->entries[++__i])
-
 struct flow_rule {
 	struct flow_match	match;
 	struct flow_action	action;
-- 
2.21.1

Re: [patch net-next] flow_offload: use flow_action_for_each in flow_action_mixed_hw_stats_types_check()

[Edward Cree]

On 09/03/2020 17:44, Jiri Pirko wrote:
> Instead of manually iterating over entries, use flow_action_for_each
> helper. Move the helper and wrap it to fit to 80 cols on the way.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Acked-by: Edward Cree <ecree@solarflare.com>

Re: [patch net-next] flow_offload: use flow_action_for_each in flow_action_mixed_hw_stats_types_check()

[Pablo Neira Ayuso]

On Mon, Mar 09, 2020 at 06:44:47PM +0100, Jiri Pirko wrote:
> Instead of manually iterating over entries, use flow_action_for_each
> helper. Move the helper and wrap it to fit to 80 cols on the way.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  include/net/flow_offload.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
> index 64807aa03cee..7b7bd9215156 100644
> --- a/include/net/flow_offload.h
> +++ b/include/net/flow_offload.h
> @@ -256,6 +256,11 @@ static inline bool flow_offload_has_one_action(const struct flow_action *action)
>  	return action->num_entries == 1;
>  }
>  
> +#define flow_action_for_each(__i, __act, __actions)			\
> +        for (__i = 0, __act = &(__actions)->entries[0];			\
> +	     __i < (__actions)->num_entries;				\
> +	     __act = &(__actions)->entries[++__i])
> +
>  static inline bool
>  flow_action_mixed_hw_stats_types_check(const struct flow_action *action,
>  				       struct netlink_ext_ack *extack)
> @@ -267,7 +272,7 @@ flow_action_mixed_hw_stats_types_check(const struct flow_action *action,
>  	if (flow_offload_has_one_action(action))
>  		return true;
>  
> -	for (i = 0; i < action->num_entries; i++) {
> +	flow_action_for_each(i, action_entry, action) {
>  		action_entry = &action->entries[i];
                ^^^

action_entry is set twice, right? One from flow_action_for_each() and
again here. You can probably remove this line too.

Re: [patch net-next] flow_offload: use flow_action_for_each in flow_action_mixed_hw_stats_types_check()

[Jiri Pirko]

Mon, Mar 09, 2020 at 07:33:25PM CET, pablo@netfilter.org wrote:
>On Mon, Mar 09, 2020 at 06:44:47PM +0100, Jiri Pirko wrote:
>> Instead of manually iterating over entries, use flow_action_for_each
>> helper. Move the helper and wrap it to fit to 80 cols on the way.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  include/net/flow_offload.h | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/include/net/flow_offload.h b/include/net/flow_offload.h
>> index 64807aa03cee..7b7bd9215156 100644
>> --- a/include/net/flow_offload.h
>> +++ b/include/net/flow_offload.h
>> @@ -256,6 +256,11 @@ static inline bool flow_offload_has_one_action(const struct flow_action *action)
>>  	return action->num_entries == 1;
>>  }
>>  
>> +#define flow_action_for_each(__i, __act, __actions)			\
>> +        for (__i = 0, __act = &(__actions)->entries[0];			\
>> +	     __i < (__actions)->num_entries;				\
>> +	     __act = &(__actions)->entries[++__i])
>> +
>>  static inline bool
>>  flow_action_mixed_hw_stats_types_check(const struct flow_action *action,
>>  				       struct netlink_ext_ack *extack)
>> @@ -267,7 +272,7 @@ flow_action_mixed_hw_stats_types_check(const struct flow_action *action,
>>  	if (flow_offload_has_one_action(action))
>>  		return true;
>>  
>> -	for (i = 0; i < action->num_entries; i++) {
>> +	flow_action_for_each(i, action_entry, action) {
>>  		action_entry = &action->entries[i];
>                ^^^
>
>action_entry is set twice, right? One from flow_action_for_each() and
>again here. You can probably remove this line too.

Correct. Will send v2