public inbox for stable@vger.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.1 36/46] net: bridge: mcast: re-implement br_multicast_{enable, disable}_port functions
Date: Tue,  3 Jun 2025 21:03:54 -0400	[thread overview]
Message-ID: <20250604010404.5109-36-sashal@kernel.org> (raw)
In-Reply-To: <20250604010404.5109-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 5972821ce1950..e28c9db0c4db2 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1931,12 +1931,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);
 }
 
@@ -1963,11 +1968,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)
@@ -4156,9 +4217,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:05 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-04  1:03 [PATCH AUTOSEL 6.1 01/46] net: macb: Check return value of dma_set_mask_and_coherent() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 02/46] net: lan743x: Modify the EEPROM and OTP size for PCI1xxxx devices Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 03/46] tipc: use kfree_sensitive() for aead cleanup Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 04/46] bpf: Check rcu_read_lock_trace_held() in bpf_map_lookup_percpu_elem() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 05/46] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 06/46] emulex/benet: correct command version selection in be_cmd_get_stats() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 07/46] wifi: mt76: mt76x2: Add support for LiteOn WN4516R,WN4519R Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 08/46] wifi: mt76: mt7921: add 160 MHz AP for mt7922 device Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 09/46] sctp: Do not wake readers in __sctp_write_space() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 10/46] cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 11/46] i2c: tegra: check msg length in SMBUS block read Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 12/46] i2c: npcm: Add clock toggle recovery Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 13/46] net: dlink: add synchronization for stats update Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 14/46] wifi: ath11k: Fix QMI memory reuse logic Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 15/46] tcp: always seek for minimal rtt in tcp_rcv_rtt_update() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 16/46] tcp: fix initial tp->rcvq_space.space value for passive TS enabled flows Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 17/46] x86/sgx: Prevent attempts to reclaim poisoned pages Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 18/46] ipv4/route: Use this_cpu_inc() for stats on PREEMPT_RT Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 19/46] openvswitch: Stricter validation for the userspace action Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 20/46] net: atlantic: generate software timestamp just before the doorbell Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 21/46] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_set_by_name() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 22/46] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get_direction() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 23/46] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_gpio_set_direction() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 24/46] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 25/46] net: mlx4: add SOF_TIMESTAMPING_TX_SOFTWARE flag when getting ts info Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 26/46] net: vertexcom: mse102x: Return code for mse102x_rx_pkt_spi Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 27/46] wireless: purelifi: plfxlc: fix memory leak in plfxlc_usb_wreq_asyn() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 28/46] wifi: mac80211: do not offer a mesh path if forwarding is disabled Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 29/46] bpftool: Fix cgroup command to only show cgroup bpf programs Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 30/46] clk: rockchip: rk3036: mark ddrphy as critical Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 31/46] libbpf: Add identical pointer detection to btf_dedup_is_equiv() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 32/46] scsi: lpfc: Fix lpfc_check_sli_ndlp() handling for GEN_REQUEST64 commands Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 33/46] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 34/46] wifi: mac80211_hwsim: Prevent tsf from setting if beacon is disabled Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 35/46] net: bridge: mcast: update multicast contex when vlan state is changed Sasha Levin
2025-06-04  1:03 ` Sasha Levin [this message]
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 37/46] vxlan: Do not treat dst cache initialization errors as fatal Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 38/46] software node: Correct a OOB check in software_node_get_reference_args() Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 39/46] pinctrl: mcp23s08: Reset all pins to input at probe Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 40/46] scsi: lpfc: Use memcpy() for BIOS version Sasha Levin
2025-06-04  1:03 ` [PATCH AUTOSEL 6.1 41/46] sock: Correct error checking condition for (assign|release)_proto_idx() Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 6.1 42/46] i40e: fix MMIO write access to an invalid page in i40e_clear_hw Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 6.1 43/46] ice: fix check for existing switch rule Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 6.1 44/46] bpf, sockmap: Fix data lost during EAGAIN retries Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 6.1 45/46] net: ethernet: cortina: Use TOE/TSO on all TCP Sasha Levin
2025-06-04  1:04 ` [PATCH AUTOSEL 6.1 46/46] octeontx2-pf: Add error log forcn10k_map_unmap_rq_policer() 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=20250604010404.5109-36-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox