Re: [PATCH net-next 3/4] bnge: add ndo_set_rx_mode_async support
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-07-29 02:09:13
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. ---
--- Patch [3]: [PATCH net-next 3/4] bnge: add ndo_set_rx_mode_async support ---commit 1fc0ed8b56a0fd5a87f788e357261a7f2693625b Author: Vikas Gupta [off-list ref] bnge: add ndo_set_rx_mode_async support This commit registers bnge_set_rx_mode() as ndo_set_rx_mode_async to handle filter updates asynchronously. It computes UC list updates directly from the snapshot, allowing sleepable HWRM commands without holding the address lock.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c index 1e3cdaeaa03dc..e67536a8c4304 100644 --- a/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c +++ b/drivers/net/ethernet/broadcom/bnge/bnge_netdev.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2272,6 +2269,48 @@ static int bnge_cfg_rx_mode(struct bnge_net *bn, struct netdev_hw_addr_list *uc, return rc; } +static int bnge_set_rx_mode(struct net_device *dev, + struct netdev_hw_addr_list *uc, + struct netdev_hw_addr_list *mc) +{ + struct bnge_net *bn = netdev_priv(dev); + struct bnge_vnic_info *vnic;
[ ... ]
+ if (dev->flags & IFF_ALLMULTI) {
+ mask |= CFA_L2_SET_RX_MASK_REQ_MASK_ALL_MCAST;
+ vnic->mc_list_count = 0;
+ } else if (dev->flags & IFF_MULTICAST) {
+ mc_update = bnge_mc_list_updated(bn, &mask, mc);
+ }
+
+ if (mask != vnic->rx_mask || uc_update || mc_update) {
+ vnic->rx_mask = mask;
+ return bnge_cfg_rx_mode(bn, uc, uc_update, true);
+ }[Severity: High] If bnge_cfg_rx_mode() returns -EAGAIN, the network core schedules a retry timer. However, because vnic->rx_mask is eagerly updated here (and vnic->mc_list is mutated in-place inside bnge_mc_list_updated()), won't the subsequent retry falsely detect no state changes and return 0 without programming the hardware? Could this leave the hardware filter state permanently desynchronized from the network stack?
+ + return 0; +}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2704,7 +2743,7 @@ static int bnge_init_chip(struct bnge_net *bn) vnic->rx_mask |= mask; } - rc = bnge_cfg_rx_mode(bn, &bn->netdev->uc, false); + rc = bnge_cfg_rx_mode(bn, &bn->netdev->uc, true, false); if (rc == -EAGAIN) { netif_rx_mode_schedule_retry(bn->netdev); rc = 0;
[Severity: High] This is a pre-existing issue, but does iterating the live netdev multicast list earlier in bnge_init_chip() require holding netif_addr_lock_bh()? Just above this call in bnge_init_chip(), the code evaluates: bnge_mc_list_updated(bn, &mask, &bn->netdev->mc) Since this happens without netif_addr_lock_bh() held, could it race with concurrent multicast updates (such as dev_mc_add() from IGMP/MLD timers), potentially leading to a use-after-free or infinite loop during list traversal?