All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Yong Wang <yongwang@nvidia.com>, Andy Roulin <aroulin@nvidia.com>,
	Ido Schimmel <idosch@nvidia.com>, Petr Machata <petrm@nvidia.com>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	"David S . Miller" <davem@davemloft.net>,
	Sasha Levin <sashal@kernel.org>,
	bridge@lists.linux.dev, netdev@vger.kernel.org
Subject: [PATCH AUTOSEL 6.6 44/62] net: bridge: mcast: re-implement br_multicast_{enable, disable}_port functions
Date: Tue,  3 Jun 2025 21:01:55 -0400	[thread overview]
Message-ID: <20250604010213.3462-44-sashal@kernel.org> (raw)
In-Reply-To: <20250604010213.3462-1-sashal@kernel.org>

From: Yong Wang <yongwang@nvidia.com>

[ Upstream commit 4b30ae9adb047dd0a7982975ec3933c529537026 ]

When a bridge port STP state is changed from BLOCKING/DISABLED to
FORWARDING, the port's igmp query timer will NOT re-arm itself if the
bridge has been configured as per-VLAN multicast snooping.

Solve this by choosing the correct multicast context(s) to enable/disable
port multicast based on whether per-VLAN multicast snooping is enabled or
not, i.e. using per-{port, VLAN} context in case of per-VLAN multicast
snooping by re-implementing br_multicast_enable_port() and
br_multicast_disable_port() functions.

Before the patch, the IGMP query does not happen in the last step of the
following test sequence, i.e. no growth for tx counter:
 # ip link add name br1 up type bridge vlan_filtering 1 mcast_snooping 1 mcast_vlan_snooping 1 mcast_querier 1 mcast_stats_enabled 1
 # bridge vlan global set vid 1 dev br1 mcast_snooping 1 mcast_querier 1 mcast_query_interval 100 mcast_startup_query_count 0
 # ip link add name swp1 up master br1 type dummy
 # bridge link set dev swp1 state 0
 # ip -j -p stats show dev swp1 group xstats_slave subgroup bridge suite mcast | jq '.[]["multicast"]["igmp_queries"]["tx_v2"]'
1
 # sleep 1
 # ip -j -p stats show dev swp1 group xstats_slave subgroup bridge suite mcast | jq '.[]["multicast"]["igmp_queries"]["tx_v2"]'
1
 # bridge link set dev swp1 state 3
 # sleep 2
 # ip -j -p stats show dev swp1 group xstats_slave subgroup bridge suite mcast | jq '.[]["multicast"]["igmp_queries"]["tx_v2"]'
1

After the patch, the IGMP query happens in the last step of the test:
 # ip link add name br1 up type bridge vlan_filtering 1 mcast_snooping 1 mcast_vlan_snooping 1 mcast_querier 1 mcast_stats_enabled 1
 # bridge vlan global set vid 1 dev br1 mcast_snooping 1 mcast_querier 1 mcast_query_interval 100 mcast_startup_query_count 0
 # ip link add name swp1 up master br1 type dummy
 # bridge link set dev swp1 state 0
 # ip -j -p stats show dev swp1 group xstats_slave subgroup bridge suite mcast | jq '.[]["multicast"]["igmp_queries"]["tx_v2"]'
1
 # sleep 1
 # ip -j -p stats show dev swp1 group xstats_slave subgroup bridge suite mcast | jq '.[]["multicast"]["igmp_queries"]["tx_v2"]'
1
 # bridge link set dev swp1 state 3
 # sleep 2
 # ip -j -p stats show dev swp1 group xstats_slave subgroup bridge suite mcast | jq '.[]["multicast"]["igmp_queries"]["tx_v2"]'
3

Signed-off-by: Yong Wang <yongwang@nvidia.com>
Reviewed-by: Andy Roulin <aroulin@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my comprehensive analysis, here's my assessment: **YES** ##
Analysis Summary This commit should be backported to stable kernel
trees. Here's my detailed reasoning: ### **What the Bug Fixes** The
commit fixes a critical bug where **IGMP query timers fail to re-arm**
when a bridge port transitions from BLOCKING/DISABLED to FORWARDING
state in **per-VLAN multicast snooping** configurations. ### **Technical
Problem** 1. **Context**: When `BROPT_MCAST_VLAN_SNOOPING_ENABLED` is
active, each VLAN has its own multicast context (`port_mcast_ctx`) with
separate query timers 2. **Bug**: The old implementation in
`br_multicast_enable_port()` and `br_multicast_disable_port()` only
operated on the port's main multicast context (`port->multicast_ctx`) 3.
**Impact**: Per-VLAN multicast contexts were never properly
enabled/disabled during STP state transitions 4. **Result**: IGMP/MLD
query timers for VLANs remain permanently disabled, causing multicast
flooding instead of proper snooping ### **Evidence from Commit** The
commit message provides clear **before/after test results**: -
**Before**: IGMP query count stays at 1 (timer not re-arming) -
**After**: IGMP query count increases to 3 (timer properly re-arming)
This demonstrates the fix works and solves a real, testable problem. ###
**Code Analysis** The fix **re-implements** `br_multicast_enable_port()`
and `br_multicast_disable_port()` to: 1. **Check VLAN snooping status**:
If `BROPT_MCAST_VLAN_SNOOPING_ENABLED` is set 2. **Iterate through
VLANs**: Process each VLAN's multicast context individually 3. **Proper
state handling**: Only enable contexts for VLANs in LEARNING/FORWARDING
states 4. **Fallback behavior**: Use old behavior when VLAN snooping is
disabled ### **Backporting Criteria Met** ✅ **Bug Fix**: Fixes
functional regression in multicast snooping ✅ **Self-contained**:
Changes isolated to bridge multicast code ✅ **No API changes**: Pure
internal refactoring with same public interface ✅ **Production Impact**:
Affects real-world VLAN multicast deployments ✅ **Well-tested**:
Includes specific test case demonstrating the fix ✅ **Minimal Risk**:
Well-reviewed code from experienced maintainers (Nvidia networking team)
### **Stability Impact** - **Risk Level**: Low - internal refactoring
with clear improvement - **Regression Potential**: Very low - maintains
backward compatibility - **User Impact**: High - fixes broken multicast
functionality in VLAN scenarios This is exactly the type of targeted bug
fix that stable trees are designed to include: it fixes a clear
functional regression affecting production deployments while carrying
minimal risk of introducing new issues.

 net/bridge/br_multicast.c | 77 +++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index ea71a64f915f9..fa16ee88ec396 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -2104,12 +2104,17 @@ static void __br_multicast_enable_port_ctx(struct net_bridge_mcast_port *pmctx)
 	}
 }
 
-void br_multicast_enable_port(struct net_bridge_port *port)
+static void br_multicast_enable_port_ctx(struct net_bridge_mcast_port *pmctx)
 {
-	struct net_bridge *br = port->br;
+	struct net_bridge *br = pmctx->port->br;
 
 	spin_lock_bh(&br->multicast_lock);
-	__br_multicast_enable_port_ctx(&port->multicast_ctx);
+	if (br_multicast_port_ctx_is_vlan(pmctx) &&
+	    !(pmctx->vlan->priv_flags & BR_VLFLAG_MCAST_ENABLED)) {
+		spin_unlock_bh(&br->multicast_lock);
+		return;
+	}
+	__br_multicast_enable_port_ctx(pmctx);
 	spin_unlock_bh(&br->multicast_lock);
 }
 
@@ -2136,11 +2141,67 @@ static void __br_multicast_disable_port_ctx(struct net_bridge_mcast_port *pmctx)
 	br_multicast_rport_del_notify(pmctx, del);
 }
 
+static void br_multicast_disable_port_ctx(struct net_bridge_mcast_port *pmctx)
+{
+	struct net_bridge *br = pmctx->port->br;
+
+	spin_lock_bh(&br->multicast_lock);
+	if (br_multicast_port_ctx_is_vlan(pmctx) &&
+	    !(pmctx->vlan->priv_flags & BR_VLFLAG_MCAST_ENABLED)) {
+		spin_unlock_bh(&br->multicast_lock);
+		return;
+	}
+
+	__br_multicast_disable_port_ctx(pmctx);
+	spin_unlock_bh(&br->multicast_lock);
+}
+
+static void br_multicast_toggle_port(struct net_bridge_port *port, bool on)
+{
+#if IS_ENABLED(CONFIG_BRIDGE_VLAN_FILTERING)
+	if (br_opt_get(port->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED)) {
+		struct net_bridge_vlan_group *vg;
+		struct net_bridge_vlan *vlan;
+
+		rcu_read_lock();
+		vg = nbp_vlan_group_rcu(port);
+		if (!vg) {
+			rcu_read_unlock();
+			return;
+		}
+
+		/* iterate each vlan, toggle vlan multicast context */
+		list_for_each_entry_rcu(vlan, &vg->vlan_list, vlist) {
+			struct net_bridge_mcast_port *pmctx =
+						&vlan->port_mcast_ctx;
+			u8 state = br_vlan_get_state(vlan);
+			/* enable vlan multicast context when state is
+			 * LEARNING or FORWARDING
+			 */
+			if (on && br_vlan_state_allowed(state, true))
+				br_multicast_enable_port_ctx(pmctx);
+			else
+				br_multicast_disable_port_ctx(pmctx);
+		}
+		rcu_read_unlock();
+		return;
+	}
+#endif
+	/* toggle port multicast context when vlan snooping is disabled */
+	if (on)
+		br_multicast_enable_port_ctx(&port->multicast_ctx);
+	else
+		br_multicast_disable_port_ctx(&port->multicast_ctx);
+}
+
+void br_multicast_enable_port(struct net_bridge_port *port)
+{
+	br_multicast_toggle_port(port, true);
+}
+
 void br_multicast_disable_port(struct net_bridge_port *port)
 {
-	spin_lock_bh(&port->br->multicast_lock);
-	__br_multicast_disable_port_ctx(&port->multicast_ctx);
-	spin_unlock_bh(&port->br->multicast_lock);
+	br_multicast_toggle_port(port, false);
 }
 
 static int __grp_src_delete_marked(struct net_bridge_port_group *pg)
@@ -4329,9 +4390,9 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
 		__br_multicast_open(&br->multicast_ctx);
 	list_for_each_entry(p, &br->port_list, list) {
 		if (on)
-			br_multicast_disable_port(p);
+			br_multicast_disable_port_ctx(&p->multicast_ctx);
 		else
-			br_multicast_enable_port(p);
+			br_multicast_enable_port_ctx(&p->multicast_ctx);
 	}
 
 	list_for_each_entry(vlan, &vg->vlan_list, vlist)
-- 
2.39.5


  parent reply	other threads:[~2025-06-04  1:03 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04  1:01 [PATCH AUTOSEL 6.6 01/62] net: macb: Check return value of dma_set_mask_and_coherent() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 02/62] net: lan743x: Modify the EEPROM and OTP size for PCI1xxxx devices Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 03/62] tipc: use kfree_sensitive() for aead cleanup Sasha Levin
2025-06-04  1:01 ` [f2fs-dev] [PATCH AUTOSEL 6.6 04/62] f2fs: use vmalloc instead of kvmalloc in .init_{, de}compress_ctx Sasha Levin via Linux-f2fs-devel
2025-06-04  1:01   ` [PATCH AUTOSEL 6.6 04/62] f2fs: use vmalloc instead of kvmalloc in .init_{,de}compress_ctx Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 05/62] bpf: Check rcu_read_lock_trace_held() in bpf_map_lookup_percpu_elem() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 06/62] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 07/62] wifi: mt76: mt7996: drop fragments with multicast or broadcast RA Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 08/62] emulex/benet: correct command version selection in be_cmd_get_stats() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 09/62] wifi: mt76: mt76x2: Add support for LiteOn WN4516R,WN4519R Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 10/62] wifi: mt76: mt7921: add 160 MHz AP for mt7922 device Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 11/62] sctp: Do not wake readers in __sctp_write_space() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 12/62] cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 13/62] i2c: tegra: check msg length in SMBUS block read Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 14/62] i2c: npcm: Add clock toggle recovery Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 15/62] net: dlink: add synchronization for stats update Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 16/62] wifi: ath12k: fix macro definition HAL_RX_MSDU_PKT_LENGTH_GET Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 17/62] wifi: ath12k: fix a possible dead lock caused by ab->base_lock Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 18/62] wifi: ath11k: Fix QMI memory reuse logic Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 19/62] wifi: rtw89: leave idle mode when setting WEP encryption for AP mode Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 20/62] tcp: always seek for minimal rtt in tcp_rcv_rtt_update() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 21/62] tcp: fix initial tp->rcvq_space.space value for passive TS enabled flows Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 22/62] x86/sgx: Prevent attempts to reclaim poisoned pages Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 23/62] ipv4/route: Use this_cpu_inc() for stats on PREEMPT_RT Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 24/62] openvswitch: Stricter validation for the userspace action Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 25/62] net: atlantic: generate software timestamp just before the doorbell Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 26/62] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_set_by_name() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 27/62] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get_direction() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 28/62] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_gpio_set_direction() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 29/62] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 30/62] net: mlx4: add SOF_TIMESTAMPING_TX_SOFTWARE flag when getting ts info Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 31/62] net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 32/62] wireless: purelifi: plfxlc: fix memory leak in plfxlc_usb_wreq_asyn() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 33/62] wifi: mac80211: do not offer a mesh path if forwarding is disabled Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 34/62] bpftool: Fix cgroup command to only show cgroup bpf programs Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 35/62] clk: rockchip: rk3036: mark ddrphy as critical Sasha Levin
2025-06-04  1:01   ` Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 36/62] libbpf: Add identical pointer detection to btf_dedup_is_equiv() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 37/62] scsi: lpfc: Fix lpfc_check_sli_ndlp() handling for GEN_REQUEST64 commands Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 38/62] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 39/62] wifi: iwlwifi: pcie: make sure to lock rxq->read Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 40/62] wifi: mac80211_hwsim: Prevent tsf from setting if beacon is disabled Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 41/62] wifi: mac80211: VLAN traffic in multicast path Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 42/62] wifi: iwlwifi: Add missing MODULE_FIRMWARE for Qu-c0-jf-b0 Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 43/62] net: bridge: mcast: update multicast contex when vlan state is changed Sasha Levin
2025-06-04  1:01 ` Sasha Levin [this message]
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 45/62] vxlan: Do not treat dst cache initialization errors as fatal Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 46/62] net: ethernet: ti: am65-cpsw: handle -EPROBE_DEFER Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 47/62] software node: Correct a OOB check in software_node_get_reference_args() Sasha Levin
2025-06-04  1:01 ` [PATCH AUTOSEL 6.6 48/62] pinctrl: mcp23s08: Reset all pins to input at probe Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 49/62] wifi: ath12k: fix failed to set mhi state error during reboot with hardware grouping Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 50/62] scsi: lpfc: Use memcpy() for BIOS version Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 51/62] sock: Correct error checking condition for (assign|release)_proto_idx() Sasha Levin
2025-06-04  1:02 ` [Intel-wired-lan] [PATCH AUTOSEL 6.6 52/62] i40e: fix MMIO write access to an invalid page in i40e_clear_hw Sasha Levin
2025-06-04  1:02   ` Sasha Levin
2025-06-04  1:02 ` [Intel-wired-lan] [PATCH AUTOSEL 6.6 53/62] ice: fix check for existing switch rule Sasha Levin
2025-06-04  1:02   ` Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 54/62] usbnet: asix AX88772: leave the carrier control to phylink Sasha Levin
2025-06-04  1:02 ` [f2fs-dev] [PATCH AUTOSEL 6.6 55/62] f2fs: fix to set atomic write status more clear Sasha Levin via Linux-f2fs-devel
2025-06-04  1:02   ` Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 56/62] bpf, sockmap: Fix data lost during EAGAIN retries Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 57/62] net: ethernet: cortina: Use TOE/TSO on all TCP Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 58/62] octeontx2-pf: Add error log forcn10k_map_unmap_rq_policer() Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 59/62] wifi: ath11k: determine PM policy based on machine model Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 60/62] wifi: ath12k: fix link valid field initialization in the monitor Rx Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 61/62] wifi: ath12k: fix incorrect CE addresses Sasha Levin
2025-06-04  1:02 ` [PATCH AUTOSEL 6.6 62/62] wifi: ath12k: Pass correct values of center freq1 and center freq2 for 160 MHz Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250604010213.3462-44-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=aroulin@nvidia.com \
    --cc=bridge@lists.linux.dev \
    --cc=davem@davemloft.net \
    --cc=idosch@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=petrm@nvidia.com \
    --cc=razor@blackwall.org \
    --cc=stable@vger.kernel.org \
    --cc=yongwang@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.