Re: [PATCH net-next v10 03/14] net: sched: wrap doit/dumpit methods

[Stanislav Fomichev <stfomichev@gmail.com>]

On 03/02, Jamal Hadi Salim wrote:
> On Sat, Mar 1, 2025 at 7:09 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > In preparation for grabbing netdev instance lock around qdisc
> > operations, introduce tc_xxx wrappers that lookup netdev
> > and call respective __tc_xxx helper to do the actual work.
> > No functional changes.
> >
> > Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> > Cc: Cong Wang <xiyou.wangcong@gmail.com>
> > Cc: Jiri Pirko <jiri@resnulli.us>
> > Cc: Saeed Mahameed <saeed@kernel.org>
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > ---
> >  net/sched/sch_api.c | 190 ++++++++++++++++++++++++++++----------------
> >  1 file changed, 122 insertions(+), 68 deletions(-)
> >
> > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> > index e3e91cf867eb..e0be3af4daa9 100644
> > --- a/net/sched/sch_api.c
> > +++ b/net/sched/sch_api.c
> > @@ -1505,27 +1505,18 @@ const struct nla_policy rtm_tca_policy[TCA_MAX + 1] = {
> >   * Delete/get qdisc.
> >   */
> >
> [..]
> > +static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n,
> > +                          struct netlink_ext_ack *extack)
> > +{
> > +       struct net *net = sock_net(skb->sk);
> > +       struct tcmsg *tcm = nlmsg_data(n);
> > +       struct nlattr *tca[TCA_MAX + 1];
> > +       struct net_device *dev;
> > +       bool replay;
> > +       int err;
> > +
> > +replay:
> 
> For 1-1 mapping to original code, the line:
> struct tcmsg *tcm = nlmsg_data(n);
> 
> Should move below the replay goto..

Since nlmsg_data is just doing pointer arithmetics and the (pointer) argument
doesn't change I was assuming it's ok to reshuffle to make
tc_get_qdisc and tc_modify_qdisc look more consistent. LMK if you
disagree, happy to move them back.


> Other than that:
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>

Thank you for the review!