netdev - Re: [PATCH net-next v2] net: sysfs: Do not create sysfs for non BQL device

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]

Date: Fri, 16 Feb 2024 19:45:37 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Florian Fainelli <f.fainelli@...il.com>
Cc: Stephen Hemminger <stephen@...workplumber.org>, Breno Leitao <leitao@...ian.org>, kuba@...nel.org, 
	davem@...emloft.net, pabeni@...hat.com, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, horms@...nel.org, 
	Johannes Berg <johannes.berg@...el.com>
Subject: Re: [PATCH net-next v2] net: sysfs: Do not create sysfs for non BQL device

On Fri, Feb 16, 2024 at 7:41 PM Florian Fainelli <f.fainelli@...il.com> wrote:
>
> On 2/16/24 09:29, Stephen Hemminger wrote:
> > On Fri, 16 Feb 2024 01:41:52 -0800
> > Breno Leitao <leitao@...ian.org> wrote:
> >
> >> +static bool netdev_uses_bql(const struct net_device *dev)
> >> +{
> >> +    if (dev->features & NETIF_F_LLTX ||
> >> +        dev->priv_flags & IFF_NO_QUEUE)
> >> +            return false;
> >> +
> >> +    return IS_ENABLED(CONFIG_BQL);
> >> +}
> >
> > Various compilers will warn about missing parens in that expression.
> > It is valid but mixing & and || can be bug trap.
> >
> >       if ((dev->features & NETIF_F_LLTX) || (dev->priv_flags & IFF_NO_QUEUE))
> >               return false;
> >
> > Not all drivers will be using bql, it requires driver to have that code.
> > So really it means driver could be using BQL.
> > Not sure if there is a way to find out if driver has the required BQL bits.
>
> There is not a feature flag to be keying off if that is what you are
> after, you would need to audit the drivers and see whether they make
> calls to netdev_tx_sent_queue(), netdev_tx_reset_queue(),
> netdev_tx_completed_queue().
>
> I suppose you might be able to programmatically extract that information
> by looking at whether a given driver object file has a reference to
> dql_{reset,avail,completed} or do that at the source level, whichever is
> easier.

Note that the suggested patch does not change current functionality.

Traditionally, we had sysfs entries fpr BQL for all netdev, regardless of them
using BQL or not.

The patch seems to be a good first step.

If anyone wants to refine it further, this is great, but I suspect
very few users will benefit from
having less sysfs entries for real/physical devices....