Messages in this thread | | | Date | Fri, 10 May 2024 13:19:48 +0100 | From | Simon Horman <> | Subject | Re: [net PATCH] net/sched: Get stab before calling ops->change() |
| |
On Thu, May 09, 2024 at 10:40:43AM +0800, Xiaolei Wang wrote: > ops->change() depends on stab, there is such a situation > When no parameters are passed in for the first time, stab > is omitted, as in configuration 1 below. At this time, a > warning "Warning: sch_taprio: Size table not specified, frame > length estimates may be inaccurate" will be received. When > stab is added for the second time, parameters, like configuration > 2 below, because the stab is still empty when ops->change() > is running, you will also receive the above warning. > > 1. tc qdisc replace dev eth1 parent root handle 100 taprio \ > num_tc 5 map 0 1 2 3 4 queues 1@0 1@1 1@2 1@3 1@4 base-time 0 \ > sched-entry S 1 100000 \ > sched-entry S 2 100000 \ > sched-entry S 4 100000 \ > max-sdu 0 0 0 0 0 0 0 200 \ > flags 2 > > 2. tc qdisc replace dev eth1 parent root overhead 24 handle 100 taprio \ > num_tc 5 map 0 1 2 3 4 queues 1@0 1@1 1@2 1@3 1@4 base-time 0 \ > sched-entry S 1 100000 \ > sched-entry S 2 100000 \ > sched-entry S 4 100000 \ > max-sdu 0 0 0 0 0 0 0 200 \ > flags 2 >
Hi Xiaolei Wang,
If this is a fix, targeted at the net tree, then it should probably have a Fixes tag here (no blank line between it and other tags).
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com> > --- > net/sched/sch_api.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c > index 60239378d43f..fec358f497d5 100644 > --- a/net/sched/sch_api.c > +++ b/net/sched/sch_api.c > @@ -1404,6 +1404,16 @@ static int qdisc_change(struct Qdisc *sch, struct nlattr **tca, > struct qdisc_size_table *ostab, *stab = NULL; > int err = 0; > > + if (tca[TCA_STAB]) { > + stab = qdisc_get_stab(tca[TCA_STAB], extack); > + if (IS_ERR(stab)) > + return PTR_ERR(stab); > + } > + > + ostab = rtnl_dereference(sch->stab); > + rcu_assign_pointer(sch->stab, stab); > + qdisc_put_stab(ostab); > + > if (tca[TCA_OPTIONS]) { > if (!sch->ops->change) { > NL_SET_ERR_MSG(extack, "Change operation not supported by specified qdisc");
I am concerned that in this case the stab will be updated even if the change operation is rejected by the following code in the if (tca[TCA_OPTIONS]) block, just below the above hunk.
if (tca[TCA_INGRESS_BLOCK] || tca[TCA_EGRESS_BLOCK]) { NL_SET_ERR_MSG(extack, "Change of blocks is not supported"); return -EOPNOTSUPP; } ..
-- pw-bot: under-review
| |