netdev - Re: [PATCH net-next 2/3] net: devmem: don't call queue stop / start when the interface is down

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]

Message-ID: <CAHS8izNb3-Ly3D5qmczHjObD1zWkgOAbovMN5FY=6uqTyBHC3Q@mail.gmail.com>
Date: Thu, 6 Feb 2025 08:34:21 -0800
From: Mina Almasry <almasrymina@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org
Subject: Re: [PATCH net-next 2/3] net: devmem: don't call queue stop / start
 when the interface is down

On Wed, Feb 5, 2025 at 5:26 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Wed, 5 Feb 2025 12:35:30 -0800 Mina Almasry wrote:
> > Why not return an error if !netif_running(), and change the call site
> > in net_devmem_unbind_dmabuf() to not call into this if
> > !netif_running()? Is that a bit cleaner? It feels a bit weird to have
> > netdev_rx_queue_restart() do a bunch of allocations and driver calls
> > unnecessarily when it's really not going to do anything, no?
>
> The bindings survive ifdown, right? So presumably they exist while
> the device is down. If they exist while the device is down, why can
> they not be created? Feels inconsistent.

Ah, good point. I guess binding to a device that is down is WAI indeed.

Another approach could be to return success without doing any driver
calls at all then, because when the device is down there is no running
queue to restart. Although that would have an effect that devmem TCP
would start working on devices that don't support the queue API at all
(I guess the user could bring the interface down, do bindings, then
back up).

But this works too, so

Reviewed-by: Mina Almasry <almasrymina@...gle.com>

-- 
Thanks,
Mina