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] [day] [month] [year] [list]

Message-ID: <CAL+tcoAPq3WpcSvqwUgiQJrEbJC4pYv0Omq9-Y53vFrCiQMVhQ@mail.gmail.com>
Date: Fri, 14 Jun 2024 00:20:57 +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:49 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Thu, Jun 13, 2024 at 5:37 PM Jason Xing <kerneljasonxing@...il.com> wrote:
> >
> > 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:
>
> >     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... :(
>
> I think you missed the point of the original suggestion leading to Breno patch.
>
> At Google, we create gazillion of netns per minute, with few virtual
> drivers in them (like loopback, ipvlan)
>
> This was pretty easy to avoid /sys/class/net/lo/queues/tx-0/byte_queue_limits/*
> creation. cpu savings for little investment.
>
> But when it comes to physical devices, I really do not see the benefit
> of being picky about some sysfs files.
>
> So let me repeat my question : Why do you need this ?

Sometimes people can see the sysfs files, sometimes not. They may get
confused. For non-BQL drivers, those sysfs files are totally needless
(not working) with one single flag.

Well, Eric, I don't expect this patch to keep involving more
discussions. If you or Jakub or other maintainers decide to reject
this patch, I'm fine. I can take it. We spend too much time on this
trivial patch. Many patches like this are trivial, not worth spending
you too much precious time on this. As I said, I thought it's very
simple and easy...

Thanks for your explanation.

Thanks,
Jason