diff mbox series

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

Message ID 20250522195952.29265-4-linus.luessing@c0d3.blue
State New
Headers show
Series net: bridge: propagate safe mcast snooping to switchdev + DSA | expand

Commit Message

Linus Lüssing May 22, 2025, 7:17 p.m. UTC 
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(-)

Comments

Simon Horman May 23, 2025, 1:02 p.m. UTC | #1 
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
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.

...
diff mbox series

Patch

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;