| Message ID | 20240112113930.1647666-1-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e18405d0be8001fa4c5f9e61471f6ffd59c7a1b3 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: sched: track device in tcf_block_get/put_ext() only for clsact binder types | expand |
On Fri, Jan 12, 2024 at 12:39:30PM +0100, Jiri Pirko wrote: > From: Jiri Pirko <jiri@nvidia.com> > > Clsact/ingress qdisc is not the only one using shared block, > red is also using it. The device tracking was originally introduced > by commit 913b47d3424e ("net/sched: Introduce tc block netdev > tracking infra") for clsact/ingress only. Commit 94e2557d086a ("net: > sched: move block device tracking into tcf_block_get/put_ext()") > mistakenly enabled that for red as well. > > Fix that by adding a check for the binder type being clsact when adding > device to the block->ports xarray. > > Reported-by: Ido Schimmel <idosch@idosch.org> > Closes: https://lore.kernel.org/all/ZZ6JE0odnu1lLPtu@shredder/ > Fixes: 94e2557d086a ("net: sched: move block device tracking into tcf_block_get/put_ext()") > Signed-off-by: Jiri Pirko <jiri@nvidia.com> Tested-by: Ido Schimmel <idosch@nvidia.com>
On Fri, Jan 12, 2024 at 6:39 AM Jiri Pirko <jiri@resnulli.us> wrote: > > From: Jiri Pirko <jiri@nvidia.com> > > Clsact/ingress qdisc is not the only one using shared block, > red is also using it. The device tracking was originally introduced > by commit 913b47d3424e ("net/sched: Introduce tc block netdev > tracking infra") for clsact/ingress only. Commit 94e2557d086a ("net: > sched: move block device tracking into tcf_block_get/put_ext()") > mistakenly enabled that for red as well. > > Fix that by adding a check for the binder type being clsact when adding > device to the block->ports xarray. > > Reported-by: Ido Schimmel <idosch@idosch.org> > Closes: https://lore.kernel.org/all/ZZ6JE0odnu1lLPtu@shredder/ > Fixes: 94e2557d086a ("net: sched: move block device tracking into tcf_block_get/put_ext()") > Signed-off-by: Jiri Pirko <jiri@nvidia.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal > --- > net/sched/cls_api.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index e3236a3169c3..92a12e3d0fe6 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -1424,6 +1424,14 @@ static void tcf_block_owner_del(struct tcf_block *block, > WARN_ON(1); > } > > +static bool tcf_block_tracks_dev(struct tcf_block *block, > + struct tcf_block_ext_info *ei) > +{ > + return tcf_block_shared(block) && > + (ei->binder_type == FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS || > + ei->binder_type == FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS); > +} > + > int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q, > struct tcf_block_ext_info *ei, > struct netlink_ext_ack *extack) > @@ -1462,7 +1470,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q, > if (err) > goto err_block_offload_bind; > > - if (tcf_block_shared(block)) { > + if (tcf_block_tracks_dev(block, ei)) { > err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL); > if (err) { > NL_SET_ERR_MSG(extack, "block dev insert failed"); > @@ -1516,7 +1524,7 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q, > > if (!block) > return; > - if (tcf_block_shared(block)) > + if (tcf_block_tracks_dev(block, ei)) > xa_erase(&block->ports, dev->ifindex); > tcf_chain0_head_change_cb_del(block, ei); > tcf_block_owner_del(block, q, ei->binder_type); > -- > 2.43.0 >
> From: Jiri Pirko <jiri@nvidia.com> > > Clsact/ingress qdisc is not the only one using shared block, > red is also using it. The device tracking was originally introduced > by commit 913b47d3424e ("net/sched: Introduce tc block netdev > tracking infra") for clsact/ingress only. Commit 94e2557d086a ("net: > sched: move block device tracking into tcf_block_get/put_ext()") > mistakenly enabled that for red as well. > > Fix that by adding a check for the binder type being clsact when adding > device to the block->ports xarray. > > Reported-by: Ido Schimmel <idosch@idosch.org> > Closes: https://lore.kernel.org/all/ZZ6JE0odnu1lLPtu@shredder/ > Fixes: 94e2557d086a ("net: sched: move block device tracking into tcf_block_get/put_ext()") > Signed-off-by: Jiri Pirko <jiri@nvidia.com> Tested-by: Victor Nogueira <victor@mojatatu.com>
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Fri, 12 Jan 2024 12:39:30 +0100 you wrote: > From: Jiri Pirko <jiri@nvidia.com> > > Clsact/ingress qdisc is not the only one using shared block, > red is also using it. The device tracking was originally introduced > by commit 913b47d3424e ("net/sched: Introduce tc block netdev > tracking infra") for clsact/ingress only. Commit 94e2557d086a ("net: > sched: move block device tracking into tcf_block_get/put_ext()") > mistakenly enabled that for red as well. > > [...] Here is the summary with links: - [net] net: sched: track device in tcf_block_get/put_ext() only for clsact binder types https://git.kernel.org/netdev/net/c/e18405d0be80 You are awesome, thank you!
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index e3236a3169c3..92a12e3d0fe6 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -1424,6 +1424,14 @@ static void tcf_block_owner_del(struct tcf_block *block, WARN_ON(1); } +static bool tcf_block_tracks_dev(struct tcf_block *block, + struct tcf_block_ext_info *ei) +{ + return tcf_block_shared(block) && + (ei->binder_type == FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS || + ei->binder_type == FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS); +} + int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q, struct tcf_block_ext_info *ei, struct netlink_ext_ack *extack) @@ -1462,7 +1470,7 @@ int tcf_block_get_ext(struct tcf_block **p_block, struct Qdisc *q, if (err) goto err_block_offload_bind; - if (tcf_block_shared(block)) { + if (tcf_block_tracks_dev(block, ei)) { err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL); if (err) { NL_SET_ERR_MSG(extack, "block dev insert failed"); @@ -1516,7 +1524,7 @@ void tcf_block_put_ext(struct tcf_block *block, struct Qdisc *q, if (!block) return; - if (tcf_block_shared(block)) + if (tcf_block_tracks_dev(block, ei)) xa_erase(&block->ports, dev->ifindex); tcf_chain0_head_change_cb_del(block, ei); tcf_block_owner_del(block, q, ei->binder_type);