[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM5PR1301MB21724323009FF1C33518E4C5E78E9@DM5PR1301MB2172.namprd13.prod.outlook.com>
Date: Wed, 20 Jul 2022 09:19:01 +0000
From: Baowen Zheng <baowen.zheng@...igine.com>
To: Oz Shlomo <ozsh@...dia.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: Simon Horman <simon.horman@...igine.com>,
"David S. Miller" <davem@...emloft.net>,
Roi Dayan <roid@...dia.com>
Subject: RE: [PATCH net v2] net/sched: cls_api: Fix flow action initialization
>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@...dia.com>
>Reviewed-by: Roi Dayan <roid@...dia.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@...igine.com>
Powered by blists - more mailing lists