netdev - Re: [PATCH net-next] net: dqs: introduce NETIF_F_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: Mon, 10 Jun 2024 19:07:09 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: edumazet@...gle.com, 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] net: dqs: introduce NETIF_F_NO_BQL device feature

On Mon, Jun 10, 2024 at 2:33 PM Jiri Pirko <jiri@...nulli.us> wrote:
>
> Sun, Jun 09, 2024 at 03:17:32PM CEST, kerneljasonxing@...il.com wrote:
> >From: Jason Xing <kernelxing@...cent.com>
> >
> >Since commit 74293ea1c4db6 ("net: sysfs: Do not create sysfs for non
> >BQL device") limits the non-BQL driver not creating byte_queue_limits
> >directory, I found there is one exception, namely, virtio-net driver,
> >which should also be limited in netdev_uses_bql().
> >
> >I decided to introduce a NO_BQL bit in device feature because
> >1) it can help us limit virtio-net driver for now.
> >2) if we found another non-BQL driver, we can take it into account.
> >3) we can replace all the driver meeting those two statements in
> >netdev_uses_bql() in future.
> >
> >For now, I would like to make the first step to use this new bit for dqs
> >use instead of replacing/applying all the non-BQL drivers.
> >
> >After this patch, 1) there is no byte_queue_limits directory in virtio-net
> >driver. 2) running ethtool -k eth1 shows "no-bql: on [fixed]".
>
> Wait, you introduce this flag only for the sake of virtio_net driver,
> don't you. Since there is currently an attempt to implement bql in
> virtio_net, wouldn't it make this flag obsolete? Can't you wait?

I admit the way of introducing a new flag is not a good way to go, but
I cannot figure out a better way. I'm still thinking. Virtio_net
driver, I think, is not the only non-BQL we miss in dqs.

You can keep trying sending BQL patches for the virtio_net driver but
they don't conflict with the current patch. Can you guarantee you can
smoothly introduce BQL for virtio_net in a few days? I have no
confidence in your recent action about blindly deleting old features
and introducing a new one without considering the real world, I mean,
the world outside of the community. They are thousands and hundreds of
servers running outside. You have to prove deleting an old one doesn't
harm the users instead of asking users to prove the old feature is
useful. It's very weird. The key reason is about whether it breaks
userspace compatibility and how you handle the compatibility. For
example, I don't think kernel developers can delete some old proc
files even though they are buggy/inaccurate. I'm just saying what I'm
worried about. Let the maintainers decide finally.
+ Jason Wang, + Michael S. Tsirkin

Let's get back to this patch, I think actually it is a bug we need to
fix since netdev_uses_bql() decided to limit non-BQL drivers. I'm
still thinking, as I said...