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 5.15 25/33] net: bridge: mcast: re-implement br_multicast_{enable, disable}_port functions
Date: Tue, 3 Jun 2025 21:05:16 -0400 [thread overview]
Message-ID: <20250604010524.6091-25-sashal@kernel.org> (raw)
In-Reply-To: <20250604010524.6091-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 3cd2b648408d6..085c9e706bc47 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)
@@ -4130,9 +4191,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
next prev parent reply other threads:[~2025-06-04 1:06 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-04 1:04 [PATCH AUTOSEL 5.15 01/33] net: macb: Check return value of dma_set_mask_and_coherent() Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 02/33] tipc: use kfree_sensitive() for aead cleanup Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 03/33] i2c: designware: Invoke runtime suspend on quick slave re-registration Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 04/33] emulex/benet: correct command version selection in be_cmd_get_stats() Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 05/33] wifi: mt76: mt76x2: Add support for LiteOn WN4516R,WN4519R Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 06/33] sctp: Do not wake readers in __sctp_write_space() Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 07/33] cpufreq: scmi: Skip SCMI devices that aren't used by the CPUs Sasha Levin
2025-06-04 1:04 ` [PATCH AUTOSEL 5.15 08/33] i2c: npcm: Add clock toggle recovery Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 09/33] net: dlink: add synchronization for stats update Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 10/33] tcp: always seek for minimal rtt in tcp_rcv_rtt_update() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 11/33] tcp: fix initial tp->rcvq_space.space value for passive TS enabled flows Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 12/33] ipv4/route: Use this_cpu_inc() for stats on PREEMPT_RT Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 13/33] openvswitch: Stricter validation for the userspace action Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 14/33] net: atlantic: generate software timestamp just before the doorbell Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 15/33] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_set_by_name() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 16/33] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get_direction() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 17/33] pinctrl: armada-37xx: propagate error from armada_37xx_pmx_gpio_set_direction() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 18/33] pinctrl: armada-37xx: propagate error from armada_37xx_gpio_get() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 19/33] net: mlx4: add SOF_TIMESTAMPING_TX_SOFTWARE flag when getting ts info Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 20/33] wifi: mac80211: do not offer a mesh path if forwarding is disabled Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 21/33] clk: rockchip: rk3036: mark ddrphy as critical Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 22/33] libbpf: Add identical pointer detection to btf_dedup_is_equiv() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 23/33] scsi: lpfc: Fix lpfc_check_sli_ndlp() handling for GEN_REQUEST64 commands Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 24/33] iommu/amd: Ensure GA log notifier callbacks finish running before module unload Sasha Levin
2025-06-04 1:05 ` Sasha Levin [this message]
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 26/33] vxlan: Do not treat dst cache initialization errors as fatal Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 27/33] software node: Correct a OOB check in software_node_get_reference_args() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 28/33] pinctrl: mcp23s08: Reset all pins to input at probe Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 29/33] scsi: lpfc: Use memcpy() for BIOS version Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 30/33] sock: Correct error checking condition for (assign|release)_proto_idx() Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 31/33] i40e: fix MMIO write access to an invalid page in i40e_clear_hw Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 32/33] bpf, sockmap: Fix data lost during EAGAIN retries Sasha Levin
2025-06-04 1:05 ` [PATCH AUTOSEL 5.15 33/33] 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=20250604010524.6091-25-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