| 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
| ||
|
Message-ID: <aWSwnz8XieyG7FUB@fedora>
Date: Mon, 12 Jan 2026 08:28:15 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Tonghao Zhang <tonghao@...aicloud.com>
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>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
Andrew Lunn <andrew+netdev@...n.ch>,
Nikolay Aleksandrov <razor@...ckwall.org>,
Jason Xing <kerneljasonxing@...il.com>
Subject: Re: [PATCH RESEND net-next v4 1/4] net: bonding: use workqueue to
make sure peer notify updated in lacp mode
On Mon, Jan 12, 2026 at 10:40:48AM +0800, Tonghao Zhang wrote:
> The rtnl lock might be locked, preventing ad_cond_set_peer_notif() from
> acquiring the lock and updating send_peer_notif. This patch addresses
> the issue by using a workqueue. Since updating send_peer_notif does
> not require high real-time performance, such delayed updates are entirely
> acceptable.
>
> In fact, checking this value and using it in multiple places, all operations
> are protected at the same time by rtnl lock, such as
> - read send_peer_notif
> - send_peer_notif--
> - bond_should_notify_peers
>
> By the way, rtnl lock is still required, when accessing bond.params.* for
> updating send_peer_notif. In lacp mode, resetting send_peer_notif in
> workqueue is safe, simple and effective way.
>
> Cc: Jay Vosburgh <jv@...sburgh.net>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Simon Horman <horms@...nel.org>
> Cc: Jonathan Corbet <corbet@....net>
> Cc: Andrew Lunn <andrew+netdev@...n.ch>
> Cc: Nikolay Aleksandrov <razor@...ckwall.org>
> Cc: Hangbin Liu <liuhangbin@...il.com>
> Cc: Jason Xing <kerneljasonxing@...il.com>
> Suggested-by: Hangbin Liu <liuhangbin@...il.com>
> Signed-off-by: Tonghao Zhang <tonghao@...aicloud.com>
> ---
> v4:
> - keep the netdevice notifier order.
> v2/3:
> - no change
> v1:
> - This patch is actually version v3, https://patchwork.kernel.org/project/netdevbpf/patch/20251118090305.35558-1-tonghao@bamaicloud.com/
> - add a comment why we use the trylock.
> - add this patch to series
> ---
> drivers/net/bonding/bond_3ad.c | 7 ++--
> drivers/net/bonding/bond_main.c | 57 +++++++++++++++++++++++++--------
> include/net/bonding.h | 2 ++
> 3 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index 1a8de2bf8655..01ae0269a138 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -1008,11 +1008,8 @@ static void ad_cond_set_peer_notif(struct port *port)
> {
> struct bonding *bond = port->slave->bond;
>
> - if (bond->params.broadcast_neighbor && rtnl_trylock()) {
> - bond->send_peer_notif = bond->params.num_peer_notif *
> - max(1, bond->params.peer_notif_delay);
> - rtnl_unlock();
> - }
> + if (bond->params.broadcast_neighbor)
> + bond_peer_notify_work_rearm(bond, 0);
> }
>
> /**
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 3d56339a8a10..edf6dac8a98f 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1195,6 +1195,35 @@ static bool bond_should_notify_peers(struct bonding *bond)
> return true;
> }
>
> +/* Use this to update send_peer_notif when RTNL may be held in other places. */
> +void bond_peer_notify_work_rearm(struct bonding *bond, unsigned long delay)
> +{
> + queue_delayed_work(bond->wq, &bond->peer_notify_work, delay);
> +}
> +
> +/* Peer notify update handler. Holds only RTNL */
> +static void bond_peer_notify_reset(struct bonding *bond)
> +{
> + bond->send_peer_notif = bond->params.num_peer_notif *
> + max(1, bond->params.peer_notif_delay);
> +}
> +
> +static void bond_peer_notify_handler(struct work_struct *work)
> +{
> + struct bonding *bond = container_of(work, struct bonding,
> + peer_notify_work.work);
> +
> + if (!rtnl_trylock()) {
> + bond_peer_notify_work_rearm(bond, 1);
> + return;
> + }
> +
> + bond_peer_notify_reset(bond);
> +
> + rtnl_unlock();
> + return;
> +}
> +
> /**
> * bond_change_active_slave - change the active slave into the specified one
> * @bond: our bonding struct
> @@ -1270,8 +1299,6 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> BOND_SLAVE_NOTIFY_NOW);
>
> if (new_active) {
> - bool should_notify_peers = false;
> -
> bond_set_slave_active_flags(new_active,
> BOND_SLAVE_NOTIFY_NOW);
>
> @@ -1279,19 +1306,17 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> bond_do_fail_over_mac(bond, new_active,
> old_active);
>
> + call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
> +
> if (netif_running(bond->dev)) {
> - bond->send_peer_notif =
> - bond->params.num_peer_notif *
> - max(1, bond->params.peer_notif_delay);
> - should_notify_peers =
> - bond_should_notify_peers(bond);
> - }
> + bond_peer_notify_reset(bond);
>
> - call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, bond->dev);
> - if (should_notify_peers) {
> - bond->send_peer_notif--;
> - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
> - bond->dev);
> + if (bond_should_notify_peers(bond)) {
> + bond->send_peer_notif--;
> + call_netdevice_notifiers(
> + NETDEV_NOTIFY_PEERS,
> + bond->dev);
> + }
> }
> }
> }
> @@ -4213,6 +4238,10 @@ static u32 bond_xmit_hash_xdp(struct bonding *bond, struct xdp_buff *xdp)
>
> void bond_work_init_all(struct bonding *bond)
> {
> + /* ndo_stop, bond_close() will try to flush the work under
> + * the rtnl lock. The workqueue must not block on rtnl lock
> + * to avoid deadlock.
> + */
> INIT_DELAYED_WORK(&bond->mcast_work,
> bond_resend_igmp_join_requests_delayed);
> INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor);
> @@ -4220,6 +4249,7 @@ void bond_work_init_all(struct bonding *bond)
> INIT_DELAYED_WORK(&bond->arp_work, bond_arp_monitor);
> INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
> INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler);
> + INIT_DELAYED_WORK(&bond->peer_notify_work, bond_peer_notify_handler);
> }
>
> void bond_work_cancel_all(struct bonding *bond)
> @@ -4230,6 +4260,7 @@ void bond_work_cancel_all(struct bonding *bond)
> cancel_delayed_work_sync(&bond->ad_work);
> cancel_delayed_work_sync(&bond->mcast_work);
> cancel_delayed_work_sync(&bond->slave_arr_work);
> + cancel_delayed_work_sync(&bond->peer_notify_work);
> }
>
> static int bond_open(struct net_device *bond_dev)
> diff --git a/include/net/bonding.h b/include/net/bonding.h
> index 49edc7da0586..63d08056a4a4 100644
> --- a/include/net/bonding.h
> +++ b/include/net/bonding.h
> @@ -254,6 +254,7 @@ struct bonding {
> struct delayed_work ad_work;
> struct delayed_work mcast_work;
> struct delayed_work slave_arr_work;
> + struct delayed_work peer_notify_work;
> #ifdef CONFIG_DEBUG_FS
> /* debugging support via debugfs */
> struct dentry *debug_dir;
> @@ -709,6 +710,7 @@ struct bond_vlan_tag *bond_verify_device_path(struct net_device *start_dev,
> int level);
> int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave);
> void bond_slave_arr_work_rearm(struct bonding *bond, unsigned long delay);
> +void bond_peer_notify_work_rearm(struct bonding *bond, unsigned long delay);
> void bond_work_init_all(struct bonding *bond);
> void bond_work_cancel_all(struct bonding *bond);
>
> --
> 2.34.1
>
Reviewed-by: Hangbin Liu <liuhangbin@...il.com>
Powered by blists - more mailing lists