diff mbox series

[net,v2] net/sched: cls_api: Fix flow action initialization

Message ID 20220719122409.18703-1-ozsh@nvidia.com (mailing list archive)
State Accepted
Commit c0f47c2822aadeb8b2829f3e4c3792f184c7be33
Delegated to: Netdev Maintainers
Headers show
Series [net,v2] net/sched: cls_api: Fix flow action initialization | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers warning 6 maintainers not CCed: jhs@mojatatu.com xiyou.wangcong@gmail.com jiri@resnulli.us pabeni@redhat.com kuba@kernel.org edumazet@google.com
netdev/build_clang success Errors and warnings before: 6 this patch: 6
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 9 this patch: 9
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 31 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Oz Shlomo July 19, 2022, 12:24 p.m. UTC
The cited commit refactored the flow action initialization sequence to
use an interface method when translating tc action instances to flow
offload objects. The refactored version skips the initialization of the
generic flow action attributes for tc actions, such as pedit, that allocate
more than one offload entry. This can cause potential issues for drivers
mapping flow action ids.

Populate the generic flow action fields for all the flow action entries.

Fixes: c54e1d920f04 ("flow_offload: add ops to tc_action_ops for flow action setup")
Signed-off-by: Oz Shlomo <ozsh@nvidia.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>

----
v1 -> v2:
 - coalese the generic flow action fields initialization to a single loop
---
 net/sched/cls_api.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Baowen Zheng July 20, 2022, 9:19 a.m. UTC | #1
>Subject: [PATCH net v2] net/sched: cls_api: Fix flow action initialization
>
>The cited commit refactored the flow action initialization sequence to use an
>interface method when translating tc action instances to flow offload objects.
>The refactored version skips the initialization of the generic flow action
>attributes for tc actions, such as pedit, that allocate more than one offload
>entry. This can cause potential issues for drivers mapping flow action ids.
>
>Populate the generic flow action fields for all the flow action entries.
>
>Fixes: c54e1d920f04 ("flow_offload: add ops to tc_action_ops for flow action
>setup")
>Signed-off-by: Oz Shlomo <ozsh@nvidia.com>
>Reviewed-by: Roi Dayan <roid@nvidia.com>
>
>----
>v1 -> v2:
> - coalese the generic flow action fields initialization to a single loop
>---
> net/sched/cls_api.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index
>c7a240232b8d..790d6809be81 100644
>--- a/net/sched/cls_api.c
>+++ b/net/sched/cls_api.c
>@@ -3534,7 +3534,7 @@ int tc_setup_action(struct flow_action *flow_action,
> 		    struct tc_action *actions[],
> 		    struct netlink_ext_ack *extack)
> {
>-	int i, j, index, err = 0;
>+	int i, j, k, index, err = 0;
> 	struct tc_action *act;
>
> 	BUILD_BUG_ON(TCA_ACT_HW_STATS_ANY !=
>FLOW_ACTION_HW_STATS_ANY); @@ -3554,14 +3554,18 @@ int
>tc_setup_action(struct flow_action *flow_action,
> 		if (err)
> 			goto err_out_locked;
>
>-		entry->hw_stats = tc_act_hw_stats(act->hw_stats);
>-		entry->hw_index = act->tcfa_index;
> 		index = 0;
> 		err = tc_setup_offload_act(act, entry, &index, extack);
>-		if (!err)
>-			j += index;
>-		else
>+		if (err)
> 			goto err_out_locked;
>+
>+		for (k = 0; k < index ; k++) {
>+			entry[k].hw_stats = tc_act_hw_stats(act->hw_stats);
>+			entry[k].hw_index = act->tcfa_index;
>+		}
>+
>+		j += index;
>+
> 		spin_unlock_bh(&act->tcfa_lock);
> 	}
>
>--
>1.8.3.1
-----------------------------------------------------
Thanks, this change looks good to me now.

Reviewed-by: Baowen Zheng <baowen.zheng@corigine.com>
patchwork-bot+netdevbpf@kernel.org July 20, 2022, 10 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Tue, 19 Jul 2022 15:24:09 +0300 you wrote:
> The cited commit refactored the flow action initialization sequence to
> use an interface method when translating tc action instances to flow
> offload objects. The refactored version skips the initialization of the
> generic flow action attributes for tc actions, such as pedit, that allocate
> more than one offload entry. This can cause potential issues for drivers
> mapping flow action ids.
> 
> [...]

Here is the summary with links:
  - [net,v2] net/sched: cls_api: Fix flow action initialization
    https://git.kernel.org/netdev/net/c/c0f47c2822aa

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index c7a240232b8d..790d6809be81 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3534,7 +3534,7 @@  int tc_setup_action(struct flow_action *flow_action,
 		    struct tc_action *actions[],
 		    struct netlink_ext_ack *extack)
 {
-	int i, j, index, err = 0;
+	int i, j, k, index, err = 0;
 	struct tc_action *act;
 
 	BUILD_BUG_ON(TCA_ACT_HW_STATS_ANY != FLOW_ACTION_HW_STATS_ANY);
@@ -3554,14 +3554,18 @@  int tc_setup_action(struct flow_action *flow_action,
 		if (err)
 			goto err_out_locked;
 
-		entry->hw_stats = tc_act_hw_stats(act->hw_stats);
-		entry->hw_index = act->tcfa_index;
 		index = 0;
 		err = tc_setup_offload_act(act, entry, &index, extack);
-		if (!err)
-			j += index;
-		else
+		if (err)
 			goto err_out_locked;
+
+		for (k = 0; k < index ; k++) {
+			entry[k].hw_stats = tc_act_hw_stats(act->hw_stats);
+			entry[k].hw_index = act->tcfa_index;
+		}
+
+		j += index;
+
 		spin_unlock_bh(&act->tcfa_lock);
 	}