netdev - Re: [PATCH net-next v3] net: dqs: introduce IFF_NO

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: Thu, 13 Jun 2024 23:37:09 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Jakub Kicinski <kuba@...nel.org>, pabeni@...hat.com, davem@...emloft.net, 
	dsahern@...nel.org, mst@...hat.com, jasowang@...hat.com, 
	xuanzhuo@...ux.alibaba.com, eperezma@...hat.com, leitao@...ian.org, 
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v3] net: dqs: introduce IFF_NO_BQL private flag
 for non-BQL drivers

On Thu, Jun 13, 2024 at 11:26 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Thu, Jun 13, 2024 at 5:02 PM Jakub Kicinski <kuba@...nel.org> wrote:
> >
> > On Thu, 13 Jun 2024 22:55:16 +0800 Jason Xing wrote:
> > > I wonder why the status of this patch was changed to 'Changes
> > > Requested'? Is there anything else I should adjust?
> >
> > Sorry to flip the question on you, but do you think the patch should
> > be merged as is? Given Jiri is adding BQL support to virtio?
>
> Also what is the rationale for all this discussion ?
>
> Don't we have many sys files that are never used anyway ?

At the very beginning, I thought the current patch is very simple and
easy to get merged because I just found other non-BQL drivers passing
the checks in netdev_uses_bql(). Also see the commit:
commit 74293ea1c4db62cb969e741fbfd479a34d935024
Author: Breno Leitao <leitao@...ian.org>
Date:   Fri Feb 16 01:41:52 2024 -0800

    net: sysfs: Do not create sysfs for non BQL device

    Creation of sysfs entries is expensive, mainly for workloads that
    constantly creates netdev and netns often.

    Do not create BQL sysfs entries for devices that don't need,
    basically those that do not have a real queue, i.e, devices that has
    NETIF_F_LLTX and IFF_NO_QUEUE, such as `lo` interface.

    This will remove the /sys/class/net/eth0/queues/tx-X/byte_queue_limits/
    directory for these devices.

    In the example below, eth0 has the `byte_queue_limits` directory but not
    `lo`.

            # ls /sys/class/net/lo/queues/tx-0/
            traffic_class  tx_maxrate  tx_timeout  xps_cpus  xps_rxqs

            # ls /sys/class/net/eth0/queues/tx-0/byte_queue_limits/
            hold_time  inflight  limit  limit_max  limit_min

    This also removes the #ifdefs, since we can also use netdev_uses_bql() to
    check if the config is enabled. (as suggested by Jakub).

    Suggested-by: Eric Dumazet <edumazet@...gle.com>
    Signed-off-by: Breno Leitao <leitao@...ian.org>
    Link: https://lore.kernel.org/r/20240216094154.3263843-1-leitao@debian.org
    Signed-off-by: Jakub Kicinski <kuba@...nel.org>

I followed this patch and introduced a flag only.

Actually, it's against my expectations. It involved too many
discussions. As I said again: at the very beginning, I thought it's
very easy to get merged... :(