netdev - Re: [PATCH net-next v4] net: bonding: use atomic instead of rtnl

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: <7E50B628-5ABD-4263-BCCC-31227A1E02E8@bamaicloud.com>
Date: Mon, 17 Nov 2025 11:56:56 +0800
From: Tonghao Zhang <tonghao@...aicloud.com>
To: Paolo Abeni <pabeni@...hat.com>,
 Jay Vosburgh <jv@...sburgh.net>
Cc: netdev@...r.kernel.org,
 Jay Vosburgh <jv@...sburgh.net>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Simon Horman <horms@...nel.org>,
 Jonathan Corbet <corbet@....net>,
 Andrew Lunn <andrew+netdev@...n.ch>,
 Nikolay Aleksandrov <razor@...ckwall.org>,
 Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [PATCH net-next v4] net: bonding: use atomic instead of
 rtnl_mutex, to make sure peer notify updated

> On Nov 13, 2025, at 18:25, Paolo Abeni <pabeni@...hat.com> wrote:
> 
> On 11/10/25 10:13 AM, Tonghao Zhang wrote:
>> @@ -814,4 +814,11 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
>> return NET_XMIT_DROP;
>> }
>> 
>> +static inline void bond_peer_notify_reset(struct bonding *bond)
>> +{
>> + atomic_set(&bond->send_peer_notif,
>> + bond->params.num_peer_notif *
>> + max(1, bond->params.peer_notif_delay));
> 
> The above reads params.* without any due lock; at very least it should
> include READ_ONCE() annotation here (and WRITE_ONCE on the param update
> side).
Yes, the rtnl lock is necessary.
> 
> But it also mean it could observe inconsistent values for
> params.num_peer_notif and params.peer_notif_delay in case of concurrent
> update.
> 
> The possible race between bond_mii_monitor() and
> bond_change_active_slave() concurrently updating
> send_peer_notif is still avoided by the rtnl lock, so the changelog is
> IMHO confusing WRT the actual code semantic.
In fact, replacing rtnl lock with atomic is not easy (bonding parameters updates and call_netdevice_notifiers still require rtnl lock), and atomic makes the update logic more complicated. I think the workqueue can solve the problem of updating send_peer_notif.

Hi Jay, can I repost the patch? https://patchwork.kernel.org/project/netdevbpf/patch/20251021052249.47250-1-tonghao@bamaicloud.com/
> 
> /P
> 
>