netdev - Re: [PATCH net-next v4 6/8] net: sched: create tc_can_offload

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 20 Jan 2018 11:54:27 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     David Miller <davem@...emloft.net>,
        David Ahern <dsahern@...il.com>, aring@...atatu.com,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com,
        Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH net-next v4 6/8] net: sched: create
 tc_can_offload_extack() wrapper

Sat, Jan 20, 2018 at 11:33:31AM CET, jakub.kicinski@...ronome.com wrote:
>On Sat, Jan 20, 2018 at 2:22 AM, Jiri Pirko <jiri@...nulli.us> wrote:
>> Sat, Jan 20, 2018 at 11:12:25AM CET, jakub.kicinski@...ronome.com wrote:
>>>net/sched/sch_prio.c:   if (!tc_can_offload(dev) ||
>>>!dev->netdev_ops->ndo_setup_tc)
>>>net/sched/sch_prio.c:   if (!tc_can_offload(dev) ||
>>>!dev->netdev_ops->ndo_setup_tc)
>>>net/sched/sch_red.c:    if (!tc_can_offload(dev) ||
>>>!dev->netdev_ops->ndo_setup_tc)
>>>net/sched/sch_red.c:    if (!tc_can_offload(dev) ||
>>>!dev->netdev_ops->ndo_setup_tc)
>>>
>>>Do you mean the qdisc offloads too?  The whole lot?
>>
>> Yes.
>
>Actually looking at the qdisc code and destroy callbacks, if we plumb
>it through everywhere won't that mean user will see error messages on
>destroy of qdiscs/filters which were never offloaded?
>
>Just looking at prio_offload() as a simple example.  prio_destroy()
>will always call tc_can_offload().

Hmmm. You are right.
Either we pass null from there (NL_SET_ERR_MSG can cope), or we leave
tc_can_offload helper as is and let the caller to set the extack.
I see that tc_can_offload_extack is probably good idea then.

But please use it in all drivers that are calling tc_can_offload so the
user experience is consistent for all.

Thanks!