netdev - Re: [PATCH net-next 01/22] net: sched: act_api: implement generic walker and search for tc action

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM0EoMnxxA5y2W22aMXF+QRqckbkGm9eJoEnu-CaKhgWMM7kdA@mail.gmail.com>
Date:   Fri, 2 Sep 2022 09:53:42 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Zhengchao Shao <shaozhengchao@...wei.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        kuba@...nel.org, pabeni@...hat.com, xiyou.wangcong@...il.com,
        jiri@...nulli.us, martin.lau@...ux.dev, daniel@...earbox.net,
        john.fastabend@...il.com, ast@...nel.org, andrii@...nel.org,
        song@...nel.org, yhs@...com, kpsingh@...nel.org, sdf@...gle.com,
        haoluo@...gle.com, jolsa@...nel.org, weiyongjun1@...wei.com,
        yuehaibing@...wei.com
Subject: Re: [PATCH net-next 01/22] net: sched: act_api: implement generic
 walker and search for tc action

On Fri, Sep 2, 2022 at 7:22 AM Zhengchao Shao <shaozhengchao@...wei.com> wrote:
>
> Being able to get tc_action_net by using net_id stored in tc_action_ops
> and execute the generic walk/search function, add __tcf_generic_walker()
> and __tcf_idr_search() helpers.
>

These are nice cleanups.
Can you please run all tdc tests for all changes you are making to
the tc subsystem? Maybe do a kindness and add more tests.

Just small  opinions below. Otherwise you can add my ACK.

> Signed-off-by: Zhengchao Shao <shaozhengchao@...wei.com>
> ---
>  include/net/act_api.h |  1 +
>  net/sched/act_api.c   | 48 +++++++++++++++++++++++++++++++++++++------
>  2 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 9cf6870b526e..a79d6e58519e 100644
\

> @@ -926,7 +945,8 @@ int tcf_register_action(struct tc_action_ops *act,
>         struct tc_action_ops *a;
>         int ret;
>
> -       if (!act->act || !act->dump || !act->init || !act->walk || !act->lookup)
> +       if (!act->act || !act->dump || !act->init ||
> +           (!act->net_id && (!act->walk || !act->lookup)))

I can understand net_id, but why && (!act->walk || !act->lookup) ?
Assumedly they are now optional, no?


> +       if (ops->walk) {
> +               err = ops->walk(net, skb, &dcb, RTM_DELACTION, ops, extack);
> +       } else {
> +               err = __tcf_generic_walker(net, skb, &dcb, RTM_DELACTION, ops, extack);
> +       }

Bikeshed mod: those braces.

cheers,
jamal