netdev - Re: [PATCH net-next] netpoll: remove netpoll

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: <20240905-witty-naughty-kakapo-7eae8b@devvm32600>
Date: Thu, 5 Sep 2024 02:27:40 -0700
From: Breno Leitao <leitao@...ian.org>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next] netpoll: remove netpoll_srcu

Hello Eric,

On Thu, Sep 05, 2024 at 08:49:09AM +0000, Eric Dumazet wrote:
> netpoll_srcu is currently used from netpoll_poll_disable() and
> __netpoll_cleanup()
> 
> Both functions run under RTNL, using netpoll_srcu adds confusion
> and no additional protection.

Thanks for fixing it. I have never understood that SRCU in netpoll.

Reading this code now again, I have the impression that netpoll_srcu was
created to be able to dereference ->npinfo using an RCU primitive.

> Moreover the synchronize_srcu() call in __netpoll_cleanup() is
> performed before clearing np->dev->npinfo, which violates RCU rules.

I think the goal was to make sure that all the readers are not in the
critical section anymore, and wait for them. But it is unclear why it is
mixing srcu_read_lock() and rcu_read_lock() together.

Anyway, thanks for solving this.

> After this patch, netpoll_poll_disable() and netpoll_poll_enable()
> simply use rtnl_dereference().
> 
> This saves a big chunk of memory (more than 192KB on platforms
> with 512 cpus)
> 
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Breno Leitao <leitao@...ian.org>

Reviewed-by: Breno Leitao <leitao@...ian.org>

I've double checked that netpoll_poll_disable() and
netpoll_poll_enable() is always called with rtnl held, so, the change is
safe.