[PATCH net-next 0/5] net: bridge: propagate safe mcast snooping to switchdev + DSA

[PATCH net-next 0/5] net: bridge: propagate safe mcast snooping to switchdev + DSA

[Linus Lüssing]

For a plain Linux bridge we have a safety mechanism before applying
multicast snooping to payload IP packets in the fast path: We only apply it
if both multicast snooping is enabled and an active IGMP/MLD querier was
detected. Otherwise we default to flooding IPv4/IPv6 multicast traffic.

This reduces the risk of creating multicast packet loss and by that
packet loss for IPv6 unicast, too, which relies on multicast to work.
Without an active IGMP/MLD querier on the link we are not able to get
IGMP/MLD reports reliably and by that wouldn't have a complete picture
about all multicast listeners.

This safety mechanism was introduced in commit
b00589af3b04 ("bridge: disable snooping if there is no querier").

To be able to use this safty mechanism on DSA/switchdev capable hardware
switches, too, and to ensure that a DSA bridge behaves similar to
a plain software bridge this patchset adds a new variable to track
if multicast snooping is active / safely applicable. And notifies DSA
and switchdev when this changes.

This has been tested on an OpenWrt powered Realtek RTL8382 switch,
a ZyXEL GS1900-24HP v1, with the following, pending patchset for OpenWrt
to integrate this: https://github.com/openwrt/openwrt/pull/18780

Regards, Linus

[PATCH net-next 1/5] net: bridge: mcast: explicitly track active state

[Linus Lüssing]

Combining and tracking the active state into new, per protocol family
variables.

For one thing this slightly reduces the work and checks in fast path
for multicast payload traffic. For another this is in preparation to
be able to notify DSA/switchdev on the (in)applicability of multicast
snooping on multicast payload traffic. Especially on vanishing IGMP/MLD
queriers.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/br_device.c    |   5 +-
 net/bridge/br_input.c     |   2 +-
 net/bridge/br_multicast.c | 179 +++++++++++++++++++++++++++++++++++---
 net/bridge/br_private.h   |  33 ++-----
 4 files changed, 180 insertions(+), 39 deletions(-)

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index a818fdc22da9..315ed3d33406 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -102,7 +102,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		mdst = br_mdb_entry_skb_get(brmctx, skb, vid);
 		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
-		    br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst))
+		    br_multicast_snooping_active(brmctx, eth_hdr(skb), mdst))
 			br_multicast_flood(mdst, skb, brmctx, false, true);
 		else
 			br_flood(br, skb, BR_PKT_MULTICAST, false, true, vid);
@@ -168,7 +168,10 @@ static int br_dev_open(struct net_device *dev)
 	netdev_update_features(dev);
 	netif_start_queue(dev);
 	br_stp_enable_bridge(br);
+
+	spin_lock_bh(&br->multicast_lock);
 	br_multicast_open(br);
+	spin_unlock_bh(&br->multicast_lock);
 
 	if (br_opt_get(br, BROPT_MULTICAST_ENABLED))
 		br_multicast_join_snoopers(br);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 232133a0fd21..0c632655d66c 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -187,7 +187,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 	case BR_PKT_MULTICAST:
 		mdst = br_mdb_entry_skb_get(brmctx, skb, vid);
 		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
-		    br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst)) {
+		    br_multicast_snooping_active(brmctx, eth_hdr(skb), mdst)) {
 			if ((mdst && mdst->host_joined) ||
 			    br_multicast_is_router(brmctx, skb)) {
 				local_rcv = true;
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index dcbf058de1e3..b66d2173e321 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1069,6 +1069,125 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm
 	return skb;
 }
 
+static bool
+__br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
+			      struct bridge_mcast_other_query *querier,
+			      const bool is_ipv6)
+{
+	bool own_querier_enabled;
+
+	if (brmctx->multicast_querier) {
+		if (is_ipv6 && !br_opt_get(brmctx->br, BROPT_HAS_IPV6_ADDR))
+			own_querier_enabled = false;
+		else
+			own_querier_enabled = true;
+	} else {
+		own_querier_enabled = false;
+	}
+
+	return !timer_pending(&querier->delay_timer) &&
+	       (own_querier_enabled || timer_pending(&querier->timer));
+}
+
+static bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
+					struct ethhdr *eth,
+					const struct net_bridge_mdb_entry *mdb)
+{
+	switch (eth->h_proto) {
+	case (htons(ETH_P_IP)):
+		return __br_multicast_querier_exists(brmctx,
+			&brmctx->ip4_other_query, false);
+#if IS_ENABLED(CONFIG_IPV6)
+	case (htons(ETH_P_IPV6)):
+		return __br_multicast_querier_exists(brmctx,
+			&brmctx->ip6_other_query, true);
+#endif
+	default:
+		return !!mdb && br_group_is_l2(&mdb->addr);
+	}
+}
+
+static bool br_ip4_multicast_check_active(struct net_bridge_mcast *brmctx,
+					  bool *active)
+{
+	if (!__br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query,
+					   false))
+		*active = false;
+
+	if (brmctx->ip4_active == *active)
+		return false;
+
+	return true;
+}
+
+static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
+					 bool *active)
+{
+#if IS_ENABLED(CONFIG_IPV6)
+	if (!__br_multicast_querier_exists(brmctx, &brmctx->ip6_other_query,
+					   true))
+		*active = false;
+
+	if (brmctx->ip6_active == *active)
+		return false;
+
+	return true;
+#elif
+	*active = false;
+	return false;
+#endif
+}
+
+/**
+ * __br_multicast_update_active() - update mcast active state
+ * @brmctx: the bridge multicast context to check
+ * @force_inactive: forcefully deactivate mcast active state
+ * @extack: netlink extended ACK structure
+ *
+ * This (potentially) updates the IPv4/IPv6 multicast active state. And by
+ * that enables or disables snooping of multicast payload traffic in fast
+ * path.
+ *
+ * The multicast active state is set, per protocol family, if:
+ *
+ * - an IGMP/MLD querier is present
+ * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge
+ *
+ * And is unset otherwise.
+ *
+ * This function should be called by anything that changes one of the
+ * above prerequisites.
+ *
+ * Return: 0 on success, a negative value otherwise.
+ */
+static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
+					bool force_inactive,
+					struct netlink_ext_ack *extack)
+{
+	bool ip4_active, ip6_active, ip4_changed, ip6_changed;
+	int ret = 0;
+
+	lockdep_assert_held_once(&brmctx->br->multicast_lock);
+
+	ip4_active = !force_inactive;
+	ip6_active = !force_inactive;
+	ip4_changed = br_ip4_multicast_check_active(brmctx, &ip4_active);
+	ip6_changed = br_ip6_multicast_check_active(brmctx, &ip6_active);
+
+	if (ip4_changed)
+		brmctx->ip4_active = ip4_active;
+	if (ip6_changed)
+		brmctx->ip6_active = ip6_active;
+
+	return ret;
+}
+
+static int br_multicast_update_active(struct net_bridge_mcast *brmctx,
+				      struct netlink_ext_ack *extack)
+{
+	return __br_multicast_update_active(brmctx, false, extack);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 						    struct net_bridge_mcast_port *pmctx,
@@ -1147,10 +1266,12 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brm
 			       &ip6h->daddr, 0, &ip6h->saddr)) {
 		kfree_skb(skb);
 		br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, false);
+		br_multicast_update_active(brmctx, NULL);
 		return NULL;
 	}
 
 	br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, true);
+	br_multicast_update_active(brmctx, NULL);
 	ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
 
 	hopopt = (u8 *)(ip6h + 1);
@@ -1762,10 +1883,28 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t)
 }
 #endif
 
-static void br_multicast_query_delay_expired(struct timer_list *t)
+static void br_ip4_multicast_query_delay_expired(struct timer_list *t)
 {
+	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
+						     ip4_other_query.delay_timer);
+
+	spin_lock(&brmctx->br->multicast_lock);
+	br_multicast_update_active(brmctx, NULL);
+	spin_unlock(&brmctx->br->multicast_lock);
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+static void br_ip6_multicast_query_delay_expired(struct timer_list *t)
+{
+	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
+						     ip6_other_query.delay_timer);
+
+	spin_lock(&brmctx->br->multicast_lock);
+	br_multicast_update_active(brmctx, NULL);
+	spin_unlock(&brmctx->br->multicast_lock);
+}
+#endif
+
 static void br_multicast_select_own_querier(struct net_bridge_mcast *brmctx,
 					    struct br_ip *ip,
 					    struct sk_buff *skb)
@@ -3981,16 +4120,13 @@ static void br_multicast_query_expired(struct net_bridge_mcast *brmctx,
 				       struct bridge_mcast_own_query *query,
 				       struct bridge_mcast_querier *querier)
 {
-	spin_lock(&brmctx->br->multicast_lock);
 	if (br_multicast_ctx_vlan_disabled(brmctx))
-		goto out;
+		return;
 
 	if (query->startup_sent < brmctx->multicast_startup_query_count)
 		query->startup_sent++;
 
 	br_multicast_send_query(brmctx, NULL, query);
-out:
-	spin_unlock(&brmctx->br->multicast_lock);
 }
 
 static void br_ip4_multicast_query_expired(struct timer_list *t)
@@ -3998,8 +4134,11 @@ static void br_ip4_multicast_query_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
 						     ip4_own_query.timer);
 
+	spin_lock(&brmctx->br->multicast_lock);
 	br_multicast_query_expired(brmctx, &brmctx->ip4_own_query,
 				   &brmctx->ip4_querier);
+	br_multicast_update_active(brmctx, NULL);
+	spin_unlock(&brmctx->br->multicast_lock);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -4008,8 +4147,11 @@ static void br_ip6_multicast_query_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
 						     ip6_own_query.timer);
 
+	spin_lock(&brmctx->br->multicast_lock);
 	br_multicast_query_expired(brmctx, &brmctx->ip6_own_query,
 				   &brmctx->ip6_querier);
+	br_multicast_update_active(brmctx, NULL);
+	spin_unlock(&brmctx->br->multicast_lock);
 }
 #endif
 
@@ -4044,11 +4186,13 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	brmctx->multicast_membership_interval = 260 * HZ;
 
 	brmctx->ip4_querier.port_ifidx = 0;
+	brmctx->ip4_active = 0;
 	seqcount_spinlock_init(&brmctx->ip4_querier.seq, &br->multicast_lock);
 	brmctx->multicast_igmp_version = 2;
 #if IS_ENABLED(CONFIG_IPV6)
 	brmctx->multicast_mld_version = 1;
 	brmctx->ip6_querier.port_ifidx = 0;
+	brmctx->ip6_active = 0;
 	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
 #endif
 
@@ -4057,7 +4201,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	timer_setup(&brmctx->ip4_other_query.timer,
 		    br_ip4_multicast_querier_expired, 0);
 	timer_setup(&brmctx->ip4_other_query.delay_timer,
-		    br_multicast_query_delay_expired, 0);
+		    br_ip4_multicast_query_delay_expired, 0);
 	timer_setup(&brmctx->ip4_own_query.timer,
 		    br_ip4_multicast_query_expired, 0);
 #if IS_ENABLED(CONFIG_IPV6)
@@ -4066,7 +4210,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	timer_setup(&brmctx->ip6_other_query.timer,
 		    br_ip6_multicast_querier_expired, 0);
 	timer_setup(&brmctx->ip6_other_query.delay_timer,
-		    br_multicast_query_delay_expired, 0);
+		    br_ip6_multicast_query_delay_expired, 0);
 	timer_setup(&brmctx->ip6_own_query.timer,
 		    br_ip6_multicast_query_expired, 0);
 #endif
@@ -4171,6 +4315,8 @@ static void __br_multicast_open(struct net_bridge_mcast *brmctx)
 #if IS_ENABLED(CONFIG_IPV6)
 	__br_multicast_open_query(brmctx->br, &brmctx->ip6_own_query);
 #endif
+
+	br_multicast_update_active(brmctx, NULL);
 }
 
 void br_multicast_open(struct net_bridge *br)
@@ -4209,6 +4355,10 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
 	timer_delete_sync(&brmctx->ip6_other_query.delay_timer);
 	timer_delete_sync(&brmctx->ip6_own_query.timer);
 #endif
+
+	spin_lock_bh(&brmctx->br->multicast_lock);
+	__br_multicast_update_active(brmctx, true, NULL);
+	spin_unlock_bh(&brmctx->br->multicast_lock);
 }
 
 void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
@@ -4234,10 +4384,13 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
 		vlan->priv_flags ^= BR_VLFLAG_MCAST_ENABLED;
 		spin_unlock_bh(&br->multicast_lock);
 
-		if (on)
+		if (on) {
+			spin_lock_bh(&br->multicast_lock);
 			__br_multicast_open(&vlan->br_mcast_ctx);
-		else
+			spin_unlock_bh(&br->multicast_lock);
+		} else {
 			__br_multicast_stop(&vlan->br_mcast_ctx);
+		}
 	} else {
 		struct net_bridge_mcast *brmctx;
 
@@ -4298,10 +4451,13 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
 	br_opt_toggle(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED, on);
 
 	/* disable/enable non-vlan mcast contexts based on vlan snooping */
-	if (on)
+	if (on) {
 		__br_multicast_stop(&br->multicast_ctx);
-	else
+	} else {
+		spin_lock_bh(&br->multicast_lock);
 		__br_multicast_open(&br->multicast_ctx);
+		spin_unlock_bh(&br->multicast_lock);
+	}
 	list_for_each_entry(p, &br->port_list, list) {
 		if (on)
 			br_multicast_disable_port(p);
@@ -4663,6 +4819,7 @@ int br_multicast_set_querier(struct net_bridge_mcast *brmctx, unsigned long val)
 #endif
 
 unlock:
+	br_multicast_update_active(brmctx, NULL);
 	spin_unlock_bh(&brmctx->br->multicast_lock);
 
 	return 0;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index d5b3c5936a79..3d05895a437f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -158,12 +158,14 @@ struct net_bridge_mcast {
 	struct bridge_mcast_other_query	ip4_other_query;
 	struct bridge_mcast_own_query	ip4_own_query;
 	struct bridge_mcast_querier	ip4_querier;
+	bool				ip4_active;
 #if IS_ENABLED(CONFIG_IPV6)
 	struct hlist_head		ip6_mc_router_list;
 	struct timer_list		ip6_mc_router_timer;
 	struct bridge_mcast_other_query	ip6_other_query;
 	struct bridge_mcast_own_query	ip6_own_query;
 	struct bridge_mcast_querier	ip6_querier;
+	bool				ip6_active;
 #endif /* IS_ENABLED(CONFIG_IPV6) */
 #endif /* CONFIG_BRIDGE_IGMP_SNOOPING */
 };
@@ -1144,37 +1146,16 @@ br_multicast_is_router(struct net_bridge_mcast *brmctx, struct sk_buff *skb)
 }
 
 static inline bool
-__br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
-			      struct bridge_mcast_other_query *querier,
-			      const bool is_ipv6)
-{
-	bool own_querier_enabled;
-
-	if (brmctx->multicast_querier) {
-		if (is_ipv6 && !br_opt_get(brmctx->br, BROPT_HAS_IPV6_ADDR))
-			own_querier_enabled = false;
-		else
-			own_querier_enabled = true;
-	} else {
-		own_querier_enabled = false;
-	}
-
-	return !timer_pending(&querier->delay_timer) &&
-	       (own_querier_enabled || timer_pending(&querier->timer));
-}
-
-static inline bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
-					       struct ethhdr *eth,
-					       const struct net_bridge_mdb_entry *mdb)
+br_multicast_snooping_active(struct net_bridge_mcast *brmctx,
+			     struct ethhdr *eth,
+			     const struct net_bridge_mdb_entry *mdb)
 {
 	switch (eth->h_proto) {
 	case (htons(ETH_P_IP)):
-		return __br_multicast_querier_exists(brmctx,
-			&brmctx->ip4_other_query, false);
+		return brmctx->ip4_active;
 #if IS_ENABLED(CONFIG_IPV6)
 	case (htons(ETH_P_IPV6)):
-		return __br_multicast_querier_exists(brmctx,
-			&brmctx->ip6_other_query, true);
+		return brmctx->ip6_active;
 #endif
 	default:
 		return !!mdb && br_group_is_l2(&mdb->addr);
-- 
2.49.0

[PATCH net-next 2/5] net: bridge: mcast: export ip{4,6}_active state to netlink

[Linus Lüssing]

Export the new ip{4,6}_active variables to netlink, to be able to
check from userspace that they are updated as intended.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 include/uapi/linux/if_bridge.h |  2 ++
 include/uapi/linux/if_link.h   | 12 ++++++++++++
 net/bridge/br_netlink.c        | 10 +++++++++-
 net/bridge/br_vlan_options.c   | 10 +++++++++-
 net/core/rtnetlink.c           |  2 +-
 5 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index a5b743a2f775..fe26646c38c8 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -584,6 +584,8 @@ enum {
 	BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS,
 	BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE,
 	BRIDGE_VLANDB_GOPTS_MSTI,
+	BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4,
+	BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6,
 	__BRIDGE_VLANDB_GOPTS_MAX
 };
 #define BRIDGE_VLANDB_GOPTS_MAX (__BRIDGE_VLANDB_GOPTS_MAX - 1)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 318386cc5b0d..41f6c461ab32 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -742,6 +742,16 @@ enum in6_addr_gen_mode {
  * @IFLA_BR_FDB_MAX_LEARNED
  *   Set the number of max dynamically learned FDB entries for the current
  *   bridge.
+ *
+ * @IFLA_BR_MCAST_ACTIVE_V4
+ *   Bridge IPv4 mcast active state, read only.
+ *
+ *   1 if an IGMP querier is present, 0 otherwise.
+ *
+ * @IFLA_BR_MCAST_ACTIVE_V6
+ *   Bridge IPv4 mcast active state, read only.
+ *
+ *   1 if an MLD querier is present, 0 otherwise.
  */
 enum {
 	IFLA_BR_UNSPEC,
@@ -794,6 +804,8 @@ enum {
 	IFLA_BR_MCAST_QUERIER_STATE,
 	IFLA_BR_FDB_N_LEARNED,
 	IFLA_BR_FDB_MAX_LEARNED,
+	IFLA_BR_MCAST_ACTIVE_V4,
+	IFLA_BR_MCAST_ACTIVE_V6,
 	__IFLA_BR_MAX,
 };
 
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 6e337937d0d7..7829d2842851 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1264,7 +1264,9 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = {
 	[IFLA_BR_VLAN_STATS_ENABLED] = { .type = NLA_U8 },
 	[IFLA_BR_MCAST_STATS_ENABLED] = { .type = NLA_U8 },
 	[IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 },
+	[IFLA_BR_MCAST_ACTIVE_V4] = { .type = NLA_U8 },
 	[IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 },
+	[IFLA_BR_MCAST_ACTIVE_V6] = { .type = NLA_U8 },
 	[IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 },
 	[IFLA_BR_MULTI_BOOLOPT] =
 		NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)),
@@ -1625,7 +1627,9 @@ static size_t br_get_size(const struct net_device *brdev)
 	       nla_total_size_64bit(sizeof(u64)) + /* IFLA_BR_MCAST_QUERY_RESPONSE_INTVL */
 	       nla_total_size_64bit(sizeof(u64)) + /* IFLA_BR_MCAST_STARTUP_QUERY_INTVL */
 	       nla_total_size(sizeof(u8)) +	/* IFLA_BR_MCAST_IGMP_VERSION */
+	       nla_total_size(sizeof(u8)) +     /* IFLA_BR_MCAST_ACTIVE_V4 */
 	       nla_total_size(sizeof(u8)) +	/* IFLA_BR_MCAST_MLD_VERSION */
+	       nla_total_size(sizeof(u8)) +     /* IFLA_BR_MCAST_ACTIVE_V6 */
 	       br_multicast_querier_state_size() + /* IFLA_BR_MCAST_QUERIER_STATE */
 #endif
 #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
@@ -1717,12 +1721,16 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
 			br->multicast_ctx.multicast_startup_query_count) ||
 	    nla_put_u8(skb, IFLA_BR_MCAST_IGMP_VERSION,
 		       br->multicast_ctx.multicast_igmp_version) ||
+	    nla_put_u8(skb, IFLA_BR_MCAST_ACTIVE_V4,
+		       br->multicast_ctx.ip4_active) ||
 	    br_multicast_dump_querier_state(skb, &br->multicast_ctx,
 					    IFLA_BR_MCAST_QUERIER_STATE))
 		return -EMSGSIZE;
 #if IS_ENABLED(CONFIG_IPV6)
 	if (nla_put_u8(skb, IFLA_BR_MCAST_MLD_VERSION,
-		       br->multicast_ctx.multicast_mld_version))
+		       br->multicast_ctx.multicast_mld_version) ||
+	    nla_put_u8(skb, IFLA_BR_MCAST_ACTIVE_V6,
+		       br->multicast_ctx.ip6_active))
 		return -EMSGSIZE;
 #endif
 	clockval = jiffies_to_clock_t(br->multicast_ctx.multicast_last_member_interval);
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 8fa89b04ee94..a97657be51a7 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -369,6 +369,8 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 		       !!(v_opts->priv_flags & BR_VLFLAG_GLOBAL_MCAST_ENABLED)) ||
 	    nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION,
 		       v_opts->br_mcast_ctx.multicast_igmp_version) ||
+	    nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4,
+		       v_opts->br_mcast_ctx.ip4_active) ||
 	    nla_put_u32(skb, BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_CNT,
 			v_opts->br_mcast_ctx.multicast_last_member_count) ||
 	    nla_put_u32(skb, BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_CNT,
@@ -423,7 +425,9 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 
 #if IS_ENABLED(CONFIG_IPV6)
 	if (nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION,
-		       v_opts->br_mcast_ctx.multicast_mld_version))
+		       v_opts->br_mcast_ctx.multicast_mld_version) ||
+	    nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6,
+		       v_opts->br_mcast_ctx.ip4_active))
 		goto out_err;
 #endif
 #endif
@@ -448,7 +452,9 @@ static size_t rtnl_vlan_global_opts_nlmsg_size(const struct net_bridge_vlan *v)
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_SNOOPING */
 		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION */
+		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4 */
 		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION */
+		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6 */
 		+ nla_total_size(sizeof(u32)) /* BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_CNT */
 		+ nla_total_size(sizeof(u32)) /* BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_CNT */
 		+ nla_total_size(sizeof(u64)) /* BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_INTVL */
@@ -630,9 +636,11 @@ static const struct nla_policy br_vlan_db_gpol[BRIDGE_VLANDB_GOPTS_MAX + 1] = {
 	[BRIDGE_VLANDB_GOPTS_RANGE]	= { .type = NLA_U16 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_SNOOPING]	= { .type = NLA_U8 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION]	= { .type = NLA_U8 },
+	[BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6]	= { .type = NLA_U8 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_QUERY_INTVL]	= { .type = NLA_U64 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_QUERIER]	= { .type = NLA_U8 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION]	= { .type = NLA_U8 },
+	[BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4]	= { .type = NLA_U8 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_CNT]	= { .type = NLA_U32 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_CNT]	= { .type = NLA_U32 },
 	[BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_INTVL]	= { .type = NLA_U64 },
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c5a7f41982a5..dee32084be59 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -62,7 +62,7 @@
 
 #include "dev.h"
 
-#define RTNL_MAX_TYPE		50
+#define RTNL_MAX_TYPE		52
 #define RTNL_SLAVE_MAX_TYPE	44
 
 struct rtnl_link {
-- 
2.49.0

[PATCH net-next 3/5] net: bridge: mcast: check if snooping is enabled for active state

[Linus Lüssing]

To be able to use the upcoming SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE
as a potential replacement for SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED
also check and toggle the active state if multicast snooping is enabled
or disabled. So that MC_ACTIVE not only checks the querier state, but
also if multicast snooping is enabled in general.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 include/uapi/linux/if_link.h |  6 ++++--
 net/bridge/br_multicast.c    | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 41f6c461ab32..479d039477cb 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -746,12 +746,14 @@ enum in6_addr_gen_mode {
  * @IFLA_BR_MCAST_ACTIVE_V4
  *   Bridge IPv4 mcast active state, read only.
  *
- *   1 if an IGMP querier is present, 0 otherwise.
+ *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an IGMP querier is present,
+ *   0 otherwise.
  *
  * @IFLA_BR_MCAST_ACTIVE_V6
  *   Bridge IPv4 mcast active state, read only.
  *
- *   1 if an MLD querier is present, 0 otherwise.
+ *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an MLD querier is present,
+ *   0 otherwise.
  */
 enum {
 	IFLA_BR_UNSPEC,
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index b66d2173e321..0bbaa21c1479 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1150,6 +1150,7 @@ static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
  *
  * The multicast active state is set, per protocol family, if:
  *
+ * - multicast snooping is enabled
  * - an IGMP/MLD querier is present
  * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge
  *
@@ -1169,6 +1170,13 @@ static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
 
 	lockdep_assert_held_once(&brmctx->br->multicast_lock);
 
+	if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED))
+		force_inactive = true;
+
+	if (br_opt_get(brmctx->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
+	    br_multicast_ctx_vlan_disabled(brmctx))
+		force_inactive = true;
+
 	ip4_active = !force_inactive;
 	ip6_active = !force_inactive;
 	ip4_changed = br_ip4_multicast_check_active(brmctx, &ip4_active);
@@ -1396,6 +1404,22 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 	return NULL;
 }
 
+static int br_multicast_toggle_enabled(struct net_bridge *br, bool on,
+				       struct netlink_ext_ack *extack)
+{
+	int err, old;
+
+	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on);
+
+	err = br_multicast_update_active(&br->multicast_ctx, extack);
+	if (err && err != -EOPNOTSUPP) {
+		br_opt_toggle(br, BROPT_MULTICAST_ENABLED, old);
+		return err;
+	}
+
+	return 0;
+}
+
 struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br,
 						    struct br_ip *group)
 {
@@ -1409,7 +1433,7 @@ struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br,
 	if (atomic_read(&br->mdb_hash_tbl.nelems) >= br->hash_max) {
 		trace_br_mdb_full(br->dev, group);
 		br_mc_disabled_update(br->dev, false, NULL);
-		br_opt_toggle(br, BROPT_MULTICAST_ENABLED, false);
+		br_multicast_toggle_enabled(br, false, NULL);
 		return ERR_PTR(-E2BIG);
 	}
 
@@ -4382,6 +4406,7 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
 
 		spin_lock_bh(&br->multicast_lock);
 		vlan->priv_flags ^= BR_VLFLAG_MCAST_ENABLED;
+		br_multicast_update_active(&vlan->br_mcast_ctx, NULL);
 		spin_unlock_bh(&br->multicast_lock);
 
 		if (on) {
@@ -4405,6 +4430,7 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
 			__br_multicast_enable_port_ctx(&vlan->port_mcast_ctx);
 		else
 			__br_multicast_disable_port_ctx(&vlan->port_mcast_ctx);
+		br_multicast_update_active(brmctx, NULL);
 		spin_unlock_bh(&br->multicast_lock);
 	}
 }
@@ -4728,7 +4754,12 @@ int br_multicast_toggle(struct net_bridge *br, unsigned long val,
 	if (err)
 		goto unlock;
 
-	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, !!val);
+	err = br_multicast_toggle_enabled(br, !!val, extack);
+	if (err == -EOPNOTSUPP)
+		err = 0;
+	if (err)
+		goto unlock;
+
 	if (!br_opt_get(br, BROPT_MULTICAST_ENABLED)) {
 		change_snoopers = true;
 		goto unlock;
-- 
2.49.0

[PATCH net-next 4/5] net: bridge: switchdev: notify on mcast active changes

[Linus Lüssing]

Let the bridge notify switchdev if the multicast
active state toggles. So that switch drivers can act on it
accordingly, especially to avoid packetloss.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 Documentation/networking/switchdev.rst |  8 +++----
 include/net/switchdev.h                | 10 ++++++++
 net/bridge/br_multicast.c              | 33 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/switchdev.rst b/Documentation/networking/switchdev.rst
index 2966b7122f05..130f7a36fc73 100644
--- a/Documentation/networking/switchdev.rst
+++ b/Documentation/networking/switchdev.rst
@@ -558,7 +558,7 @@ Because IGMP snooping can be turned on/off at runtime, the switchdev driver
 must be able to reconfigure the underlying hardware on the fly to honor the
 toggling of that option and behave appropriately.
 
-A switchdev driver can also refuse to support dynamic toggling of the multicast
-snooping knob at runtime and require the destruction of the bridge device(s)
-and creation of a new bridge device(s) with a different multicast snooping
-value.
+A switchdev driver must also be able to react to vanishing or appearing
+IGMP/MLD queriers. If no querier is present then, even if IGMP/MLD snooping
+is enabled, the switch must treat this as if IGMP/MLD snooping were disabled.
+The SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE notification allows to track this.
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 8346b0d29542..abcc34a81e00 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -27,6 +27,7 @@ enum switchdev_attr_id {
 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL,
 	SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
+	SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE,
 	SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
 	SWITCHDEV_ATTR_ID_BRIDGE_MST,
 	SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
@@ -43,6 +44,14 @@ struct switchdev_brport_flags {
 	unsigned long mask;
 };
 
+struct switchdev_mc_active {
+	short vid;
+	u8 ip4:1,
+	   ip6:1,
+	   ip4_changed:1,
+	   ip6_changed:1;
+};
+
 struct switchdev_vlan_msti {
 	u16 vid;
 	u16 msti;
@@ -64,6 +73,7 @@ struct switchdev_attr {
 		u16 vlan_protocol;			/* BRIDGE_VLAN_PROTOCOL */
 		bool mst;				/* BRIDGE_MST */
 		bool mc_disabled;			/* MC_DISABLED */
+		struct switchdev_mc_active mc_active;	/* MC_ACTIVE */
 		u8 mrp_port_role;			/* MRP_PORT_ROLE */
 		struct switchdev_vlan_msti vlan_msti;	/* VLAN_MSTI */
 	} u;
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 0bbaa21c1479..aec106f9c17d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1138,6 +1138,27 @@ static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
 #endif
 }
 
+static int br_multicast_notify_active(struct net_bridge_mcast *brmctx,
+				      bool ip4_active, bool ip6_active,
+				      bool ip4_changed, bool ip6_changed,
+				      struct netlink_ext_ack *extack)
+{
+	struct switchdev_attr attr = {
+		.orig_dev = brmctx->br->dev,
+		.id = SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE,
+		.flags = SWITCHDEV_F_DEFER,
+		.u.mc_active = {
+			.vid = brmctx->vlan ? brmctx->vlan->vid : -1,
+			.ip4 = ip4_active,
+			.ip6 = ip6_active,
+			.ip4_changed = ip4_changed,
+			.ip6_changed = ip6_changed,
+		},
+	};
+
+	return switchdev_port_attr_set(brmctx->br->dev, &attr, extack);
+}
+
 /**
  * __br_multicast_update_active() - update mcast active state
  * @brmctx: the bridge multicast context to check
@@ -1159,6 +1180,8 @@ static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
  * This function should be called by anything that changes one of the
  * above prerequisites.
  *
+ * Any multicast active state toggling is further notified to switchdev.
+ *
  * Return: 0 on success, a negative value otherwise.
  */
 static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
@@ -1182,11 +1205,21 @@ static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
 	ip4_changed = br_ip4_multicast_check_active(brmctx, &ip4_active);
 	ip6_changed = br_ip6_multicast_check_active(brmctx, &ip6_active);
 
+	if (!ip4_changed && !ip6_changed)
+		goto out;
+
+	ret = br_multicast_notify_active(brmctx, ip4_active, ip6_active,
+					 ip4_changed, ip6_changed,
+					 extack);
+	if (ret && ret != -EOPNOTSUPP)
+		goto out;
+
 	if (ip4_changed)
 		brmctx->ip4_active = ip4_active;
 	if (ip6_changed)
 		brmctx->ip6_active = ip6_active;
 
+out:
 	return ret;
 }
 
-- 
2.49.0

[PATCH net-next 5/5] net: dsa: forward bridge/switchdev mcast active notification

[Linus Lüssing]

Add a new "port_mdb_active()" handler to the DSA API. This allows DSA
drivers to receive the multicast active notification from the
bridge. So that switch drivers can act on it accordingly, especially
to avoid packetloss.

The switchdev notifier "handled" attribute is propagated, too, so that a
DSA based switch driver can decide whether it wants to act on the event
for each port individually or only on the first one, on behalf of all
others.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 Documentation/networking/dsa/dsa.rst |  9 +++++++++
 include/net/dsa.h                    |  5 +++++
 net/dsa/port.c                       | 19 +++++++++++++++++++
 net/dsa/port.h                       |  3 +++
 net/dsa/switch.c                     | 13 +++++++++++++
 net/dsa/switch.h                     | 11 ++++++++++-
 net/dsa/user.c                       | 21 ++++++++++++++++++++-
 7 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/dsa/dsa.rst b/Documentation/networking/dsa/dsa.rst
index 7b2e69cd7ef0..0b0be619be04 100644
--- a/Documentation/networking/dsa/dsa.rst
+++ b/Documentation/networking/dsa/dsa.rst
@@ -1025,6 +1025,15 @@ Bridge VLAN filtering
   the specified MAC address from the specified VLAN ID if it was mapped into
   this port forwarding database.
 
+- ``port_mdb_active``: bridge layer function invoked when the bridge starts (or
+  stops) to actively apply multicast snooping to multicast payload, i.e. when
+  multicast snooping is enabled and a multicast querier is present on the link
+  for a particular protocol family (or not). A switch should (by default) ensure:
+  To flood multicast packets for the given protocol family if multicast snooping
+  is inactive - to avoid multicast (and consequently also IPv6 unicast, which
+  depends on multicast for NDP) packet loss. And should (by default) avoid
+  forwarding to an active port if there is no listener or multicast router on it.
+
 Link aggregation
 ----------------
 
diff --git a/include/net/dsa.h b/include/net/dsa.h
index a0a9481c52c2..edc0e6821ba2 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1080,6 +1080,11 @@ struct dsa_switch_ops {
 	int	(*port_mdb_del)(struct dsa_switch *ds, int port,
 				const struct switchdev_obj_port_mdb *mdb,
 				struct dsa_db db);
+	int	(*port_mdb_active)(struct dsa_switch *ds, int port,
+				   const struct switchdev_mc_active mc_active,
+				   struct netlink_ext_ack *extack,
+				   bool handled);
+
 	/*
 	 * RXNFC
 	 */
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 5c9d1798e830..a1e692d9122e 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -1290,6 +1290,25 @@ int dsa_port_bridge_host_mdb_del(const struct dsa_port *dp,
 	return dsa_port_host_mdb_del(dp, mdb, db);
 }
 
+int dsa_port_bridge_mdb_active(const struct dsa_port *dp,
+			       const struct switchdev_mc_active mc_active,
+			       struct netlink_ext_ack *extack,
+			       bool handled)
+{
+	struct dsa_switch *ds = dp->ds;
+	struct dsa_notifier_mdb_active_info info = {
+		.dp = dp,
+		.mc_active = mc_active,
+		.extack = extack,
+		.handled = handled,
+	};
+
+	if (!ds->ops->port_mdb_active)
+		return -EOPNOTSUPP;
+
+	return dsa_port_notify(dp, DSA_NOTIFIER_MDB_ACTIVE, &info);
+}
+
 int dsa_port_vlan_add(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan,
 		      struct netlink_ext_ack *extack)
diff --git a/net/dsa/port.h b/net/dsa/port.h
index 6bc3291573c0..0e92815e7de2 100644
--- a/net/dsa/port.h
+++ b/net/dsa/port.h
@@ -75,6 +75,9 @@ int dsa_port_bridge_host_mdb_add(const struct dsa_port *dp,
 				 const struct switchdev_obj_port_mdb *mdb);
 int dsa_port_bridge_host_mdb_del(const struct dsa_port *dp,
 				 const struct switchdev_obj_port_mdb *mdb);
+int dsa_port_bridge_mdb_active(const struct dsa_port *dp,
+			       const struct switchdev_mc_active mc_active,
+			       struct netlink_ext_ack *extack, bool handled);
 int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
 			      struct switchdev_brport_flags flags,
 			      struct netlink_ext_ack *extack);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 3d2feeea897b..5b30dfe4bebd 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -652,6 +652,16 @@ static int dsa_switch_host_mdb_del(struct dsa_switch *ds,
 	return err;
 }
 
+static int dsa_switch_mdb_active(struct dsa_switch *ds,
+				 struct dsa_notifier_mdb_active_info *info)
+{
+	if (!ds->ops->port_mdb_active)
+		return -EOPNOTSUPP;
+
+	return ds->ops->port_mdb_active(ds, info->dp->index, info->mc_active,
+					info->extack, info->handled);
+}
+
 /* Port VLANs match on the targeted port and on all DSA ports */
 static bool dsa_port_vlan_match(struct dsa_port *dp,
 				struct dsa_notifier_vlan_info *info)
@@ -1026,6 +1036,9 @@ static int dsa_switch_event(struct notifier_block *nb,
 	case DSA_NOTIFIER_HOST_MDB_DEL:
 		err = dsa_switch_host_mdb_del(ds, info);
 		break;
+	case DSA_NOTIFIER_MDB_ACTIVE:
+		err = dsa_switch_mdb_active(ds, info);
+		break;
 	case DSA_NOTIFIER_VLAN_ADD:
 		err = dsa_switch_vlan_add(ds, info);
 		break;
diff --git a/net/dsa/switch.h b/net/dsa/switch.h
index be0a2749cd97..69a5004e48c8 100644
--- a/net/dsa/switch.h
+++ b/net/dsa/switch.h
@@ -24,6 +24,7 @@ enum {
 	DSA_NOTIFIER_MDB_DEL,
 	DSA_NOTIFIER_HOST_MDB_ADD,
 	DSA_NOTIFIER_HOST_MDB_DEL,
+	DSA_NOTIFIER_MDB_ACTIVE,
 	DSA_NOTIFIER_VLAN_ADD,
 	DSA_NOTIFIER_VLAN_DEL,
 	DSA_NOTIFIER_HOST_VLAN_ADD,
@@ -66,13 +67,21 @@ struct dsa_notifier_lag_fdb_info {
 	struct dsa_db db;
 };
 
-/* DSA_NOTIFIER_MDB_* */
+/* DSA_NOTIFIER_MDB_{ADD,DEL} */
 struct dsa_notifier_mdb_info {
 	const struct dsa_port *dp;
 	const struct switchdev_obj_port_mdb *mdb;
 	struct dsa_db db;
 };
 
+/* DSA_NOTIFIER_MDB_ACTIVE */
+struct dsa_notifier_mdb_active_info {
+	const struct dsa_port *dp;
+	const struct switchdev_mc_active mc_active;
+	struct netlink_ext_ack *extack;
+	int handled;
+};
+
 /* DSA_NOTIFIER_LAG_* */
 struct dsa_notifier_lag_info {
 	const struct dsa_port *dp;
diff --git a/net/dsa/user.c b/net/dsa/user.c
index 804dc7dac4f2..231b92d6e7b9 100644
--- a/net/dsa/user.c
+++ b/net/dsa/user.c
@@ -603,7 +603,7 @@ static int dsa_user_port_attr_set(struct net_device *dev, const void *ctx,
 	struct dsa_port *dp = dsa_user_to_port(dev);
 	int ret;
 
-	if (ctx && ctx != dp)
+	if (ctx && ctx != dp && attr->id != SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE)
 		return 0;
 
 	switch (attr->id) {
@@ -657,6 +657,15 @@ static int dsa_user_port_attr_set(struct net_device *dev, const void *ctx,
 
 		ret = dsa_port_vlan_msti(dp, &attr->u.vlan_msti);
 		break;
+	case SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE:
+		const bool *handled = ctx;
+
+		if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
+			return -EOPNOTSUPP;
+
+		ret = dsa_port_bridge_mdb_active(dp, attr->u.mc_active, extack,
+						 *handled);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
@@ -3758,6 +3767,11 @@ static int dsa_user_switchdev_event(struct notifier_block *unused,
 
 	switch (event) {
 	case SWITCHDEV_PORT_ATTR_SET:
+		struct switchdev_notifier_port_attr_info *item = ptr;
+
+		if (item && item->attr->id == SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE)
+			item->info.ctx = &item->handled;
+
 		err = switchdev_handle_port_attr_set(dev, ptr,
 						     dsa_user_dev_check,
 						     dsa_user_port_attr_set);
@@ -3796,6 +3810,11 @@ static int dsa_user_switchdev_blocking_event(struct notifier_block *unused,
 							    dsa_user_port_obj_del);
 		return notifier_from_errno(err);
 	case SWITCHDEV_PORT_ATTR_SET:
+		struct switchdev_notifier_port_attr_info *item = ptr;
+
+		if (item && item->attr->id == SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE)
+			item->info.ctx = &item->handled;
+
 		err = switchdev_handle_port_attr_set(dev, ptr,
 						     dsa_user_dev_check,
 						     dsa_user_port_attr_set);
-- 
2.49.0

Re: [PATCH net-next 5/5] net: dsa: forward bridge/switchdev mcast active notification

[kernel test robot]

Hi Linus,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]
[also build test WARNING on linus/master v6.15-rc7 next-20250522]
[cannot apply to net-next/main horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Linus-L-ssing/net-bridge-mcast-explicitly-track-active-state/20250523-040914
base:   net/main
patch link:    https://lore.kernel.org/r/20250522195952.29265-6-linus.luessing%40c0d3.blue
patch subject: [PATCH net-next 5/5] net: dsa: forward bridge/switchdev mcast active notification
config: i386-randconfig-001-20250523 (https://download.01.org/0day-ci/archive/20250523/202505231706.fkxIDjje-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250523/202505231706.fkxIDjje-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505231706.fkxIDjje-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> net/dsa/user.c:661:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
     661 |                 const bool *handled = ctx;
         |                 ^
   net/dsa/user.c:3770:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
    3770 |                 struct switchdev_notifier_port_attr_info *item = ptr;
         |                 ^
   net/dsa/user.c:3813:3: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
    3813 |                 struct switchdev_notifier_port_attr_info *item = ptr;
         |                 ^
   3 warnings generated.


vim +661 net/dsa/user.c

   598	
   599	static int dsa_user_port_attr_set(struct net_device *dev, const void *ctx,
   600					  const struct switchdev_attr *attr,
   601					  struct netlink_ext_ack *extack)
   602	{
   603		struct dsa_port *dp = dsa_user_to_port(dev);
   604		int ret;
   605	
   606		if (ctx && ctx != dp && attr->id != SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE)
   607			return 0;
   608	
   609		switch (attr->id) {
   610		case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
   611			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   612				return -EOPNOTSUPP;
   613	
   614			ret = dsa_port_set_state(dp, attr->u.stp_state, true);
   615			break;
   616		case SWITCHDEV_ATTR_ID_PORT_MST_STATE:
   617			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   618				return -EOPNOTSUPP;
   619	
   620			ret = dsa_port_set_mst_state(dp, &attr->u.mst_state, extack);
   621			break;
   622		case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
   623			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   624				return -EOPNOTSUPP;
   625	
   626			ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
   627						      extack);
   628			break;
   629		case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
   630			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   631				return -EOPNOTSUPP;
   632	
   633			ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
   634			break;
   635		case SWITCHDEV_ATTR_ID_BRIDGE_MST:
   636			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   637				return -EOPNOTSUPP;
   638	
   639			ret = dsa_port_mst_enable(dp, attr->u.mst, extack);
   640			break;
   641		case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
   642			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   643				return -EOPNOTSUPP;
   644	
   645			ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags,
   646							extack);
   647			break;
   648		case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
   649			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   650				return -EOPNOTSUPP;
   651	
   652			ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, extack);
   653			break;
   654		case SWITCHDEV_ATTR_ID_VLAN_MSTI:
   655			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   656				return -EOPNOTSUPP;
   657	
   658			ret = dsa_port_vlan_msti(dp, &attr->u.vlan_msti);
   659			break;
   660		case SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE:
 > 661			const bool *handled = ctx;
   662	
   663			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   664				return -EOPNOTSUPP;
   665	
   666			ret = dsa_port_bridge_mdb_active(dp, attr->u.mc_active, extack,
   667							 *handled);
   668			break;
   669		default:
   670			ret = -EOPNOTSUPP;
   671			break;
   672		}
   673	
   674		return ret;
   675	}
   676	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 1/5] net: bridge: mcast: explicitly track active state

[kernel test robot]

Hi Linus,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]
[also build test ERROR on linus/master v6.15-rc7 next-20250523]
[cannot apply to net-next/main horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Linus-L-ssing/net-bridge-mcast-explicitly-track-active-state/20250523-040914
base:   net/main
patch link:    https://lore.kernel.org/r/20250522195952.29265-2-linus.luessing%40c0d3.blue
patch subject: [PATCH net-next 1/5] net: bridge: mcast: explicitly track active state
config: x86_64-buildonly-randconfig-004-20250523 (https://download.01.org/0day-ci/archive/20250523/202505231710.LnCUsJMF-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250523/202505231710.LnCUsJMF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505231710.LnCUsJMF-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/bridge/br_multicast.c:1135:6: error: expected value in expression
    1135 | #elif
         |      ^
>> net/bridge/br_multicast.c:1180:11: error: no member named 'ip6_active' in 'struct net_bridge_mcast'; did you mean 'ip4_active'?
    1180 |                 brmctx->ip6_active = ip6_active;
         |                         ^~~~~~~~~~
         |                         ip4_active
   include/trace/events/../../../net/bridge/br_private.h:161:10: note: 'ip4_active' declared here
     161 |         bool                            ip4_active;
         |                                         ^
   2 errors generated.


vim +1135 net/bridge/br_multicast.c

  1122	
  1123	static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
  1124						 bool *active)
  1125	{
  1126	#if IS_ENABLED(CONFIG_IPV6)
  1127		if (!__br_multicast_querier_exists(brmctx, &brmctx->ip6_other_query,
  1128						   true))
  1129			*active = false;
  1130	
  1131		if (brmctx->ip6_active == *active)
  1132			return false;
  1133	
  1134		return true;
> 1135	#elif
  1136		*active = false;
  1137		return false;
  1138	#endif
  1139	}
  1140	
  1141	/**
  1142	 * __br_multicast_update_active() - update mcast active state
  1143	 * @brmctx: the bridge multicast context to check
  1144	 * @force_inactive: forcefully deactivate mcast active state
  1145	 * @extack: netlink extended ACK structure
  1146	 *
  1147	 * This (potentially) updates the IPv4/IPv6 multicast active state. And by
  1148	 * that enables or disables snooping of multicast payload traffic in fast
  1149	 * path.
  1150	 *
  1151	 * The multicast active state is set, per protocol family, if:
  1152	 *
  1153	 * - an IGMP/MLD querier is present
  1154	 * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge
  1155	 *
  1156	 * And is unset otherwise.
  1157	 *
  1158	 * This function should be called by anything that changes one of the
  1159	 * above prerequisites.
  1160	 *
  1161	 * Return: 0 on success, a negative value otherwise.
  1162	 */
  1163	static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
  1164						bool force_inactive,
  1165						struct netlink_ext_ack *extack)
  1166	{
  1167		bool ip4_active, ip6_active, ip4_changed, ip6_changed;
  1168		int ret = 0;
  1169	
  1170		lockdep_assert_held_once(&brmctx->br->multicast_lock);
  1171	
  1172		ip4_active = !force_inactive;
  1173		ip6_active = !force_inactive;
  1174		ip4_changed = br_ip4_multicast_check_active(brmctx, &ip4_active);
  1175		ip6_changed = br_ip6_multicast_check_active(brmctx, &ip6_active);
  1176	
  1177		if (ip4_changed)
  1178			brmctx->ip4_active = ip4_active;
  1179		if (ip6_changed)
> 1180			brmctx->ip6_active = ip6_active;
  1181	
  1182		return ret;
  1183	}
  1184	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 5/5] net: dsa: forward bridge/switchdev mcast active notification

[kernel test robot]

Hi Linus,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]
[also build test ERROR on linus/master v6.15-rc7 next-20250523]
[cannot apply to net-next/main horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Linus-L-ssing/net-bridge-mcast-explicitly-track-active-state/20250523-040914
base:   net/main
patch link:    https://lore.kernel.org/r/20250522195952.29265-6-linus.luessing%40c0d3.blue
patch subject: [PATCH net-next 5/5] net: dsa: forward bridge/switchdev mcast active notification
config: s390-randconfig-001-20250523 (https://download.01.org/0day-ci/archive/20250523/202505231753.V7E84RiN-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250523/202505231753.V7E84RiN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505231753.V7E84RiN-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/dsa/user.c: In function 'dsa_user_port_attr_set':
>> net/dsa/user.c:661:3: error: a label can only be part of a statement and a declaration is not a statement
      const bool *handled = ctx;
      ^~~~~
   net/dsa/user.c: In function 'dsa_user_switchdev_event':
   net/dsa/user.c:3770:3: error: a label can only be part of a statement and a declaration is not a statement
      struct switchdev_notifier_port_attr_info *item = ptr;
      ^~~~~~
   net/dsa/user.c: In function 'dsa_user_switchdev_blocking_event':
   net/dsa/user.c:3813:3: error: a label can only be part of a statement and a declaration is not a statement
      struct switchdev_notifier_port_attr_info *item = ptr;
      ^~~~~~


vim +661 net/dsa/user.c

   598	
   599	static int dsa_user_port_attr_set(struct net_device *dev, const void *ctx,
   600					  const struct switchdev_attr *attr,
   601					  struct netlink_ext_ack *extack)
   602	{
   603		struct dsa_port *dp = dsa_user_to_port(dev);
   604		int ret;
   605	
   606		if (ctx && ctx != dp && attr->id != SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE)
   607			return 0;
   608	
   609		switch (attr->id) {
   610		case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
   611			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   612				return -EOPNOTSUPP;
   613	
   614			ret = dsa_port_set_state(dp, attr->u.stp_state, true);
   615			break;
   616		case SWITCHDEV_ATTR_ID_PORT_MST_STATE:
   617			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   618				return -EOPNOTSUPP;
   619	
   620			ret = dsa_port_set_mst_state(dp, &attr->u.mst_state, extack);
   621			break;
   622		case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
   623			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   624				return -EOPNOTSUPP;
   625	
   626			ret = dsa_port_vlan_filtering(dp, attr->u.vlan_filtering,
   627						      extack);
   628			break;
   629		case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
   630			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   631				return -EOPNOTSUPP;
   632	
   633			ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
   634			break;
   635		case SWITCHDEV_ATTR_ID_BRIDGE_MST:
   636			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   637				return -EOPNOTSUPP;
   638	
   639			ret = dsa_port_mst_enable(dp, attr->u.mst, extack);
   640			break;
   641		case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
   642			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   643				return -EOPNOTSUPP;
   644	
   645			ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags,
   646							extack);
   647			break;
   648		case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
   649			if (!dsa_port_offloads_bridge_port(dp, attr->orig_dev))
   650				return -EOPNOTSUPP;
   651	
   652			ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, extack);
   653			break;
   654		case SWITCHDEV_ATTR_ID_VLAN_MSTI:
   655			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   656				return -EOPNOTSUPP;
   657	
   658			ret = dsa_port_vlan_msti(dp, &attr->u.vlan_msti);
   659			break;
   660		case SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE:
 > 661			const bool *handled = ctx;
   662	
   663			if (!dsa_port_offloads_bridge_dev(dp, attr->orig_dev))
   664				return -EOPNOTSUPP;
   665	
   666			ret = dsa_port_bridge_mdb_active(dp, attr->u.mc_active, extack,
   667							 *handled);
   668			break;
   669		default:
   670			ret = -EOPNOTSUPP;
   671			break;
   672		}
   673	
   674		return ret;
   675	}
   676	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 3/5] net: bridge: mcast: check if snooping is enabled for active state

[Simon Horman]

On Thu, May 22, 2025 at 09:17:05PM +0200, Linus Lüssing wrote:
> To be able to use the upcoming SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE
> as a potential replacement for SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED
> also check and toggle the active state if multicast snooping is enabled
> or disabled. So that MC_ACTIVE not only checks the querier state, but
> also if multicast snooping is enabled in general.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  include/uapi/linux/if_link.h |  6 ++++--
>  net/bridge/br_multicast.c    | 35 +++++++++++++++++++++++++++++++++--
>  2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 41f6c461ab32..479d039477cb 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -746,12 +746,14 @@ enum in6_addr_gen_mode {
>   * @IFLA_BR_MCAST_ACTIVE_V4
>   *   Bridge IPv4 mcast active state, read only.
>   *
> - *   1 if an IGMP querier is present, 0 otherwise.
> + *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an IGMP querier is present,
> + *   0 otherwise.
>   *
>   * @IFLA_BR_MCAST_ACTIVE_V6
>   *   Bridge IPv4 mcast active state, read only.
>   *
> - *   1 if an MLD querier is present, 0 otherwise.
> + *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an MLD querier is present,
> + *   0 otherwise.
>   */
>  enum {
>  	IFLA_BR_UNSPEC,
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index b66d2173e321..0bbaa21c1479 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1150,6 +1150,7 @@ static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
>   *
>   * The multicast active state is set, per protocol family, if:
>   *
> + * - multicast snooping is enabled
>   * - an IGMP/MLD querier is present
>   * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge
>   *
> @@ -1169,6 +1170,13 @@ static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
>  
>  	lockdep_assert_held_once(&brmctx->br->multicast_lock);
>  
> +	if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED))
> +		force_inactive = true;
> +
> +	if (br_opt_get(brmctx->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
> +	    br_multicast_ctx_vlan_disabled(brmctx))
> +		force_inactive = true;
> +
>  	ip4_active = !force_inactive;
>  	ip6_active = !force_inactive;
>  	ip4_changed = br_ip4_multicast_check_active(brmctx, &ip4_active);
> @@ -1396,6 +1404,22 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
>  	return NULL;
>  }
>  
> +static int br_multicast_toggle_enabled(struct net_bridge *br, bool on,
> +				       struct netlink_ext_ack *extack)
> +{
> +	int err, old;
> +
> +	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on);
> +
> +	err = br_multicast_update_active(&br->multicast_ctx, extack);
> +	if (err && err != -EOPNOTSUPP) {
> +		br_opt_toggle(br, BROPT_MULTICAST_ENABLED, old);

Hi Linus,

Old appears to be used uninitialised here.

Flagged by allmodconfig builds on x86_64 with clang-20.1.4.

...

Re: [PATCH net-next 1/5] net: bridge: mcast: explicitly track active state

[Ido Schimmel]

On Thu, May 22, 2025 at 09:17:03PM +0200, Linus Lüssing wrote:
> Combining and tracking the active state into new, per protocol family
> variables.
> 
> For one thing this slightly reduces the work and checks in fast path
> for multicast payload traffic. For another this is in preparation to
> be able to notify DSA/switchdev on the (in)applicability of multicast
> snooping on multicast payload traffic. Especially on vanishing IGMP/MLD
> queriers.

Adding more code in the control path in order to simplify the data path
makes sense, but IMO this patch is difficult to review and should be
split into smaller patches. At the very least the patch can be split
into a control path patch and a data path patch. The latter simplifies
the data path by making use of the new multicast states maintained by
the control path.

The control path patch can be split into smaller patches where each
patch updates the multicast states from the different places that affect
them:

1. When the delay timer expires.
2. When we stop getting queries.
3. When the bridge gains or losses an IPv6 address.
4. When "mcast_querier" is changed (globally or per-VLAN).
5. When "mcast_snooping" is changed (globally or per-VLAN).

Given the number of patches, consider splitting the offload changes into
a separate patchset. It would probably be merged faster that way.

See more comments below.

> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/bridge/br_device.c    |   5 +-
>  net/bridge/br_input.c     |   2 +-
>  net/bridge/br_multicast.c | 179 +++++++++++++++++++++++++++++++++++---
>  net/bridge/br_private.h   |  33 ++-----
>  4 files changed, 180 insertions(+), 39 deletions(-)
> 
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index a818fdc22da9..315ed3d33406 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -102,7 +102,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  		mdst = br_mdb_entry_skb_get(brmctx, skb, vid);
>  		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
> -		    br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst))
> +		    br_multicast_snooping_active(brmctx, eth_hdr(skb), mdst))
>  			br_multicast_flood(mdst, skb, brmctx, false, true);
>  		else
>  			br_flood(br, skb, BR_PKT_MULTICAST, false, true, vid);
> @@ -168,7 +168,10 @@ static int br_dev_open(struct net_device *dev)
>  	netdev_update_features(dev);
>  	netif_start_queue(dev);
>  	br_stp_enable_bridge(br);
> +
> +	spin_lock_bh(&br->multicast_lock);
>  	br_multicast_open(br);
> +	spin_unlock_bh(&br->multicast_lock);
>  
>  	if (br_opt_get(br, BROPT_MULTICAST_ENABLED))
>  		br_multicast_join_snoopers(br);
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 232133a0fd21..0c632655d66c 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -187,7 +187,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  	case BR_PKT_MULTICAST:
>  		mdst = br_mdb_entry_skb_get(brmctx, skb, vid);
>  		if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
> -		    br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst)) {
> +		    br_multicast_snooping_active(brmctx, eth_hdr(skb), mdst)) {
>  			if ((mdst && mdst->host_joined) ||
>  			    br_multicast_is_router(brmctx, skb)) {
>  				local_rcv = true;
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index dcbf058de1e3..b66d2173e321 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1069,6 +1069,125 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm
>  	return skb;
>  }
>  
> +static bool
> +__br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
> +			      struct bridge_mcast_other_query *querier,
> +			      const bool is_ipv6)

Remove the const?

> +{
> +	bool own_querier_enabled;
> +
> +	if (brmctx->multicast_querier) {
> +		if (is_ipv6 && !br_opt_get(brmctx->br, BROPT_HAS_IPV6_ADDR))
> +			own_querier_enabled = false;
> +		else
> +			own_querier_enabled = true;
> +	} else {
> +		own_querier_enabled = false;
> +	}
> +
> +	return !timer_pending(&querier->delay_timer) &&
> +	       (own_querier_enabled || timer_pending(&querier->timer));
> +}
> +
> +static bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
> +					struct ethhdr *eth,

Make this const or just pass the EtherType?

> +					const struct net_bridge_mdb_entry *mdb)
> +{
> +	switch (eth->h_proto) {
> +	case (htons(ETH_P_IP)):
> +		return __br_multicast_querier_exists(brmctx,
> +			&brmctx->ip4_other_query, false);
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case (htons(ETH_P_IPV6)):
> +		return __br_multicast_querier_exists(brmctx,
> +			&brmctx->ip6_other_query, true);
> +#endif
> +	default:
> +		return !!mdb && br_group_is_l2(&mdb->addr);
> +	}
> +}
> +
> +static bool br_ip4_multicast_check_active(struct net_bridge_mcast *brmctx,
> +					  bool *active)
> +{
> +	if (!__br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query,
> +					   false))
> +		*active = false;
> +
> +	if (brmctx->ip4_active == *active)
> +		return false;
> +
> +	return true;
> +}
> +
> +static int br_ip6_multicast_check_active(struct net_bridge_mcast *brmctx,
> +					 bool *active)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (!__br_multicast_querier_exists(brmctx, &brmctx->ip6_other_query,
> +					   true))
> +		*active = false;
> +
> +	if (brmctx->ip6_active == *active)
> +		return false;
> +
> +	return true;
> +#elif
> +	*active = false;
> +	return false;
> +#endif
> +}
> +
> +/**
> + * __br_multicast_update_active() - update mcast active state
> + * @brmctx: the bridge multicast context to check
> + * @force_inactive: forcefully deactivate mcast active state
> + * @extack: netlink extended ACK structure
> + *
> + * This (potentially) updates the IPv4/IPv6 multicast active state. And by
> + * that enables or disables snooping of multicast payload traffic in fast
> + * path.
> + *
> + * The multicast active state is set, per protocol family, if:
> + *
> + * - an IGMP/MLD querier is present
> + * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge
> + *
> + * And is unset otherwise.
> + *
> + * This function should be called by anything that changes one of the
> + * above prerequisites.
> + *
> + * Return: 0 on success, a negative value otherwise.
> + */
> +static int __br_multicast_update_active(struct net_bridge_mcast *brmctx,
> +					bool force_inactive,
> +					struct netlink_ext_ack *extack)
> +{
> +	bool ip4_active, ip6_active, ip4_changed, ip6_changed;
> +	int ret = 0;
> +
> +	lockdep_assert_held_once(&brmctx->br->multicast_lock);
> +
> +	ip4_active = !force_inactive;
> +	ip6_active = !force_inactive;
> +	ip4_changed = br_ip4_multicast_check_active(brmctx, &ip4_active);
> +	ip6_changed = br_ip6_multicast_check_active(brmctx, &ip6_active);

ip{4,6}_changed aren't really used in this patch. I suggest adding them
when you need them. In addition, br_ip{4,6}_multicast_check_active() are
quite confusing to me. They return a bool, but also modify a bool
argument. It would be clearer to derive the existing states from
brmctx->ip{4,6}_active and then derive the new states from something
like br_ip{4,6}_multicast_can_activate(brmctx)

> +
> +	if (ip4_changed)
> +		brmctx->ip4_active = ip4_active;
> +	if (ip6_changed)
> +		brmctx->ip6_active = ip6_active;
> +
> +	return ret;
> +}
> +
> +static int br_multicast_update_active(struct net_bridge_mcast *brmctx,
> +				      struct netlink_ext_ack *extack)
> +{
> +	return __br_multicast_update_active(brmctx, false, extack);
> +}
> +
>  #if IS_ENABLED(CONFIG_IPV6)
>  static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brmctx,
>  						    struct net_bridge_mcast_port *pmctx,
> @@ -1147,10 +1266,12 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brm
>  			       &ip6h->daddr, 0, &ip6h->saddr)) {
>  		kfree_skb(skb);
>  		br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, false);
> +		br_multicast_update_active(brmctx, NULL);
>  		return NULL;
>  	}
>  
>  	br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, true);
> +	br_multicast_update_active(brmctx, NULL);
>  	ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
>  
>  	hopopt = (u8 *)(ip6h + 1);
> @@ -1762,10 +1883,28 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t)
>  }
>  #endif
>  
> -static void br_multicast_query_delay_expired(struct timer_list *t)
> +static void br_ip4_multicast_query_delay_expired(struct timer_list *t)
>  {
> +	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
> +						     ip4_other_query.delay_timer);
> +
> +	spin_lock(&brmctx->br->multicast_lock);
> +	br_multicast_update_active(brmctx, NULL);
> +	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  
> +#if IS_ENABLED(CONFIG_IPV6)
> +static void br_ip6_multicast_query_delay_expired(struct timer_list *t)
> +{
> +	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
> +						     ip6_other_query.delay_timer);
> +
> +	spin_lock(&brmctx->br->multicast_lock);
> +	br_multicast_update_active(brmctx, NULL);
> +	spin_unlock(&brmctx->br->multicast_lock);
> +}
> +#endif
> +
>  static void br_multicast_select_own_querier(struct net_bridge_mcast *brmctx,
>  					    struct br_ip *ip,
>  					    struct sk_buff *skb)
> @@ -3981,16 +4120,13 @@ static void br_multicast_query_expired(struct net_bridge_mcast *brmctx,
>  				       struct bridge_mcast_own_query *query,
>  				       struct bridge_mcast_querier *querier)
>  {
> -	spin_lock(&brmctx->br->multicast_lock);
>  	if (br_multicast_ctx_vlan_disabled(brmctx))
> -		goto out;
> +		return;
>  
>  	if (query->startup_sent < brmctx->multicast_startup_query_count)
>  		query->startup_sent++;
>  
>  	br_multicast_send_query(brmctx, NULL, query);
> -out:
> -	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  
>  static void br_ip4_multicast_query_expired(struct timer_list *t)
> @@ -3998,8 +4134,11 @@ static void br_ip4_multicast_query_expired(struct timer_list *t)
>  	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
>  						     ip4_own_query.timer);
>  
> +	spin_lock(&brmctx->br->multicast_lock);
>  	br_multicast_query_expired(brmctx, &brmctx->ip4_own_query,
>  				   &brmctx->ip4_querier);
> +	br_multicast_update_active(brmctx, NULL);
> +	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -4008,8 +4147,11 @@ static void br_ip6_multicast_query_expired(struct timer_list *t)
>  	struct net_bridge_mcast *brmctx = from_timer(brmctx, t,
>  						     ip6_own_query.timer);
>  
> +	spin_lock(&brmctx->br->multicast_lock);
>  	br_multicast_query_expired(brmctx, &brmctx->ip6_own_query,
>  				   &brmctx->ip6_querier);
> +	br_multicast_update_active(brmctx, NULL);

It is not clear to me why the new states are updated from
br_ip{4,6}_multicast_query_expired(). These functions are called when
the bridge is the querier and it is time to send a new query.

Did you mean to place these in br_ip{4,6}_multicast_querier_expired()
which are invoked when the other querier expired?

> +	spin_unlock(&brmctx->br->multicast_lock);
>  }
>  #endif
>  
> @@ -4044,11 +4186,13 @@ void br_multicast_ctx_init(struct net_bridge *br,
>  	brmctx->multicast_membership_interval = 260 * HZ;
>  
>  	brmctx->ip4_querier.port_ifidx = 0;
> +	brmctx->ip4_active = 0;
>  	seqcount_spinlock_init(&brmctx->ip4_querier.seq, &br->multicast_lock);
>  	brmctx->multicast_igmp_version = 2;
>  #if IS_ENABLED(CONFIG_IPV6)
>  	brmctx->multicast_mld_version = 1;
>  	brmctx->ip6_querier.port_ifidx = 0;
> +	brmctx->ip6_active = 0;
>  	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
>  #endif
>  
> @@ -4057,7 +4201,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
>  	timer_setup(&brmctx->ip4_other_query.timer,
>  		    br_ip4_multicast_querier_expired, 0);
>  	timer_setup(&brmctx->ip4_other_query.delay_timer,
> -		    br_multicast_query_delay_expired, 0);
> +		    br_ip4_multicast_query_delay_expired, 0);
>  	timer_setup(&brmctx->ip4_own_query.timer,
>  		    br_ip4_multicast_query_expired, 0);
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -4066,7 +4210,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
>  	timer_setup(&brmctx->ip6_other_query.timer,
>  		    br_ip6_multicast_querier_expired, 0);
>  	timer_setup(&brmctx->ip6_other_query.delay_timer,
> -		    br_multicast_query_delay_expired, 0);
> +		    br_ip6_multicast_query_delay_expired, 0);
>  	timer_setup(&brmctx->ip6_own_query.timer,
>  		    br_ip6_multicast_query_expired, 0);
>  #endif
> @@ -4171,6 +4315,8 @@ static void __br_multicast_open(struct net_bridge_mcast *brmctx)
>  #if IS_ENABLED(CONFIG_IPV6)
>  	__br_multicast_open_query(brmctx->br, &brmctx->ip6_own_query);
>  #endif
> +
> +	br_multicast_update_active(brmctx, NULL);
>  }
>  
>  void br_multicast_open(struct net_bridge *br)
> @@ -4209,6 +4355,10 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
>  	timer_delete_sync(&brmctx->ip6_other_query.delay_timer);
>  	timer_delete_sync(&brmctx->ip6_own_query.timer);
>  #endif
> +
> +	spin_lock_bh(&brmctx->br->multicast_lock);
> +	__br_multicast_update_active(brmctx, true, NULL);
> +	spin_unlock_bh(&brmctx->br->multicast_lock);
>  }
>  
>  void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
> @@ -4234,10 +4384,13 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
>  		vlan->priv_flags ^= BR_VLFLAG_MCAST_ENABLED;
>  		spin_unlock_bh(&br->multicast_lock);
>  
> -		if (on)
> +		if (on) {
> +			spin_lock_bh(&br->multicast_lock);
>  			__br_multicast_open(&vlan->br_mcast_ctx);
> -		else
> +			spin_unlock_bh(&br->multicast_lock);
> +		} else {
>  			__br_multicast_stop(&vlan->br_mcast_ctx);
> +		}
>  	} else {
>  		struct net_bridge_mcast *brmctx;
>  
> @@ -4298,10 +4451,13 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
>  	br_opt_toggle(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED, on);
>  
>  	/* disable/enable non-vlan mcast contexts based on vlan snooping */
> -	if (on)
> +	if (on) {
>  		__br_multicast_stop(&br->multicast_ctx);
> -	else
> +	} else {
> +		spin_lock_bh(&br->multicast_lock);
>  		__br_multicast_open(&br->multicast_ctx);
> +		spin_unlock_bh(&br->multicast_lock);
> +	}
>  	list_for_each_entry(p, &br->port_list, list) {
>  		if (on)
>  			br_multicast_disable_port(p);
> @@ -4663,6 +4819,7 @@ int br_multicast_set_querier(struct net_bridge_mcast *brmctx, unsigned long val)
>  #endif
>  
>  unlock:
> +	br_multicast_update_active(brmctx, NULL);
>  	spin_unlock_bh(&brmctx->br->multicast_lock);
>  
>  	return 0;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index d5b3c5936a79..3d05895a437f 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -158,12 +158,14 @@ struct net_bridge_mcast {
>  	struct bridge_mcast_other_query	ip4_other_query;
>  	struct bridge_mcast_own_query	ip4_own_query;
>  	struct bridge_mcast_querier	ip4_querier;
> +	bool				ip4_active;
>  #if IS_ENABLED(CONFIG_IPV6)
>  	struct hlist_head		ip6_mc_router_list;
>  	struct timer_list		ip6_mc_router_timer;
>  	struct bridge_mcast_other_query	ip6_other_query;
>  	struct bridge_mcast_own_query	ip6_own_query;
>  	struct bridge_mcast_querier	ip6_querier;
> +	bool				ip6_active;
>  #endif /* IS_ENABLED(CONFIG_IPV6) */
>  #endif /* CONFIG_BRIDGE_IGMP_SNOOPING */
>  };
> @@ -1144,37 +1146,16 @@ br_multicast_is_router(struct net_bridge_mcast *brmctx, struct sk_buff *skb)
>  }
>  
>  static inline bool
> -__br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
> -			      struct bridge_mcast_other_query *querier,
> -			      const bool is_ipv6)
> -{
> -	bool own_querier_enabled;
> -
> -	if (brmctx->multicast_querier) {
> -		if (is_ipv6 && !br_opt_get(brmctx->br, BROPT_HAS_IPV6_ADDR))
> -			own_querier_enabled = false;
> -		else
> -			own_querier_enabled = true;
> -	} else {
> -		own_querier_enabled = false;
> -	}
> -
> -	return !timer_pending(&querier->delay_timer) &&
> -	       (own_querier_enabled || timer_pending(&querier->timer));
> -}
> -
> -static inline bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
> -					       struct ethhdr *eth,
> -					       const struct net_bridge_mdb_entry *mdb)
> +br_multicast_snooping_active(struct net_bridge_mcast *brmctx,
> +			     struct ethhdr *eth,
> +			     const struct net_bridge_mdb_entry *mdb)
>  {
>  	switch (eth->h_proto) {
>  	case (htons(ETH_P_IP)):
> -		return __br_multicast_querier_exists(brmctx,
> -			&brmctx->ip4_other_query, false);
> +		return brmctx->ip4_active;
>  #if IS_ENABLED(CONFIG_IPV6)
>  	case (htons(ETH_P_IPV6)):
> -		return __br_multicast_querier_exists(brmctx,
> -			&brmctx->ip6_other_query, true);
> +		return brmctx->ip6_active;
>  #endif
>  	default:
>  		return !!mdb && br_group_is_l2(&mdb->addr);
> -- 
> 2.49.0
> 

Re: [PATCH net-next 2/5] net: bridge: mcast: export ip{4,6}_active state to netlink

[Ido Schimmel]

On Thu, May 22, 2025 at 09:17:04PM +0200, Linus Lüssing wrote:
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 318386cc5b0d..41f6c461ab32 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -742,6 +742,16 @@ enum in6_addr_gen_mode {
>   * @IFLA_BR_FDB_MAX_LEARNED
>   *   Set the number of max dynamically learned FDB entries for the current
>   *   bridge.
> + *
> + * @IFLA_BR_MCAST_ACTIVE_V4
> + *   Bridge IPv4 mcast active state, read only.
> + *
> + *   1 if an IGMP querier is present, 0 otherwise.
> + *
> + * @IFLA_BR_MCAST_ACTIVE_V6
> + *   Bridge IPv4 mcast active state, read only.
> + *
> + *   1 if an MLD querier is present, 0 otherwise.
>   */

[...]

> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 6e337937d0d7..7829d2842851 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1264,7 +1264,9 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = {
>  	[IFLA_BR_VLAN_STATS_ENABLED] = { .type = NLA_U8 },
>  	[IFLA_BR_MCAST_STATS_ENABLED] = { .type = NLA_U8 },
>  	[IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 },
> +	[IFLA_BR_MCAST_ACTIVE_V4] = { .type = NLA_U8 },
>  	[IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 },
> +	[IFLA_BR_MCAST_ACTIVE_V6] = { .type = NLA_U8 },

The new attributes should be set to 'NLA_REJECT' if they are meant to be
read only. They can also be removed from the policy, but being explicit
and using 'NLA_REJECT' is better IMO.