[PATCH net-next v4 00/14] net: bridge: reduce multicast checks in fast path

[PATCH net-next v4 00/14] net: bridge: reduce multicast checks in fast path

[Linus Lüssing]

This patchset introduces new state variables to combine and reduce the
number of checks we would otherwise perform on every multicast packet
in fast/data path. Instead of checking if our querier is enabled or
if another querier timer is pending, plus if the initial query
grace period has elapsed, plus for IPv6 and for our own querier if we
have an IPv6 address yet - and all this for every multicast packet -
we can now simply check one boolean state variable per protocol family
combining all this, the new ip{4,6}_active. Or MCAST_ACTIVE_V4 /
MCAST_ACTIVE_V6 to netlink/userspace.
  
The second reason for introducing these new, internal multicast active
variables is to later propagate a safety mechanism which was introduced
in commit b00589af3b04 ("bridge: disable snooping if there is no querier")
to switchdev/DSA, too. That is to notify switchdev/DSA if multicast
snooping can safely be applied without potential packet loss.

An example usage/integration of this with the modified Realtek rtl83xx
switch driver can be found in a draft pull-request at the OpenWrt
project: https://github.com/openwrt/openwrt/pull/18780
This was tested at least on an ZyXEL GS1900-24HP v1 switch.

iproute2 patch:
https://patchwork.kernel.org/project/netdevbpf/patch/20260206023704.4839-1-linus.luessing@c0d3.blue/

Regards, Linus

---

# Changelog v4

* rebased to current net-next/main branch
* ensure line lengths of 80 characters in various patches
* [PATCH net-next v3 01/14] net: bridge: mcast: export ip{4,6}_active state to netlink
  * fix intermittent "unused variable" warnings when building without IPv6 or
    without IGMP_SNOOPING support by using "__maybe_unused" annotations
* [PATCH net-next v3 02/14] net: bridge: mcast: track active state, adding tests
  * adding petrm@nvidia.com to CC
  * disabling shellcheck SC2317, as it seems to be unable to understand traps
    (https://github.com/koalaman/shellcheck/issues/2542)
    and the tests_run() function correctly
  * changing order of bridge_mdb_active.sh invocation in Makefile to keep an
    alphabetic order to make check_selftest happy
  * fixing line number passing in test_inactive(),
    `test_inactive_nolog "$1"` -> `test_inactive_nolog "$LINENO"`
  * adding `local lineno="$1"` to test_inactive_nolog() for better readability
  * reduce line lengths to 80 in bridge_mdb_active.sh to make
    netdev's checkpatch happy
* [PATCH net-next v3 03/14] net: bridge: mcast: avoid sleeping on bridge-down
  * adding const's to br_multicast_is_stopping()'s parameters
  * removing extra newline in __vlan_add()
  * fixing compilation without IGMP_SNOOPING by moving the
    spinlocking from __vlan_add() into br_multicast_ctx_init()
* [PATCH net-next v3 04/14] net: bridge: mcast: track active state, IGMP/MLD querier appearance
  * fixing compilation without IPv6 support by always including
    brmctx->ip6_active (but keeping it set to false without IPv6 support)
* [PATCH net-next v3 13/14] net: bridge: mcast: use combined active state in fast/data path
  * passing brmctx as const in br_multicast_snooping_active()
  * removing unnecessary brackets in switch cases in
    br_multicast_querier_exists()


# Changelog v3

* rebased to current net-next/main branch
* [PATCH net-next v3 01/14] net: bridge: mcast: export ip{4,6}_active state to netlink 
  * no changes, adding "Reviewed-by: Ido Schimmel <idosch@nvidia.com>"
* [PATCH net-next v3 02/14] net: bridge: mcast: track active state, adding tests 
  * fixing a race condition in the test_vlan_inactive_brdown()
    test, which would occasionally trigger and result in false
    fails, by adding a small delay, a call to test_active_setup_wait(),
    before checking the current state
  * increasing NUM_NETIFS from 1 to 2, to better reflect both sides
    of a veth pair (most places seem to use a NUM_NETIFS=2 for one
    veth pair, even though NUM_NETIFS leads to correct test results, too)
  * adding address family to error messages
  * adding line number to error messages
  * unifying common/similar code of mcast_vlan_active_check()
    and mcast_active_check() in a new mcast_active_check_ret()
  * adding bridge_mdb_active.sh to TEST_PROGS in
    tools/testing/selftests/net/forwarding/Makefile
  * avoiding ${} around variable names in bash math contexts
  * adding double quotes in several places around variables when fetching
    their values to avoid globbing and word splitting
  * adding "# skipping:" to purposefully skipped function calls in
    tests for inactive multicast state to better highlight their
    intentional difference to their fully active counterpart
  * skip the newly added bridge_mdb_active.sh test with a warning
    if iproute2 is too old
  * reducing one occurence of a double newline to a single one
    in bridge_mdb_active.sh
  * adding linux-kselftest@vger.kernel.org shuah@kernel.org to CC
  * reducing column sizes to less than or equal to 80 in a various places
* [PATCH net-next v3 03/14] net: bridge: mcast: avoid sleeping on bridge-down
  * moving multicast spinlock grabs for br_multicast_open() and
    br_multicast_stop() into these functions and
    turning br_multicast_open() and br_multicast_stop() into small
    wrapers to grab the multicast spinlock and then calling
    the otherwise not self-locking br_multicast_open_locked() /
    br_multicast_stop_locked()
    * fixes a build error with CONFIG_BRIDGE_IGMP_SNOOPING unset
  * renaming br_multicast_stopping() to br_multicast_is_stopping()
  * reworded commit message to try to clarify how the new
    timer_shutdown() use is supposed to work
* [PATCH net-next v3 04/14] net: bridge: mcast: track active state, IGMP/MLD querier appearance 
  * turn the br_info() call in br_multicast_notify_active()
    into a br_debug() to reduce potential log clutter
  * renaming br_multicast_stopping() to br_multicast_is_stopping()
* [PATCH net-next v3 05/14] net: bridge: mcast: track active state, foreign IGMP/MLD querier disappearance
  * no changes, adding "Reviewed-by: Ido Schimmel <idosch@nvidia.com>"
* [PATCH net-next v3 06/14] net: bridge: mcast: track active state, IPv6 address availability
  * no changes, adding "Reviewed-by: Ido Schimmel <idosch@nvidia.com>"
* [PATCH net-next v3 07/14] net: bridge: mcast: track active state, own MLD querier disappearance 
  * dropping the not quite fitting final statement in the commit message
    about this being the last necessary check
  * adding "Reviewed-by: Ido Schimmel <idosch@nvidia.com>"
* [PATCH net-next v3 08/14] net: bridge: mcast: track active state, if snooping is enabled
  * removing update on enabling/disabling of VLAN aware
    multicast snooping, adding it the VLAN specific patch instead
* [PATCH net-next v3 09/14] net: bridge: mcast: track active state, VLAN snooping
  * readding update on enabling/disabling VLAN aware multicast snooping
  * adding a goto in br_multicast_update_active() to avoid
    unnecessary checks
  * turning "if (!br_multicast_ctx_is_vlan(brmctx))" in
    br_multicast_update_active() into an "else if" to avoid
    unnecessary checks
* [PATCH net-next v3 10/14] net: bridge: mcast: track active state, bridge up/down 
  * adding a goto in br_multicast_update_active() to avoid
    unnecessary checks
* [PATCH net-next v3 11/14] net: bridge: mcast: track active state, prepare for outside lock reads
  * removing the unnecessary READ_ONCE() macros in spinlocked
    sections in br_multicast_notify_active() and br_multicast_update_active()
* [PATCH net-next v3 12/14] net: bridge: mcast: use combined active state in netlink
  * no changes, adding "Reviewed-by: Ido Schimmel <idosch@nvidia.com>"
* [PATCH net-next v3 13/14] net: bridge: mcast: use combined active state in fast/data path
  * changing the proto parameter to br_multicast_querier_exists() from
    int to u16
  * dropping unnecessary parenthesis in switch cases
  * avoiding lines over 80 columns for calls to
    br_multicast_snooping_active() in br_{device,input}.c and
    the br_multicast_querier_exists() definition in br_multicast.c
* [PATCH net-next v3 14/14] net: bridge: mcast: add inactive state assertions
  * replace WARN_ON()s with WARN_ON_ONCE()

# Changelog v2

Summary of notable changes:

With the newly added selftests things are moved around a bit,
to be able to early on verify every commit that the general
and netlink ABI behaviour did not change.

The locking for br_multicast_open() and br_multicast_stop()
is now more symmetric.

The newly added tests revealed that in the previous pull-request
the checks were insufficient/incorrect when using VLANs, this
got fixed.

An inactive assertion / kernel splat was fixed.


* all: rebased to current net-next/main (a90f6dcefca6)
* [PATCH net-next v2 1/14] net: bridge: mcast: export ip{4,6}_active state to netlink
 * removed the ip{4,6}_active state variables for now,
   using/mimicking the current checks used in fast-path
   to define the intended result for the netlink/userspace ABI
 * br_vlan_global_opts_fill(): removing "const" from
   "struct net_bridge_vlan *v_opts" (for now), to allow
   dereferencing it
   br->multicast_ctx.ip{4,6}_active to start with,
* [PATCH net-next v2 2/14] net: bridge: mcast: track active state, adding tests
  * NEW: test that the new netlink interface has the intended results
* [PATCH net-next v2 3/14] net: bridge: mcast: avoid sleeping on bridge-down
  * NEW: to be able
* [PATCH net-next v2 4/14] net: bridge: mcast: track active state, IGMP/MLD querier appearance
  * adding br_multicast_notify_active() with a simple br_info()
    to start with to track the evolving ip{4,6}_active behaviour
  * br_ip{4,6}_multicast_query_delay_expired(): don't enable
    ip{4,6}_active if we are about to get disabled by
    adding/checking br_multicast_stopping()
  * adding protocol specific ...delay_timer.function resets
    in (new) br_multicast_reset_timer_cbs(), instead of
    changing timer_setup()s in br_multicast_ctx_init()
* [PATCH net-next v2 5/14] net: bridge: mcast: track active state, foreign IGMP/MLD querier disappearance
  * unchanged
* [PATCH net-next v2 6/14] net: bridge: mcast: track active state, IPv6 address availability
  * unchanged
* [PATCH net-next v2 7/14] net: bridge: mcast: track active state, own MLD querier disappearance
  * unchanged  
* [PATCH net-next v2 8/14] net: bridge: mcast: track active state, if snooping is enabled
  * renamed title: "... mcast: active state ..."
    -> "... mcast: track activestate ..."
  * adding VLAN aware check for BROPT_MCAST_VLAN_SNOOPING_ENABLED
  * removing misplaced, second br_multicast_update_active(brmctx)
    in br_multicast_toggle_one_vlan()
* [PATCH net-next v2 9/14] net: bridge: mcast: track active state, VLAN snooping
  * NEW
* [PATCH net-next v2 10/14] net: bridge: mcast: track active state, bridge up/down
  * adding mcast-spin-lock around br_multicast_open() in br_dev_open()a
  * adding mcast-spin-lock around br_multicast_update_active()
    in __br_multicast_stop()
  * removing (now already locked) mcast-spin-lock addition
    in br_multicast_toggle_one_vlan()
  * removing (now already locked) mcast-spin-lock addition
    in br_multicast_toggle_vlan_snooping()
* [PATCH net-next v2 11/14] net: bridge: mcast: track active state, prepare for outside lock reads
  * NEW
* [PATCH net-next v2 12/14] net: bridge: mcast: use combined active state in netlink
  * NEW (partially from: "net: bridge: mcast: export ip{4,6}_active state to netlink")
* [PATCH net-next v2 13/14] net: bridge: mcast: use combined active state in fast/data path
  * using READ_ONCE() on ip{4,6}_active states
* [PATCH net-next v2 14/14] net: bridge: mcast: add inactive state assertions
  * with the moved lock, bail out early if we are about to
    get disabled (via br_multicast_stopping()) and just entered
    br_ip{4,6}_multicast_querier_expired()
  * removed br_multicast_assert_inactive() in
    br_multicast_toggle_one_vlan(), as it can toggle to enabled

# Changelog (v1)

Changelog to / follow-up of: [PATCH net-next 0/5] net: bridge: propagate safe mcast snooping to switchdev + DSA
-> https://lkml.org/lkml/2025/5/22/1413

* removed the switchdev/DSA changes for now
* splitting "[PATCH net-next 1/5] net: bridge: mcast: explicitly track active state"
  into:
  * net: bridge: mcast: track active state, IGMP/MLD querier appearance
  * net: bridge: mcast: track active state, foreign IGMP/MLD querier disappearance
  * net: bridge: mcast: track active state, IPv6 address availability
  * net: bridge: mcast: track active state, own MLD querier disappearance
  * net: bridge: mcast: use combined active state in fast/data path
  * net: bridge: mcast: track active state, bridge up/down

* rebased to current net-next/main:
  * from_timer() -> timer_container_of()

* net: bridge: mcast: export ip{4,6}_active state to netlink:
  * changing NLA_U8 to NLA_REJECT to make it read-only

* moved br_multicast_update_active() call from br_ip{4,6}_multicast_query_expired()
  (own querier timer callback) to br_ip{4,6}_multicast_querier_expired()
  (other querier timer callback)
  * even though both should have worked as br_multicast_querier_expired()
    would call br_multicast_start_querier()->...->br_multicast_query_expired(),
    even if the own querier is disabled, but let's use the more direct way

* simplified br_multicast_update_active():
  * no return value for now, don't track if the active state has changed,
    these aren't necessary (yet)
  * removed __br_multicast_update_active() variant as was used to force
    an inactive state in __br_multicast_stop(), instead using an
    netif_running(brmctx->br->dev) check in br_multicast_update_active()
  * replaced br_ip{4,6}_multicast_check_active() with simpler
    br_ip{4,6}_multicast_update_active() and
    br_ip{4,6}_multicast_querier_exists()
  * fixing build errors with CONFIG_IPV6 unset
* simplified br_multicast_toggle_enabled()
  * no return value for now
  * fixes "old used uninitialized" issue

* removed const from __br_multicast_querier_exists()'s "bool is_ipv6"
* replaced "struct ethhdr *eth" in br_multicast_{snooping,querier}_active()
  with direct ethernet protocol integer attributes
* added a few comments in br_multicast_update_active() calling functions

Linus Lüssing (14):
  net: bridge: mcast: export ip{4,6}_active state to netlink
  net: bridge: mcast: track active state, adding tests
  net: bridge: mcast: avoid sleeping on bridge-down
  net: bridge: mcast: track active state, IGMP/MLD querier appearance
  net: bridge: mcast: track active state, foreign IGMP/MLD querier
    disappearance
  net: bridge: mcast: track active state, IPv6 address availability
  net: bridge: mcast: track active state, own MLD querier disappearance
  net: bridge: mcast: track active state, if snooping is enabled
  net: bridge: mcast: track active state, VLAN snooping
  net: bridge: mcast: track active state, bridge up/down
  net: bridge: mcast: track active state, prepare for outside lock reads
  net: bridge: mcast: use combined active state in netlink
  net: bridge: mcast: use combined active state in fast/data path
  net: bridge: mcast: add inactive state assertions

 include/uapi/linux/if_bridge.h                |   2 +
 include/uapi/linux/if_link.h                  |  14 +
 net/bridge/br_device.c                        |   3 +-
 net/bridge/br_input.c                         |   3 +-
 net/bridge/br_multicast.c                     | 405 ++++++++--
 net/bridge/br_netlink.c                       |  10 +-
 net/bridge/br_private.h                       |  51 +-
 net/bridge/br_vlan_options.c                  |  10 +-
 net/core/rtnetlink.c                          |   2 +-
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../net/forwarding/bridge_mdb_active.sh       | 742 ++++++++++++++++++
 11 files changed, 1157 insertions(+), 86 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/bridge_mdb_active.sh

-- 
2.53.0

[PATCH net-next v4 01/14] 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>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 include/uapi/linux/if_bridge.h |  2 ++
 include/uapi/linux/if_link.h   | 12 ++++++++++++
 net/bridge/br_netlink.c        | 20 +++++++++++++++++++-
 net/bridge/br_private.h        |  2 +-
 net/bridge/br_vlan_options.c   | 25 +++++++++++++++++++++++--
 net/core/rtnetlink.c           |  2 +-
 6 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index e52f8207ab27..910103b1ef03 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 83a96c56b8ca..66c59953d301 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -744,6 +744,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 IPv6 mcast active state, read only.
+ *
+ *   1 if an MLD querier is present, 0 otherwise.
  */
 enum {
 	IFLA_BR_UNSPEC,
@@ -796,6 +806,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 0264730938f4..7fff59c3bf76 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_REJECT },
 	[IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 },
+	[IFLA_BR_MCAST_ACTIVE_V6] = { .type = NLA_REJECT },
 	[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)
@@ -1639,6 +1643,8 @@ static size_t br_get_size(const struct net_device *brdev)
 
 static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
 {
+	struct ethhdr eth6 __maybe_unused = { .h_proto = htons(ETH_P_IPV6) };
+	struct ethhdr eth4 __maybe_unused = { .h_proto = htons(ETH_P_IP) };
 	struct net_bridge *br = netdev_priv(brdev);
 	u32 forward_delay = jiffies_to_clock_t(br->forward_delay);
 	u32 hello_time = jiffies_to_clock_t(br->hello_time);
@@ -1717,12 +1723,24 @@ 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,
+		       netif_running(brdev) &&
+		       br_opt_get(br, BROPT_MULTICAST_ENABLED) &&
+		       !br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
+		       br_multicast_querier_exists(&br->multicast_ctx, &eth4,
+						   NULL)) ||
 	    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,
+		       netif_running(brdev) &&
+		       br_opt_get(br, BROPT_MULTICAST_ENABLED) &&
+		       !br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
+		       br_multicast_querier_exists(&br->multicast_ctx, &eth6,
+						   NULL)))
 		return -EMSGSIZE;
 #endif
 	clockval = jiffies_to_clock_t(br->multicast_ctx.multicast_last_member_interval);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 9b55d38ea9ed..545be319b6cb 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1902,7 +1902,7 @@ int br_vlan_rtm_process_global_options(struct net_device *dev,
 bool br_vlan_global_opts_can_enter_range(const struct net_bridge_vlan *v_curr,
 					 const struct net_bridge_vlan *r_end);
 bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
-			      const struct net_bridge_vlan *v_opts);
+			      struct net_bridge_vlan *v_opts);
 
 /* vlan state manipulation helpers using *_ONCE to annotate lock-free access,
  * while br_vlan_set_state() may access data protected by multicast_lock.
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 5514e1fc8d1f..06045d68a2d1 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -367,10 +367,13 @@ bool br_vlan_global_opts_can_enter_range(const struct net_bridge_vlan *v_curr,
 }
 
 bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
-			      const struct net_bridge_vlan *v_opts)
+			      struct net_bridge_vlan *v_opts)
 {
+	struct ethhdr eth6 __maybe_unused = { .h_proto = htons(ETH_P_IPV6) };
+	struct ethhdr eth4 __maybe_unused = { .h_proto = htons(ETH_P_IP) };
 	struct nlattr *nest2 __maybe_unused;
 	u64 clockval __maybe_unused;
+	bool mcact __maybe_unused;
 	struct nlattr *nest;
 
 	nest = nla_nest_start(skb, BRIDGE_VLANDB_GLOBAL_OPTIONS);
@@ -385,10 +388,17 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 		goto out_err;
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
+	mcact = netif_running(v_opts->br->dev) &&
+		br_opt_get(v_opts->br, BROPT_MULTICAST_ENABLED) &&
+		br_opt_get(v_opts->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
+		!br_multicast_ctx_vlan_global_disabled(&v_opts->br_mcast_ctx) &&
+		br_multicast_querier_exists(&v_opts->br_mcast_ctx, &eth4, NULL);
+
 	if (nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_SNOOPING,
 		       !!(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, mcact) ||
 	    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,
@@ -442,8 +452,15 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 	}
 
 #if IS_ENABLED(CONFIG_IPV6)
+	mcact = netif_running(v_opts->br->dev) &&
+		br_opt_get(v_opts->br, BROPT_MULTICAST_ENABLED) &&
+		br_opt_get(v_opts->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
+		!br_multicast_ctx_vlan_global_disabled(&v_opts->br_mcast_ctx) &&
+		br_multicast_querier_exists(&v_opts->br_mcast_ctx, &eth6, NULL);
+
 	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, mcact))
 		goto out_err;
 #endif
 #endif
@@ -468,7 +485,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 */
@@ -650,9 +669,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_REJECT },
 	[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_REJECT },
 	[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 dad4b1054955..8febe7475f1f 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.53.0

[PATCH net-next v4 02/14] net: bridge: mcast: track active state, adding tests

[Linus Lüssing]

Before making any significant changes to the internals of the Linux
bridge add some tests regarding the multicast activity. This is
also to verify that we have the semantics of the new
*_MCAST_ACTIVE_{V4,V6} netlink attributes as expected.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../net/forwarding/bridge_mdb_active.sh       | 742 ++++++++++++++++++
 2 files changed, 743 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/bridge_mdb_active.sh

diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index bbaf4d937dd8..6692751b24d5 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -7,6 +7,7 @@ TEST_PROGS := \
 	bridge_igmp.sh \
 	bridge_locked_port.sh \
 	bridge_mdb.sh \
+	bridge_mdb_active.sh \
 	bridge_mdb_host.sh \
 	bridge_mdb_max.sh \
 	bridge_mdb_port_down.sh \
diff --git a/tools/testing/selftests/net/forwarding/bridge_mdb_active.sh b/tools/testing/selftests/net/forwarding/bridge_mdb_active.sh
new file mode 100755
index 000000000000..3c93c55ffa61
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/bridge_mdb_active.sh
@@ -0,0 +1,742 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# shellcheck disable=SC2317
+#
+# Disabling SC2317 because:
+# 1) shellcheck does not seem to like a trap together with a final exit call
+# 2) shellcheck does not seem to understand that tests_run calls all functions
+#    listed in ALL_TESTS
+
+# +-------+      +---------+
+# | brq0  |      | br0     |
+# | + $h1 |      | + $swp1 |
+# +----|--+      +----|----+
+#      |              |
+#      \--------------/
+#
+#
+# This script checks if we have the expected mcast_active_v{4,6} state
+# on br0 in a variety of scenarios. This state determines if ultimately
+# multicast snooping is applied to multicast data packets
+# (multicast snooping active) or if they are (by default) flooded instead
+# (multicast snooping inactive).
+#
+# Notably, multicast snooping can be enabled but still be inactive if not all
+# requirements to safely apply multicast snooping to multicast data packets
+# are met.
+#
+# Depending on the test case an IGMP/MLD querier might be on brq0, on br0
+# or neither.
+
+
+ALL_TESTS="
+	test_inactive
+	test_active_other_querier
+	test_active_own_querier
+	test_inactive_brdown
+	test_inactive_nov6
+	test_inactive_snooping_off
+	test_inactive_querier_off
+	test_inactive_other_querier_norespdelay
+	test_inactive_own_querier_norespdelay
+	test_inactive
+	test_vlan_inactive
+	test_vlan_active_other_querier
+	test_vlan_active_own_querier
+	test_vlan_inactive_brdown
+	test_vlan_inactive_nov6
+	test_vlan_inactive_snooping_off
+	test_vlan_inactive_vlans_snooping_off
+	test_vlan_inactive_vlan_snooping_off
+	test_vlan_inactive_other_querier_norespdelay
+	test_vlan_inactive_own_querier_norespdelay
+"
+
+NUM_NETIFS=2
+MCAST_MAX_RESP_IVAL_SEC=1
+MCAST_VLAN_ID=42
+source lib.sh
+
+switch_create()
+{
+	ip link add dev br0 type bridge\
+		vlan_filtering 0 \
+		mcast_query_response_interval $((MCAST_MAX_RESP_IVAL_SEC*100))\
+		mcast_snooping 0 \
+		mcast_vlan_snooping 0
+	ip link add dev brq0 type bridge\
+		vlan_filtering 0 \
+		mcast_query_response_interval $((MCAST_MAX_RESP_IVAL_SEC*100))\
+		mcast_snooping 0 \
+		mcast_vlan_snooping 0
+
+	echo 1 > /proc/sys/net/ipv6/conf/br0/disable_ipv6
+	echo 1 > /proc/sys/net/ipv6/conf/brq0/disable_ipv6
+
+	ip link set dev "$swp1" master br0
+	ip link set dev "$h1" master brq0
+
+	ip link set dev "$h1" up
+	ip link set dev "$swp1" up
+}
+
+switch_destroy()
+{
+	ip link set dev "$swp1" down
+	ip link set dev "$h1" down
+
+	ip link del dev brq0
+	ip link del dev br0
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	swp1=${NETIFS[p2]}
+
+	switch_create
+}
+
+cleanup()
+{
+	pre_cleanup
+	switch_destroy
+}
+
+mcast_active_check_failed()
+{
+	local af="$1"
+	local vlan="$2"
+	local active="$3"
+	local lineno="$4"
+
+	check_err 1 "$lineno: Mcast ${vlan}${active} check failed ($af)"
+}
+
+mcast_active_check_ret()
+{
+	local ret="$1"
+	local af="$2"
+	local state="$3"
+	local vlan="$4"
+	local lineno="$5"
+
+	if [ "$state" -eq 0 ]; then
+		if [ "$ret" -eq 0 ]; then
+			mcast_active_check_failed \
+				"$af" "$vlan" "inactive" "$lineno"
+		fi
+	elif [ "$state" -eq 1 ]; then
+		if [ "$ret" -ne 0 ]; then
+			mcast_active_check_failed \
+				"$af" "$vlan" "active" "$lineno"
+		fi
+	fi
+}
+
+jq_check_mcast_active()
+{
+	local af="$1"
+
+	jq -e ".[] | select(.linkinfo.info_data.mcast_active_$af == 1)"
+}
+
+mcast_active_check()
+{
+	local af="$1"
+	local state="$2"
+	local lineno="$3"
+
+	ip -d -j link show dev br0\
+		| jq_check_mcast_active "$af" &> /dev/null
+	mcast_active_check_ret "$?" "$af" "$state" "" "$lineno"
+}
+
+jq_check_vlan_mcast_active()
+{
+	local vid="$1"
+	local af="$2"
+
+	jq -e ".[].vlans.[] | select(.vlan == $vid and .mcast_active_$af == 1)"
+}
+
+mcast_vlan_active_check()
+{
+	local af="$1"
+	local state="$2"
+	local lineno="$3"
+	local vid="${MCAST_VLAN_ID}"
+
+	bridge -j vlan global show dev br0\
+		| jq_check_vlan_mcast_active "$vid" "$af" &> /dev/null
+	mcast_active_check_ret "$?" "$af" "$state" "VLAN " "$lineno"
+}
+
+mcast_assert_active_v4()
+{
+	mcast_active_check "v4" "1" "$1"
+}
+
+mcast_assert_active_v6()
+{
+	mcast_active_check "v6" "1" "$1"
+}
+
+mcast_assert_inactive_v4()
+{
+	mcast_active_check "v4" "0" "$1"
+}
+
+mcast_assert_inactive_v6()
+{
+	mcast_active_check "v6" "0" "$1"
+}
+
+mcast_vlan_assert_active_v4()
+{
+	mcast_vlan_active_check "v4" "1" "$1"
+}
+
+mcast_vlan_assert_active_v6()
+{
+	mcast_vlan_active_check "v6" "1" "$1"
+}
+
+mcast_vlan_assert_inactive_v4()
+{
+	mcast_vlan_active_check "v4" "0" "$1"
+}
+
+mcast_vlan_assert_inactive_v6()
+{
+	mcast_vlan_active_check "v6" "0" "$1"
+}
+
+test_inactive_nolog()
+{
+	local lineno="$1"
+
+	ip link set dev br0 down
+	ip link set dev brq0 down
+	ip link set dev br0 type bridge mcast_snooping 0
+	ip link set dev brq0 type bridge mcast_snooping 0
+	ip link set dev br0 type bridge mcast_querier 0
+	ip link set dev brq0 type bridge mcast_querier 0
+	ip link set dev br0 type bridge mcast_vlan_snooping 0
+	ip link set dev br0 type bridge vlan_filtering 0
+
+	echo 1 > /proc/sys/net/ipv6/conf/br0/disable_ipv6
+	echo 1 > /proc/sys/net/ipv6/conf/brq0/disable_ipv6
+
+	mcast_assert_inactive_v4 "$lineno"
+	mcast_assert_inactive_v6 "$lineno"
+}
+
+test_inactive()
+{
+	RET=0
+
+	test_inactive_nolog "$LINENO"
+	log_test "Mcast inactive test"
+}
+
+wait_lladdr_dad() {
+	local jq_select
+	local jq_tentative
+
+	jq_select="select(.scope == \"link\" and ((.tentative == true) | not))"
+	jq_tentative="map(${jq_select}) | .[]"
+
+	ip -6 -j a s dev "$1"\
+		| jq -e ".[].addr_info | ${jq_tentative}" &> /dev/null
+}
+
+test_active_setup_bridge()
+{
+	[ -n "$1" ] && echo 0 > /proc/sys/net/ipv6/conf/br0/disable_ipv6
+	[ -n "$2" ] && echo 0 > /proc/sys/net/ipv6/conf/brq0/disable_ipv6
+
+	[ -n "$3" ] && ip link set dev br0 up
+	[ -n "$4" ] && ip link set dev brq0 up
+	[ -n "$5" ] && slowwait 3 wait_lladdr_dad br0
+	[ -n "$6" ] && slowwait 3 wait_lladdr_dad brq0
+}
+
+test_active_setup_config()
+{
+	[ -n "$1" ] && ip link set dev br0 type bridge mcast_snooping 1
+	[ -n "$2" ] && ip link set dev brq0 type bridge mcast_snooping 1
+	[ -n "$3" ] && ip link set dev br0 type bridge mcast_querier 1
+	[ -n "$4" ] && ip link set dev brq0 type bridge mcast_querier 1
+}
+
+test_active_setup_wait()
+{
+	sleep $((MCAST_MAX_RESP_IVAL_SEC * 2))
+}
+
+test_active_setup_reset_own_querier()
+{
+	ip link set dev br0 type bridge mcast_querier 0
+	ip link set dev br0 type bridge mcast_querier 1
+
+	test_active_setup_wait
+}
+
+test_vlan_active_setup_config()
+{
+	[ -n "$1" ] && ip link set dev br0 type bridge vlan_filtering 1
+	[ -n "$2" ] && ip link set dev brq0 type bridge vlan_filtering 1
+	[ -n "$3" ] && ip link set dev br0 type bridge mcast_vlan_snooping 1
+	[ -n "$4" ] && ip link set dev brq0 type bridge mcast_vlan_snooping 1
+}
+
+test_vlan_active_setup_add_vlan()
+{
+	bridge vlan add vid "${MCAST_VLAN_ID}" dev "$swp1"
+	bridge vlan add vid "${MCAST_VLAN_ID}" dev "$h1"
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev br0\
+		mcast_query_response_interval $((MCAST_MAX_RESP_IVAL_SEC*100))
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev brq0\
+		mcast_query_response_interval $((MCAST_MAX_RESP_IVAL_SEC*100))
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev br0 mcast_snooping 0
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev brq0 mcast_snooping 0
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev br0 mcast_querier 0
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev brq0 mcast_querier 0
+}
+
+test_vlan_active_setup_config_vlan()
+{
+	[ -n "$1" ] && bridge vlan global set \
+			vid "${MCAST_VLAN_ID}" dev br0 mcast_snooping 1
+	[ -n "$2" ] && bridge vlan global set \
+			vid "${MCAST_VLAN_ID}" dev brq0 mcast_snooping 1
+	[ -n "$3" ] && bridge vlan global set \
+			vid "${MCAST_VLAN_ID}" dev br0 mcast_querier 1
+	[ -n "$4" ] && bridge vlan global set \
+			vid "${MCAST_VLAN_ID}" dev brq0 mcast_querier 1
+}
+
+test_vlan_teardown()
+{
+	bridge vlan del vid "${MCAST_VLAN_ID}" dev "$swp1"
+	bridge vlan del vid "${MCAST_VLAN_ID}" dev "$h1"
+	mcast_assert_inactive_v4 "$1"
+	mcast_assert_inactive_v6 "$1"
+	mcast_vlan_assert_inactive_v4 "$1"
+	mcast_vlan_assert_inactive_v6 "$1"
+}
+
+test_vlan_active_setup_reset_own_querier()
+{
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev br0 mcast_querier 0
+	bridge vlan global set vid "${MCAST_VLAN_ID}" dev br0 mcast_querier 1
+
+	test_active_setup_wait
+}
+
+test_active_other_querier_nolog()
+{
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  "4"
+	test_active_setup_wait
+
+	mcast_assert_active_v4 "$1"
+	mcast_assert_active_v6 "$1"
+}
+
+test_active_other_querier()
+{
+	RET=0
+
+	test_active_other_querier_nolog "$LINENO"
+	test_inactive_nolog "$LINENO"
+	log_test "Mcast active with other querier test"
+}
+
+test_active_own_querier_nolog()
+{
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_active_v4 "$1"
+	mcast_assert_active_v6 "$1"
+}
+
+test_active_own_querier()
+{
+	RET=0
+
+	test_active_own_querier_nolog "$LINENO"
+	test_inactive_nolog "$LINENO"
+	log_test "Mcast active with own querier test"
+}
+
+test_active_final()
+{
+	mcast_assert_active_v4 "$1"
+	mcast_assert_active_v6 "$1"
+
+	test_inactive_nolog "$1"
+}
+
+test_inactive_brdown()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" ""  "4" ""  "6"
+	test_active_setup_config "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_bridge ""  ""  "3" ""  ""  ""
+	mcast_assert_active_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_bridge ""  ""  ""  ""  "5" ""
+	test_active_setup_reset_own_querier
+	test_active_final "$LINENO"
+
+	log_test "Mcast inactive, bridge down test"
+}
+
+test_inactive_nov6()
+{
+	RET=0
+
+	test_active_setup_bridge ""  "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_active_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_bridge "1" ""  ""  ""  "5" ""
+	test_active_setup_reset_own_querier
+	test_active_final "$LINENO"
+
+	log_test "Mcast inactive, own querier, no IPv6 address test"
+}
+
+test_inactive_snooping_off()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config ""  "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_config "1" ""  ""  ""
+	test_active_setup_reset_own_querier
+	test_active_final "$LINENO"
+
+	log_test "Mcast inactive, snooping disabled test"
+}
+
+test_inactive_querier_off()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_config ""  ""  "3" ""
+	test_active_setup_wait
+	test_active_final "$LINENO"
+
+	log_test "Mcast inactive, no querier test"
+}
+
+test_inactive_other_querier_norespdelay()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" "3" ""
+	# skipping: test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_wait
+	test_active_final "$LINENO"
+
+	log_test "Mcast inactive, other querier, no response delay test"
+}
+
+test_inactive_own_querier_norespdelay()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  "4"
+	# skipping: test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_wait
+	test_active_final "$LINENO"
+
+	log_test "Mcast inactive, own querier, no response delay test"
+}
+
+test_vlan_inactive()
+{
+	RET=0
+
+	test_inactive_nolog "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	ip link set dev br0 type bridge vlan_filtering 1
+	ip link set dev br0 type bridge mcast_vlan_snooping 1
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	ip link set dev br0 type bridge mcast_vlan_snooping 0
+	ip link set dev br0 type bridge vlan_filtering 0
+	test_active_own_querier_nolog "$LINENO"
+	ip link set dev br0 type bridge vlan_filtering 1
+	mcast_assert_active_v4 "$LINENO"
+	mcast_assert_active_v6 "$LINENO"
+
+	ip link set dev br0 type bridge mcast_vlan_snooping 1
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_inactive_nolog "$LINENO"
+	log_test "Mcast VLAN inactive test"
+}
+
+test_vlan_active_final()
+{
+	mcast_assert_inactive_v4 "$1"
+	mcast_assert_inactive_v6 "$1"
+	mcast_vlan_assert_active_v4 "$1"
+	mcast_vlan_assert_active_v6 "$1"
+
+	test_vlan_teardown "$1"
+	test_inactive_nolog "$1"
+}
+
+test_vlan_active_other_querier()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" ""  "4"
+	test_active_setup_wait
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN active, other querier test"
+}
+
+test_vlan_active_own_querier()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" "3" ""
+	test_active_setup_wait
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN active, own querier test"
+}
+
+test_vlan_inactive_brdown()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" ""  "4" ""  "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_bridge ""  ""  "3" ""  ""  ""
+	test_active_setup_wait
+	mcast_vlan_assert_active_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_bridge ""  ""  ""  ""  "5" ""
+	test_vlan_active_setup_reset_own_querier
+	test_vlan_active_final "$LINENO, RET: $RET"
+
+	log_test "Mcast VLAN inactive, bridge down test"
+}
+
+test_vlan_inactive_nov6()
+{
+	RET=0
+
+	test_active_setup_bridge ""  "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_active_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_bridge "1" ""  ""  ""  "5" ""
+	test_vlan_active_setup_reset_own_querier
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN inactive, own querier, no IPv6 address test"
+}
+
+test_vlan_inactive_snooping_off()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config ""  "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_config "1" ""  ""  ""
+	test_vlan_active_setup_reset_own_querier
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN inactive, snooping disabled test"
+}
+
+test_vlan_inactive_vlans_snooping_off()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" ""  "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_vlan_active_setup_config ""  ""  "3" ""
+	test_vlan_active_setup_reset_own_querier
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN inactive, snooping for VLANs disabled test"
+}
+
+test_vlan_inactive_vlan_snooping_off()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan ""  "2" "3" ""
+	test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_vlan_active_setup_config_vlan "1" ""  ""  ""
+	test_vlan_active_setup_reset_own_querier
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN inactive, snooping for this VLAN disabled test"
+}
+
+test_vlan_inactive_other_querier_norespdelay()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" ""  "4"
+	# skipping: test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_wait
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN inactive, other querier, no response delay test"
+}
+
+test_vlan_inactive_own_querier_norespdelay()
+{
+	RET=0
+
+	test_active_setup_bridge "1" "2" "3" "4" "5" "6"
+	test_active_setup_config "1" "2" ""  ""
+	test_vlan_active_setup_config "1" "2" "3" "4"
+	test_vlan_active_setup_add_vlan
+	test_vlan_active_setup_config_vlan "1" "2" "3" ""
+	# skipping: test_active_setup_wait
+
+	mcast_assert_inactive_v4 "$LINENO"
+	mcast_assert_inactive_v6 "$LINENO"
+	mcast_vlan_assert_inactive_v4 "$LINENO"
+	mcast_vlan_assert_inactive_v6 "$LINENO"
+
+	test_active_setup_wait
+	test_vlan_active_final "$LINENO"
+
+	log_test "Mcast VLAN inactive, own querier, no response delay test"
+}
+
+trap cleanup EXIT
+
+setup_prepare
+
+if ! ip -d link show dev br0 2>&1 | grep -q "mcast_active"; then
+	echo "SKIP: iproute2 too old, missing mcast_active support"
+	exit "$ksft_skip"
+fi
+
+setup_wait
+tests_run
+
+exit "$EXIT_STATUS"
-- 
2.53.0

[PATCH net-next v4 03/14] net: bridge: mcast: avoid sleeping on bridge-down

[Linus Lüssing]

We later want to use the multicast lock when setting the bridge
interface up or down, to be able to atomically both check all conditions
to toggle the multicast active state and to subsequently toggle it.
While most variables we check / contexts we check from are serialized
(toggled variables through netlink/sysfs) the timer_pending() check is
not and might run in parallel.

However so far we are not allowed to spinlock __br_multicast_stop() as
its call to timer_delete_sync() might sleep. Therefore replacing the
sleeping variant with the non-sleeping timer_shutdown(). It is sufficient
to only wait for any timer callback to finish when we are freeing the
multicast context.

While the disadvantage of using a non-syncing variant might lead us to
race and still execute its timer callback just after timer_shutdown() was
called timer_shutdown() also has the following additional advantage(s):
It for one thing clears the callback function pointer and by that avoids
rearming. For another a missing function pointer allows us to detect
early in the timer callback if we, this timer, were just canceled.

In other words, this also allows us to make sure that once
timer_shutdown() was called while we do potentially enter its timer
callback briefly we never run the main task of this timer. Similar to
what a timer_delete_sync() would have avoided, too. Except we are not
waiting/sleeping/syncing in br_multicast_stop() but instead (in rare cases)
briefly busy-wait/sync a bit later when grabbing the multicast spinlock
in the timer callback.

This new check also makes the netif_running() check redundant/obsolete
in these contexts.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/br_multicast.c | 146 ++++++++++++++++++++++++++++----------
 net/bridge/br_private.h   |   5 ++
 2 files changed, 113 insertions(+), 38 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 881d866d687a..cfe8d42f0214 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1663,6 +1663,14 @@ static void br_multicast_router_expired(struct net_bridge_mcast_port *pmctx,
 	spin_unlock(&br->multicast_lock);
 }
 
+static bool br_multicast_is_stopping(const struct net_bridge *br,
+				     const struct timer_list *timer)
+{
+	lockdep_assert_held_once(&br->multicast_lock);
+
+	return !timer->function;
+}
+
 static void br_ip4_multicast_router_expired(struct timer_list *t)
 {
 	struct net_bridge_mcast_port *pmctx = timer_container_of(pmctx, t,
@@ -1698,7 +1706,8 @@ static void br_multicast_local_router_expired(struct net_bridge_mcast *brmctx,
 					      struct timer_list *timer)
 {
 	spin_lock(&brmctx->br->multicast_lock);
-	if (brmctx->multicast_router == MDB_RTR_TYPE_DISABLED ||
+	if (br_multicast_is_stopping(brmctx->br, timer) ||
+	    brmctx->multicast_router == MDB_RTR_TYPE_DISABLED ||
 	    brmctx->multicast_router == MDB_RTR_TYPE_PERM ||
 	    br_ip4_multicast_is_router(brmctx) ||
 	    br_ip6_multicast_is_router(brmctx))
@@ -1728,10 +1737,11 @@ static void br_ip6_multicast_local_router_expired(struct timer_list *t)
 #endif
 
 static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
-					 struct bridge_mcast_own_query *query)
+					 struct bridge_mcast_own_query *query,
+					 struct timer_list *timer)
 {
 	spin_lock(&brmctx->br->multicast_lock);
-	if (!netif_running(brmctx->br->dev) ||
+	if (br_multicast_is_stopping(brmctx->br, timer) ||
 	    br_multicast_ctx_vlan_global_disabled(brmctx) ||
 	    !br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED))
 		goto out;
@@ -1747,7 +1757,7 @@ static void br_ip4_multicast_querier_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
 							     ip4_other_query.timer);
 
-	br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query);
+	br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query, t);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -1756,7 +1766,7 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
 							     ip6_other_query.timer);
 
-	br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query);
+	br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query, t);
 }
 #endif
 
@@ -4040,10 +4050,12 @@ int br_multicast_rcv(struct net_bridge_mcast **brmctx,
 }
 
 static void br_multicast_query_expired(struct net_bridge_mcast *brmctx,
-				       struct bridge_mcast_own_query *query)
+				       struct bridge_mcast_own_query *query,
+				       struct timer_list *timer)
 {
 	spin_lock(&brmctx->br->multicast_lock);
-	if (br_multicast_ctx_vlan_disabled(brmctx))
+	if (br_multicast_is_stopping(brmctx->br, timer) ||
+	    br_multicast_ctx_vlan_disabled(brmctx))
 		goto out;
 
 	if (query->startup_sent < brmctx->multicast_startup_query_count)
@@ -4059,7 +4071,7 @@ static void br_ip4_multicast_query_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
 							     ip4_own_query.timer);
 
-	br_multicast_query_expired(brmctx, &brmctx->ip4_own_query);
+	br_multicast_query_expired(brmctx, &brmctx->ip4_own_query, t);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -4068,7 +4080,7 @@ static void br_ip6_multicast_query_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
 							     ip6_own_query.timer);
 
-	br_multicast_query_expired(brmctx, &brmctx->ip6_own_query);
+	br_multicast_query_expired(brmctx, &brmctx->ip6_own_query, t);
 }
 #endif
 
@@ -4111,29 +4123,40 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
 #endif
 
-	timer_setup(&brmctx->ip4_mc_router_timer,
-		    br_ip4_multicast_local_router_expired, 0);
-	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);
-	timer_setup(&brmctx->ip4_own_query.timer,
-		    br_ip4_multicast_query_expired, 0);
+	timer_setup(&brmctx->ip4_mc_router_timer, NULL, 0);
+	timer_setup(&brmctx->ip4_other_query.timer, NULL, 0);
+	timer_setup(&brmctx->ip4_other_query.delay_timer, NULL, 0);
+	timer_setup(&brmctx->ip4_own_query.timer, NULL, 0);
 #if IS_ENABLED(CONFIG_IPV6)
-	timer_setup(&brmctx->ip6_mc_router_timer,
-		    br_ip6_multicast_local_router_expired, 0);
-	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);
-	timer_setup(&brmctx->ip6_own_query.timer,
-		    br_ip6_multicast_query_expired, 0);
+	timer_setup(&brmctx->ip6_mc_router_timer, NULL, 0);
+	timer_setup(&brmctx->ip6_other_query.timer, NULL, 0);
+	timer_setup(&brmctx->ip6_other_query.delay_timer, NULL, 0);
+	timer_setup(&brmctx->ip6_own_query.timer, NULL, 0);
 #endif
+
+	/* for non-vlan multicast timer callbacks:
+	 * they are set on br_multicast_open() instead
+	 */
+	if (!vlan)
+		return;
+
+	spin_lock_bh(&br->multicast_lock);
+	br_multicast_reset_timer_cbs(&vlan->br_mcast_ctx);
+	spin_unlock_bh(&br->multicast_lock);
 }
 
 void br_multicast_ctx_deinit(struct net_bridge_mcast *brmctx)
 {
-	__br_multicast_stop(brmctx);
+	timer_shutdown_sync(&brmctx->ip4_mc_router_timer);
+	timer_shutdown_sync(&brmctx->ip4_other_query.timer);
+	timer_shutdown_sync(&brmctx->ip4_other_query.delay_timer);
+	timer_shutdown_sync(&brmctx->ip4_own_query.timer);
+#if IS_ENABLED(CONFIG_IPV6)
+	timer_shutdown_sync(&brmctx->ip6_mc_router_timer);
+	timer_shutdown_sync(&brmctx->ip6_other_query.timer);
+	timer_shutdown_sync(&brmctx->ip6_other_query.delay_timer);
+	timer_shutdown_sync(&brmctx->ip6_own_query.timer);
+#endif
 }
 
 void br_multicast_init(struct net_bridge *br)
@@ -4213,9 +4236,35 @@ void br_multicast_leave_snoopers(struct net_bridge *br)
 	br_ip6_multicast_leave_snoopers(br);
 }
 
+void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx)
+{
+	lockdep_assert_held_once(&brmctx->br->multicast_lock);
+
+	brmctx->ip4_mc_router_timer.function =
+		br_ip4_multicast_local_router_expired;
+	brmctx->ip4_other_query.timer.function =
+		br_ip4_multicast_querier_expired;
+	brmctx->ip4_other_query.delay_timer.function =
+		br_multicast_query_delay_expired;
+	brmctx->ip4_own_query.timer.function =
+		br_ip4_multicast_query_expired;
+#if IS_ENABLED(CONFIG_IPV6)
+	brmctx->ip6_mc_router_timer.function =
+		br_ip6_multicast_local_router_expired;
+	brmctx->ip6_other_query.timer.function =
+		br_ip6_multicast_querier_expired;
+	brmctx->ip6_other_query.delay_timer.function =
+		br_multicast_query_delay_expired;
+	brmctx->ip6_own_query.timer.function =
+		br_ip6_multicast_query_expired;
+#endif
+}
+
 static void __br_multicast_open_query(struct net_bridge *br,
 				      struct bridge_mcast_own_query *query)
 {
+	lockdep_assert_held_once(&br->multicast_lock);
+
 	query->startup_sent = 0;
 
 	if (!br_opt_get(br, BROPT_MULTICAST_ENABLED))
@@ -4226,13 +4275,15 @@ static void __br_multicast_open_query(struct net_bridge *br,
 
 static void __br_multicast_open(struct net_bridge_mcast *brmctx)
 {
+	br_multicast_reset_timer_cbs(brmctx);
+
 	__br_multicast_open_query(brmctx->br, &brmctx->ip4_own_query);
 #if IS_ENABLED(CONFIG_IPV6)
 	__br_multicast_open_query(brmctx->br, &brmctx->ip6_own_query);
 #endif
 }
 
-void br_multicast_open(struct net_bridge *br)
+static void br_multicast_open_locked(struct net_bridge *br)
 {
 	ASSERT_RTNL();
 
@@ -4256,17 +4307,26 @@ void br_multicast_open(struct net_bridge *br)
 	}
 }
 
+void br_multicast_open(struct net_bridge *br)
+{
+	spin_lock_bh(&br->multicast_lock);
+	br_multicast_open_locked(br);
+	spin_unlock_bh(&br->multicast_lock);
+}
+
 static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
 {
-	timer_delete_sync(&brmctx->ip4_mc_router_timer);
-	timer_delete_sync(&brmctx->ip4_other_query.timer);
-	timer_delete_sync(&brmctx->ip4_other_query.delay_timer);
-	timer_delete_sync(&brmctx->ip4_own_query.timer);
+	lockdep_assert_held_once(&brmctx->br->multicast_lock);
+
+	timer_shutdown(&brmctx->ip4_mc_router_timer);
+	timer_shutdown(&brmctx->ip4_other_query.timer);
+	timer_shutdown(&brmctx->ip4_other_query.delay_timer);
+	timer_shutdown(&brmctx->ip4_own_query.timer);
 #if IS_ENABLED(CONFIG_IPV6)
-	timer_delete_sync(&brmctx->ip6_mc_router_timer);
-	timer_delete_sync(&brmctx->ip6_other_query.timer);
-	timer_delete_sync(&brmctx->ip6_other_query.delay_timer);
-	timer_delete_sync(&brmctx->ip6_own_query.timer);
+	timer_shutdown(&brmctx->ip6_mc_router_timer);
+	timer_shutdown(&brmctx->ip6_other_query.timer);
+	timer_shutdown(&brmctx->ip6_other_query.delay_timer);
+	timer_shutdown(&brmctx->ip6_own_query.timer);
 #endif
 }
 
@@ -4317,12 +4377,12 @@ 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;
-		spin_unlock_bh(&br->multicast_lock);
 
 		if (on)
 			__br_multicast_open(&vlan->br_mcast_ctx);
 		else
 			__br_multicast_stop(&vlan->br_mcast_ctx);
+		spin_unlock_bh(&br->multicast_lock);
 	} else {
 		struct net_bridge_mcast *brmctx;
 
@@ -4380,6 +4440,7 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
 	if (!vg)
 		return 0;
 
+	spin_lock_bh(&br->multicast_lock);
 	br_opt_toggle(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED, on);
 
 	/* disable/enable non-vlan mcast contexts based on vlan snooping */
@@ -4387,6 +4448,8 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
 		__br_multicast_stop(&br->multicast_ctx);
 	else
 		__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_ctx(&p->multicast_ctx);
@@ -4416,7 +4479,7 @@ bool br_multicast_toggle_global_vlan(struct net_bridge_vlan *vlan, bool on)
 	return true;
 }
 
-void br_multicast_stop(struct net_bridge *br)
+static void br_multicast_stop_locked(struct net_bridge *br)
 {
 	ASSERT_RTNL();
 
@@ -4440,6 +4503,13 @@ void br_multicast_stop(struct net_bridge *br)
 	}
 }
 
+void br_multicast_stop(struct net_bridge *br)
+{
+	spin_lock_bh(&br->multicast_lock);
+	br_multicast_stop_locked(br);
+	spin_unlock_bh(&br->multicast_lock);
+}
+
 void br_multicast_dev_del(struct net_bridge *br)
 {
 	struct net_bridge_mdb_entry *mp;
@@ -4675,7 +4745,7 @@ int br_multicast_toggle(struct net_bridge *br, unsigned long val,
 	if (!netif_running(br->dev))
 		goto unlock;
 
-	br_multicast_open(br);
+	br_multicast_open_locked(br);
 	list_for_each_entry(port, &br->port_list, list)
 		__br_multicast_enable_port_ctx(&port->multicast_ctx);
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 545be319b6cb..af206a41555c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -976,6 +976,7 @@ void br_multicast_disable_port(struct net_bridge_port *port);
 void br_multicast_init(struct net_bridge *br);
 void br_multicast_join_snoopers(struct net_bridge *br);
 void br_multicast_leave_snoopers(struct net_bridge *br);
+void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx);
 void br_multicast_open(struct net_bridge *br);
 void br_multicast_stop(struct net_bridge *br);
 void br_multicast_dev_del(struct net_bridge *br);
@@ -1426,6 +1427,10 @@ static inline void br_multicast_leave_snoopers(struct net_bridge *br)
 {
 }
 
+static inline void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx)
+{
+}
+
 static inline void br_multicast_open(struct net_bridge *br)
 {
 }
-- 
2.53.0

[PATCH net-next v4 04/14] net: bridge: mcast: track active state, IGMP/MLD querier appearance

[Linus Lüssing]

This is the first step to track in dedicated, per protocol family
variables if we can actively and safely use multicast snooping.
To later use these in the fast/data path instead of performing
all these checks for every packet and to later notify DSA/switchdev
about this state.

This toggles these new variables to true after a Maximum Response Delay
(default: 10 seconds) if a new IGMP or MLD querier has appeared. This
can be triggered either through receiving an IGMP/MLD query from another
host or by a user enabling our own IGMP/MLD querier.

This is the first of several requirements, similar to what
br_multicast_querier_exists() already checks so far, to be able to
reliably receive IGMP/MLD reports, which in turn are needed to build
a complete multicast database.

No functional change for the fast/data path yet.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/br_multicast.c | 116 +++++++++++++++++++++++++++++++++++++-
 net/bridge/br_private.h   |   2 +
 2 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index cfe8d42f0214..9c22ec63fc6b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1067,6 +1067,84 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm
 	return skb;
 }
 
+static bool br_ip4_multicast_querier_exists(struct net_bridge_mcast *brmctx)
+{
+	return __br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query,
+					     false);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static bool br_ip6_multicast_querier_exists(struct net_bridge_mcast *brmctx)
+{
+	return __br_multicast_querier_exists(brmctx, &brmctx->ip6_other_query,
+					     true);
+}
+#endif
+
+static void br_ip4_multicast_update_active(struct net_bridge_mcast *brmctx,
+					   bool force_inactive)
+{
+	if (force_inactive)
+		brmctx->ip4_active = false;
+	else
+		brmctx->ip4_active = br_ip4_multicast_querier_exists(brmctx);
+}
+
+static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx,
+					   bool force_inactive)
+{
+#if IS_ENABLED(CONFIG_IPV6)
+	if (force_inactive)
+		brmctx->ip6_active = false;
+	else
+		brmctx->ip6_active = br_ip6_multicast_querier_exists(brmctx);
+#endif
+}
+
+static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
+				       bool ip4_active_old, bool ip6_active_old)
+{
+	if (brmctx->ip4_active == ip4_active_old &&
+	    brmctx->ip6_active == ip6_active_old)
+		return;
+
+	br_debug(brmctx->br, "mc_active changed, vid: %i: v4: %i->%i, v6: %i->%i\n",
+		 brmctx->vlan ? brmctx->vlan->vid : -1,
+		 ip4_active_old, brmctx->ip4_active,
+		 ip6_active_old, brmctx->ip6_active);
+}
+
+/**
+ * br_multicast_update_active() - update mcast active state
+ * @brmctx: the bridge multicast context to check
+ *
+ * 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
+ *
+ * And is unset otherwise.
+ *
+ * This function should be called by anything that changes one of the
+ * above prerequisites.
+ */
+static void br_multicast_update_active(struct net_bridge_mcast *brmctx)
+{
+	bool ip4_active_old = brmctx->ip4_active;
+	bool ip6_active_old = brmctx->ip6_active;
+	bool force_inactive = false;
+
+	lockdep_assert_held_once(&brmctx->br->multicast_lock);
+
+	br_ip4_multicast_update_active(brmctx, force_inactive);
+	br_ip6_multicast_update_active(brmctx, force_inactive);
+
+	br_multicast_notify_active(brmctx, ip4_active_old, ip6_active_old);
+}
+
 #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,
@@ -1770,10 +1848,40 @@ 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;
+
+	brmctx = timer_container_of(brmctx, t, ip4_other_query.delay_timer);
+
+	spin_lock(&brmctx->br->multicast_lock);
+	if (!br_multicast_is_stopping(brmctx->br, t))
+		/* an own or other IGMP querier appeared some seconds ago and
+		 * all reports should have arrived by now, maybe set multicast
+		 * state to active
+		 */
+		br_multicast_update_active(brmctx);
+	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;
+
+	brmctx = timer_container_of(brmctx, t, ip6_other_query.delay_timer);
+
+	spin_lock(&brmctx->br->multicast_lock);
+	if (!br_multicast_is_stopping(brmctx->br, t))
+		/* an own or other MLD querier appeared some seconds ago and
+		 * all reports should have arrived by now, maybe set multicast
+		 * state to active
+		 */
+		br_multicast_update_active(brmctx);
+	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)
@@ -4115,6 +4223,7 @@ 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)
@@ -4122,6 +4231,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	brmctx->ip6_querier.port_ifidx = 0;
 	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
 #endif
+	brmctx->ip6_active = 0;
 
 	timer_setup(&brmctx->ip4_mc_router_timer, NULL, 0);
 	timer_setup(&brmctx->ip4_other_query.timer, NULL, 0);
@@ -4245,7 +4355,7 @@ void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx)
 	brmctx->ip4_other_query.timer.function =
 		br_ip4_multicast_querier_expired;
 	brmctx->ip4_other_query.delay_timer.function =
-		br_multicast_query_delay_expired;
+		br_ip4_multicast_query_delay_expired;
 	brmctx->ip4_own_query.timer.function =
 		br_ip4_multicast_query_expired;
 #if IS_ENABLED(CONFIG_IPV6)
@@ -4254,7 +4364,7 @@ void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx)
 	brmctx->ip6_other_query.timer.function =
 		br_ip6_multicast_querier_expired;
 	brmctx->ip6_other_query.delay_timer.function =
-		br_multicast_query_delay_expired;
+		br_ip6_multicast_query_delay_expired;
 	brmctx->ip6_own_query.timer.function =
 		br_ip6_multicast_query_expired;
 #endif
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index af206a41555c..8d1c869eb522 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -160,6 +160,7 @@ 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;
@@ -167,6 +168,7 @@ struct net_bridge_mcast {
 	struct bridge_mcast_own_query	ip6_own_query;
 	struct bridge_mcast_querier	ip6_querier;
 #endif /* IS_ENABLED(CONFIG_IPV6) */
+	bool				ip6_active;
 #endif /* CONFIG_BRIDGE_IGMP_SNOOPING */
 };
 
-- 
2.53.0

[PATCH net-next v4 05/14] net: bridge: mcast: track active state, foreign IGMP/MLD querier disappearance

[Linus Lüssing]

This change ensures that the new multicast active state variable is unset
again after a foreign IGMP/MLD querier has disappeared (default: 255
seconds). If no new, other IGMP/MLD querier took over then we can't
reliably receive IGMP/MLD reports anymore and in turn can't ensure the
completeness of our MDB anymore either.

No functional change for the fast/data path yet.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_multicast.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 9c22ec63fc6b..64aac9c415e2 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1827,6 +1827,10 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
 	br_multicast_start_querier(brmctx, query);
 
 out:
+	/* another IGMP/MLD querier disappeared, set multicast state to inactive
+	 * if our own querier is disabled, too
+	 */
+	br_multicast_update_active(brmctx);
 	spin_unlock(&brmctx->br->multicast_lock);
 }
 
-- 
2.53.0

[PATCH net-next v4 06/14] net: bridge: mcast: track active state, IPv6 address availability

[Linus Lüssing]

If we are the only potential MLD querier but don't have an IPv6
link-local address configured on our bridge interface then we can't
create a valid MLD query and in turn can't reliably receive MLD reports
and can't build a complete MDB. Hence disable the new multicast active
state variable then. Or reenable it if an IPv6 link-local address
became available.

No functional change for the fast/data path yet.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_multicast.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 64aac9c415e2..2eebfb7c5ff9 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1125,6 +1125,8 @@ static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
  * The multicast active state is set, per protocol family, if:
  *
  * - an IGMP/MLD querier is present
+ * - for own IPv6 MLD querier: an IPv6 link-local address is configured on the
+ *   bridge
  *
  * And is unset otherwise.
  *
@@ -1223,10 +1225,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);
 		return NULL;
 	}
 
 	br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, true);
+	br_multicast_update_active(brmctx);
 	ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
 
 	hopopt = (u8 *)(ip6h + 1);
-- 
2.53.0

[PATCH net-next v4 07/14] net: bridge: mcast: track active state, own MLD querier disappearance

[Linus Lüssing]

This change ensures that the new multicast active state variable is
immediately unset if our internal IGMP/MLD querier was elected and
now disabled.

If no IGMP/MLD querier exists on the link then we can't reliably receive
IGMP/MLD reports and in turn can't ensure the completeness of our MDB
anymore either.

No functional change for the fast/data path yet.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_multicast.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 2eebfb7c5ff9..f37a18f3713c 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4945,6 +4945,7 @@ int br_multicast_set_querier(struct net_bridge_mcast *brmctx, unsigned long val)
 #endif
 
 unlock:
+	br_multicast_update_active(brmctx);
 	spin_unlock_bh(&brmctx->br->multicast_lock);
 
 	return 0;
-- 
2.53.0

[PATCH net-next v4 08/14] net: bridge: mcast: track active state, if snooping is enabled

[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.

No functional change for the fast/data path yet.

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

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 66c59953d301..d963be8679b5 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -748,12 +748,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 IPv6 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 f37a18f3713c..cdc921b97243 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1124,6 +1124,7 @@ static void br_multicast_notify_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 link-local address is configured on the
  *   bridge
@@ -1141,6 +1142,9 @@ static void 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;
+
 	br_ip4_multicast_update_active(brmctx, force_inactive);
 	br_ip6_multicast_update_active(brmctx, force_inactive);
 
@@ -1355,6 +1359,12 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 	return NULL;
 }
 
+static void br_multicast_toggle_enabled(struct net_bridge *br, bool on)
+{
+	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on);
+	br_multicast_update_active(&br->multicast_ctx);
+}
+
 struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br,
 						    struct br_ip *group)
 {
@@ -1368,7 +1378,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);
 		return ERR_PTR(-E2BIG);
 	}
 
@@ -4853,7 +4863,8 @@ int br_multicast_toggle(struct net_bridge *br, unsigned long val,
 	if (err)
 		goto unlock;
 
-	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, !!val);
+	br_multicast_toggle_enabled(br, !!val);
+
 	if (!br_opt_get(br, BROPT_MULTICAST_ENABLED)) {
 		change_snoopers = true;
 		br_multicast_del_grps(br);
-- 
2.53.0

[PATCH net-next v4 09/14] net: bridge: mcast: track active state, VLAN snooping

[Linus Lüssing]

If VLAN aware multicast snooping is enabled then we need to perform a
few extra checks to figure out if multicast snooping is actually enabled
for a specific VLAN, as there is then an additional per VLAN multicast
snooping toggle.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/br_multicast.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index cdc921b97243..1059984d8147 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1142,9 +1142,31 @@ static void 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))
+	if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) {
 		force_inactive = true;
+		goto update;
+	}
 
+	if (br_opt_get(brmctx->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED)) {
+		/* with per-vlan snooping enabled there is an extra per-vlan
+		 * toggle to enable/disable snooping which we must check
+		 */
+		if (br_multicast_ctx_vlan_global_disabled(brmctx))
+			force_inactive = true;
+		/* with per-vlan snooping enabled the non-vlan multicast
+		 * snooping context is inactive
+		 */
+		else if (!br_multicast_ctx_is_vlan(brmctx))
+			force_inactive = true;
+	} else {
+		/* with per-vlan snooping disabled a vlan multicast
+		 * snooping context is inactive
+		 */
+		if (br_multicast_ctx_is_vlan(brmctx))
+			force_inactive = true;
+	}
+
+update:
 	br_ip4_multicast_update_active(brmctx, force_inactive);
 	br_ip6_multicast_update_active(brmctx, force_inactive);
 
@@ -4505,6 +4527,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);
 
 		if (on)
 			__br_multicast_open(&vlan->br_mcast_ctx);
-- 
2.53.0

[PATCH net-next v4 10/14] net: bridge: mcast: track active state, bridge up/down

[Linus Lüssing]

This is mainly for switchdev and DSA later: To ensure that we switch
to inactive before destroying a bridge interface. A switchdev/DSA driver
might have allocated resources after we switched to an enabled multicast
active state. This gives switchdev/DSA drivers a chance to free these
resources again when we destroy the bridge (later).

Putting it into the ndo_stop / bridge interface down part instead of the
ndo_uninit / bridge destroy part though for a better semantic match. If
the bridge interface is down / stopped then it is also inactive.

No functional change for the fast/data path.

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

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d963be8679b5..679368784643 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -748,14 +748,14 @@ enum in6_addr_gen_mode {
  * @IFLA_BR_MCAST_ACTIVE_V4
  *   Bridge IPv4 mcast active state, read only.
  *
- *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an IGMP querier is present,
- *   0 otherwise.
+ *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled, an IGMP querier is present
+ *   and the bridge interface is up, 0 otherwise.
  *
  * @IFLA_BR_MCAST_ACTIVE_V6
  *   Bridge IPv6 mcast active state, read only.
  *
- *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an MLD querier is present,
- *   0 otherwise.
+ *   1 if *IFLA_BR_MCAST_SNOOPING* is enabled, an MLD querier is present
+ *   and the bridge interface is up, 0 otherwise.
  */
 enum {
 	IFLA_BR_UNSPEC,
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 1059984d8147..cb78f9555db6 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1124,6 +1124,7 @@ static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
  *
  * The multicast active state is set, per protocol family, if:
  *
+ * - the bridge interface is up
  * - multicast snooping is enabled
  * - an IGMP/MLD querier is present
  * - for own IPv6 MLD querier: an IPv6 link-local address is configured on the
@@ -1142,6 +1143,11 @@ static void br_multicast_update_active(struct net_bridge_mcast *brmctx)
 
 	lockdep_assert_held_once(&brmctx->br->multicast_lock);
 
+	if (!netif_running(brmctx->br->dev)) {
+		force_inactive = true;
+		goto update;
+	}
+
 	if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) {
 		force_inactive = true;
 		goto update;
@@ -4431,6 +4437,9 @@ 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
+
+	/* bridge interface is up, maybe set multicast state to active */
+	br_multicast_update_active(brmctx);
 }
 
 static void br_multicast_open_locked(struct net_bridge *br)
@@ -4478,6 +4487,9 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
 	timer_shutdown(&brmctx->ip6_other_query.delay_timer);
 	timer_shutdown(&brmctx->ip6_own_query.timer);
 #endif
+
+	/* bridge interface is down, set multicast state to inactive */
+	br_multicast_update_active(brmctx);
 }
 
 void br_multicast_update_vlan_mcast_ctx(struct net_bridge_vlan *v, u8 state)
-- 
2.53.0

[PATCH net-next v4 11/14] net: bridge: mcast: track active state, prepare for outside lock reads

[Linus Lüssing]

We are updating ip{4,6}_active and check all variables their state relies
on while holding the bridge multicast spinlock. However we are going to
read it without this lock in a follow-up commit on fast/data path, too.

As these variables are only booleans this shouldn't be a problem,
ip{4,6}_active will be loaded and stored atomically. And those
read sides should converge eventually. But to allow tooling to verify
this use the READ_ONCE() and WRITE_ONCE macros.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/br_multicast.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index cb78f9555db6..6faa484dede7 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1085,9 +1085,10 @@ static void br_ip4_multicast_update_active(struct net_bridge_mcast *brmctx,
 					   bool force_inactive)
 {
 	if (force_inactive)
-		brmctx->ip4_active = false;
+		WRITE_ONCE(brmctx->ip4_active, false);
 	else
-		brmctx->ip4_active = br_ip4_multicast_querier_exists(brmctx);
+		WRITE_ONCE(brmctx->ip4_active,
+			   br_ip4_multicast_querier_exists(brmctx));
 }
 
 static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx,
@@ -1095,23 +1096,26 @@ static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx,
 {
 #if IS_ENABLED(CONFIG_IPV6)
 	if (force_inactive)
-		brmctx->ip6_active = false;
+		WRITE_ONCE(brmctx->ip6_active, false);
 	else
-		brmctx->ip6_active = br_ip6_multicast_querier_exists(brmctx);
+		WRITE_ONCE(brmctx->ip6_active,
+			   br_ip6_multicast_querier_exists(brmctx));
 #endif
 }
 
 static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
 				       bool ip4_active_old, bool ip6_active_old)
 {
-	if (brmctx->ip4_active == ip4_active_old &&
-	    brmctx->ip6_active == ip6_active_old)
+	int ip4_active = brmctx->ip4_active;
+	int ip6_active = brmctx->ip6_active;
+
+	if (ip4_active == ip4_active_old &&
+	    ip6_active == ip6_active_old)
 		return;
 
 	br_debug(brmctx->br, "mc_active changed, vid: %i: v4: %i->%i, v6: %i->%i\n",
 		 brmctx->vlan ? brmctx->vlan->vid : -1,
-		 ip4_active_old, brmctx->ip4_active,
-		 ip6_active_old, brmctx->ip6_active);
+		 ip4_active_old, ip4_active, ip6_active_old, ip6_active);
 }
 
 /**
@@ -4269,7 +4273,7 @@ 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;
+	WRITE_ONCE(brmctx->ip4_active, 0);
 	seqcount_spinlock_init(&brmctx->ip4_querier.seq, &br->multicast_lock);
 	brmctx->multicast_igmp_version = 2;
 #if IS_ENABLED(CONFIG_IPV6)
@@ -4277,7 +4281,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
 	brmctx->ip6_querier.port_ifidx = 0;
 	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
 #endif
-	brmctx->ip6_active = 0;
+	WRITE_ONCE(brmctx->ip6_active, 0);
 
 	timer_setup(&brmctx->ip4_mc_router_timer, NULL, 0);
 	timer_setup(&brmctx->ip4_other_query.timer, NULL, 0);
-- 
2.53.0

[PATCH net-next v4 12/14] net: bridge: mcast: use combined active state in netlink

[Linus Lüssing]

Use the new multicast ip{4,6}_active variables for the netlink output.
The result for the user should be the same. But this allows us to check
that from userspace tests, to ensure that ip{4,6}_active behave as intended
for the upcoming fast/data path changes.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 net/bridge/br_netlink.c      | 14 ++------------
 net/bridge/br_private.h      |  2 +-
 net/bridge/br_vlan_options.c | 23 +++++------------------
 3 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 7fff59c3bf76..e79fddf747f8 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1643,8 +1643,6 @@ static size_t br_get_size(const struct net_device *brdev)
 
 static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
 {
-	struct ethhdr eth6 __maybe_unused = { .h_proto = htons(ETH_P_IPV6) };
-	struct ethhdr eth4 __maybe_unused = { .h_proto = htons(ETH_P_IP) };
 	struct net_bridge *br = netdev_priv(brdev);
 	u32 forward_delay = jiffies_to_clock_t(br->forward_delay);
 	u32 hello_time = jiffies_to_clock_t(br->hello_time);
@@ -1724,11 +1722,7 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
 	    nla_put_u8(skb, IFLA_BR_MCAST_IGMP_VERSION,
 		       br->multicast_ctx.multicast_igmp_version) ||
 	    nla_put_u8(skb, IFLA_BR_MCAST_ACTIVE_V4,
-		       netif_running(brdev) &&
-		       br_opt_get(br, BROPT_MULTICAST_ENABLED) &&
-		       !br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
-		       br_multicast_querier_exists(&br->multicast_ctx, &eth4,
-						   NULL)) ||
+		       READ_ONCE(br->multicast_ctx.ip4_active)) ||
 	    br_multicast_dump_querier_state(skb, &br->multicast_ctx,
 					    IFLA_BR_MCAST_QUERIER_STATE))
 		return -EMSGSIZE;
@@ -1736,11 +1730,7 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
 	if (nla_put_u8(skb, IFLA_BR_MCAST_MLD_VERSION,
 		       br->multicast_ctx.multicast_mld_version) ||
 	    nla_put_u8(skb, IFLA_BR_MCAST_ACTIVE_V6,
-		       netif_running(brdev) &&
-		       br_opt_get(br, BROPT_MULTICAST_ENABLED) &&
-		       !br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
-		       br_multicast_querier_exists(&br->multicast_ctx, &eth6,
-						   NULL)))
+		       READ_ONCE(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_private.h b/net/bridge/br_private.h
index 8d1c869eb522..2bdfc93499fc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1909,7 +1909,7 @@ int br_vlan_rtm_process_global_options(struct net_device *dev,
 bool br_vlan_global_opts_can_enter_range(const struct net_bridge_vlan *v_curr,
 					 const struct net_bridge_vlan *r_end);
 bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
-			      struct net_bridge_vlan *v_opts);
+			      const struct net_bridge_vlan *v_opts);
 
 /* vlan state manipulation helpers using *_ONCE to annotate lock-free access,
  * while br_vlan_set_state() may access data protected by multicast_lock.
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 06045d68a2d1..47ad35583a3f 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -367,13 +367,10 @@ bool br_vlan_global_opts_can_enter_range(const struct net_bridge_vlan *v_curr,
 }
 
 bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
-			      struct net_bridge_vlan *v_opts)
+			      const struct net_bridge_vlan *v_opts)
 {
-	struct ethhdr eth6 __maybe_unused = { .h_proto = htons(ETH_P_IPV6) };
-	struct ethhdr eth4 __maybe_unused = { .h_proto = htons(ETH_P_IP) };
 	struct nlattr *nest2 __maybe_unused;
 	u64 clockval __maybe_unused;
-	bool mcact __maybe_unused;
 	struct nlattr *nest;
 
 	nest = nla_nest_start(skb, BRIDGE_VLANDB_GLOBAL_OPTIONS);
@@ -388,17 +385,12 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 		goto out_err;
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
-	mcact = netif_running(v_opts->br->dev) &&
-		br_opt_get(v_opts->br, BROPT_MULTICAST_ENABLED) &&
-		br_opt_get(v_opts->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
-		!br_multicast_ctx_vlan_global_disabled(&v_opts->br_mcast_ctx) &&
-		br_multicast_querier_exists(&v_opts->br_mcast_ctx, &eth4, NULL);
-
 	if (nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_SNOOPING,
 		       !!(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, mcact) ||
+	    nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4,
+		       READ_ONCE(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,
@@ -452,15 +444,10 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 	}
 
 #if IS_ENABLED(CONFIG_IPV6)
-	mcact = netif_running(v_opts->br->dev) &&
-		br_opt_get(v_opts->br, BROPT_MULTICAST_ENABLED) &&
-		br_opt_get(v_opts->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) &&
-		!br_multicast_ctx_vlan_global_disabled(&v_opts->br_mcast_ctx) &&
-		br_multicast_querier_exists(&v_opts->br_mcast_ctx, &eth6, NULL);
-
 	if (nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION,
 		       v_opts->br_mcast_ctx.multicast_mld_version) ||
-	    nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6, mcact))
+	    nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6,
+		       READ_ONCE(v_opts->br_mcast_ctx.ip6_active)))
 		goto out_err;
 #endif
 #endif
-- 
2.53.0

[PATCH net-next v4 13/14] net: bridge: mcast: use combined active state in fast/data path

[Linus Lüssing]

As the multicast active state variable is now always up to date and
functionally equivalent to our manual, extensive checks in fast path
we can just use this state variable in fast path, too. This allows to
save some CPU cycles for every multicast packet in the fast/data path.

Next to using brmctx->ip4_active / brmctx->ip6_active in fast path this
mostly just moves some code around to not expose it via br_private.h
anymore. While at it now also passing the ethernet protocol number
directly, instead of a pointer into the ethernet header.

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

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index f7502e62dd35..65e03e4c09bb 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -102,7 +102,8 @@ 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)->h_proto,
+						 mdst))
 			br_multicast_flood(mdst, skb, brmctx, false, true);
 		else
 			br_flood(br, skb, BR_PKT_MULTICAST, false, true, vid);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 2cbae0f9ae1f..88654a559f09 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -187,7 +187,8 @@ 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)->h_proto,
+						 mdst)) {
 			if ((mdst && mdst->host_joined) ||
 			    br_multicast_is_router(brmctx, skb) ||
 			    br->dev->flags & IFF_ALLMULTI) {
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 6faa484dede7..aa2edb63a97b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1067,6 +1067,26 @@ 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,
+			      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_ip4_multicast_querier_exists(struct net_bridge_mcast *brmctx)
 {
 	return __br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query,
@@ -1081,6 +1101,21 @@ static bool br_ip6_multicast_querier_exists(struct net_bridge_mcast *brmctx)
 }
 #endif
 
+static bool
+br_multicast_querier_exists(struct net_bridge_mcast *brmctx, u16 proto)
+{
+	switch (proto) {
+	case ETH_P_IP:
+		return br_ip4_multicast_querier_exists(brmctx);
+#if IS_ENABLED(CONFIG_IPV6)
+	case ETH_P_IPV6:
+		return br_ip6_multicast_querier_exists(brmctx);
+#endif
+	default:
+		return false;
+	}
+}
+
 static void br_ip4_multicast_update_active(struct net_bridge_mcast *brmctx,
 					   bool force_inactive)
 {
@@ -5159,7 +5194,6 @@ bool br_multicast_has_querier_anywhere(struct net_device *dev, int proto)
 {
 	struct net_bridge *br;
 	struct net_bridge_port *port;
-	struct ethhdr eth;
 	bool ret = false;
 
 	rcu_read_lock();
@@ -5172,10 +5206,7 @@ bool br_multicast_has_querier_anywhere(struct net_device *dev, int proto)
 
 	br = port->br;
 
-	memset(&eth, 0, sizeof(eth));
-	eth.h_proto = htons(proto);
-
-	ret = br_multicast_querier_exists(&br->multicast_ctx, &eth, NULL);
+	ret = br_multicast_querier_exists(&br->multicast_ctx, proto);
 
 unlock:
 	rcu_read_unlock();
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2bdfc93499fc..0d1d39081acd 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1159,37 +1159,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)
+br_multicast_snooping_active(const struct net_bridge_mcast *brmctx,
+			     __be16 eth_proto,
+			     const struct net_bridge_mdb_entry *mdb)
 {
-	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)
-{
-	switch (eth->h_proto) {
-	case (htons(ETH_P_IP)):
-		return __br_multicast_querier_exists(brmctx,
-			&brmctx->ip4_other_query, false);
+	switch (eth_proto) {
+	case htons(ETH_P_IP):
+		return READ_ONCE(brmctx->ip4_active);
 #if IS_ENABLED(CONFIG_IPV6)
-	case (htons(ETH_P_IPV6)):
-		return __br_multicast_querier_exists(brmctx,
-			&brmctx->ip6_other_query, true);
+	case htons(ETH_P_IPV6):
+		return READ_ONCE(brmctx->ip6_active);
 #endif
 	default:
 		return !!mdb && br_group_is_l2(&mdb->addr);
@@ -1458,9 +1437,10 @@ static inline bool br_multicast_is_router(struct net_bridge_mcast *brmctx,
 	return false;
 }
 
-static inline bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx,
-					       struct ethhdr *eth,
-					       const struct net_bridge_mdb_entry *mdb)
+static inline bool
+br_multicast_snooping_active(const struct net_bridge_mcast *brmctx,
+			     __be16 eth_proto,
+			     const struct net_bridge_mdb_entry *mdb)
 {
 	return false;
 }
-- 
2.53.0

[PATCH net-next v4 14/14] net: bridge: mcast: add inactive state assertions

[Linus Lüssing]

To avoid packetloss and as it is very hard from a user's perspective to
debug multicast snooping related issues it is even more crucial to properly
switch from an active to an inactive multicast snooping state than the
other way around.

Therefore adding a few kernel warnings if any of our assertions to be in
an inactive state would fail.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/bridge/br_multicast.c | 45 +++++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index aa2edb63a97b..f4b6ea674988 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1426,10 +1426,31 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx,
 	return NULL;
 }
 
+static void br_ip4_multicast_assert_inactive(struct net_bridge_mcast *brmctx)
+{
+	WARN_ON_ONCE(br_multicast_snooping_active(brmctx, htons(ETH_P_IP),
+						  NULL));
+}
+
+static void br_ip6_multicast_assert_inactive(struct net_bridge_mcast *brmctx)
+{
+	WARN_ON_ONCE(br_multicast_snooping_active(brmctx, htons(ETH_P_IPV6),
+						  NULL));
+}
+
+static void br_multicast_assert_inactive(struct net_bridge_mcast *brmctx)
+{
+	br_ip4_multicast_assert_inactive(brmctx);
+	br_ip6_multicast_assert_inactive(brmctx);
+}
+
 static void br_multicast_toggle_enabled(struct net_bridge *br, bool on)
 {
 	br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on);
 	br_multicast_update_active(&br->multicast_ctx);
+
+	if (!on)
+		br_multicast_assert_inactive(&br->multicast_ctx);
 }
 
 struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br,
@@ -1899,9 +1920,7 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
 					 struct bridge_mcast_own_query *query,
 					 struct timer_list *timer)
 {
-	spin_lock(&brmctx->br->multicast_lock);
-	if (br_multicast_is_stopping(brmctx->br, timer) ||
-	    br_multicast_ctx_vlan_global_disabled(brmctx) ||
+	if (br_multicast_ctx_vlan_global_disabled(brmctx) ||
 	    !br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED))
 		goto out;
 
@@ -1912,7 +1931,6 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
 	 * if our own querier is disabled, too
 	 */
 	br_multicast_update_active(brmctx);
-	spin_unlock(&brmctx->br->multicast_lock);
 }
 
 static void br_ip4_multicast_querier_expired(struct timer_list *t)
@@ -1920,7 +1938,16 @@ static void br_ip4_multicast_querier_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
 							     ip4_other_query.timer);
 
+	spin_lock(&brmctx->br->multicast_lock);
+	if (br_multicast_is_stopping(brmctx->br, t))
+		goto out;
+
 	br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query, t);
+
+	if (!brmctx->multicast_querier)
+		br_ip4_multicast_assert_inactive(brmctx);
+out:
+	spin_unlock(&brmctx->br->multicast_lock);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -1929,7 +1956,16 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t)
 	struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t,
 							     ip6_other_query.timer);
 
+	spin_lock(&brmctx->br->multicast_lock);
+	if (br_multicast_is_stopping(brmctx->br, t))
+		goto out;
+
 	br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query, t);
+
+	if (!brmctx->multicast_querier)
+		br_ip6_multicast_assert_inactive(brmctx);
+out:
+	spin_unlock(&brmctx->br->multicast_lock);
 }
 #endif
 
@@ -4529,6 +4565,7 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
 
 	/* bridge interface is down, set multicast state to inactive */
 	br_multicast_update_active(brmctx);
+	br_multicast_assert_inactive(brmctx);
 }
 
 void br_multicast_update_vlan_mcast_ctx(struct net_bridge_vlan *v, u8 state)
-- 
2.53.0

Re: [PATCH net-next v4 09/14] net: bridge: mcast: track active state, VLAN snooping

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:43AM +0100, Linus Lüssing wrote:
> If VLAN aware multicast snooping is enabled then we need to perform a
> few extra checks to figure out if multicast snooping is actually enabled
> for a specific VLAN, as there is then an additional per VLAN multicast
> snooping toggle.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/bridge/br_multicast.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index cdc921b97243..1059984d8147 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1142,9 +1142,31 @@ static void 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))
> +	if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) {
>  		force_inactive = true;
> +		goto update;
> +	}
>  
> +	if (br_opt_get(brmctx->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED)) {
> +		/* with per-vlan snooping enabled there is an extra per-vlan
> +		 * toggle to enable/disable snooping which we must check
> +		 */
> +		if (br_multicast_ctx_vlan_global_disabled(brmctx))
> +			force_inactive = true;
> +		/* with per-vlan snooping enabled the non-vlan multicast
> +		 * snooping context is inactive
> +		 */
> +		else if (!br_multicast_ctx_is_vlan(brmctx))
> +			force_inactive = true;
> +	} else {
> +		/* with per-vlan snooping disabled a vlan multicast
> +		 * snooping context is inactive
> +		 */
> +		if (br_multicast_ctx_is_vlan(brmctx))
> +			force_inactive = true;
> +	}
> +
> +update:
>  	br_ip4_multicast_update_active(brmctx, force_inactive);
>  	br_ip6_multicast_update_active(brmctx, force_inactive);
>  
> @@ -4505,6 +4527,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);
>  
>  		if (on)
>  			__br_multicast_open(&vlan->br_mcast_ctx);

I believe the active state is not disabled when snooping is disabled on
a specific VLAN. AFAICT that's because br_multicast_update_active() is
only called when the VLAN is actually used by the bridge (i.e.,
"brentry"). Can be reproduced with [1]. Please add it as a test case in
patch #2.

Will continue tomorrow.

[1]
#!/bin/bash

for ns in ns0 ns1; do
	ip netns del $ns &> /dev/null
	ip netns add $ns
	ip -n $ns link set dev lo up
done

ip -n ns0 link add name veth0 type veth peer name veth1 netns ns1

ip -n ns0 link add name br0 up type bridge vlan_filtering 1 mcast_snooping 1 mcast_vlan_snooping 1
ip -n ns1 link add name br1 up type bridge vlan_filtering 1 mcast_snooping 1 mcast_vlan_snooping 1

ip -n ns0 link set dev veth0 up master br0
ip -n ns1 link set dev veth1 up master br1

bridge -n ns0 vlan add vid 10 dev veth0 master
bridge -n ns1 vlan add vid 10 dev veth1 master

sleep 1

bridge -n ns0 vlan global set vid 10 dev br0 mcast_snooping 1 mcast_query_response_interval 100 mcast_querier 1
bridge -n ns1 vlan global set vid 10 dev br1 mcast_snooping 1 mcast_query_response_interval 100 mcast_querier 0

sleep 1

bridge -n ns1 -j -p vlan global show dev br1 vid 10 | jq '.[]["vlans"][]["mcast_active_v4"]'

bridge -n ns1 vlan global set vid 10 dev br1 mcast_snooping 0
sleep 1
bridge -n ns1 -j -p vlan global show dev br1 vid 10 | jq '.[]["vlans"][]["mcast_active_v4"]'

Re: [PATCH net-next v4 03/14] net: bridge: mcast: avoid sleeping on bridge-down

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:37AM +0100, Linus Lüssing wrote:
> @@ -4111,29 +4123,40 @@ void br_multicast_ctx_init(struct net_bridge *br,
>  	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
>  #endif
>  
> -	timer_setup(&brmctx->ip4_mc_router_timer,
> -		    br_ip4_multicast_local_router_expired, 0);
> -	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);
> -	timer_setup(&brmctx->ip4_own_query.timer,
> -		    br_ip4_multicast_query_expired, 0);
> +	timer_setup(&brmctx->ip4_mc_router_timer, NULL, 0);
> +	timer_setup(&brmctx->ip4_other_query.timer, NULL, 0);
> +	timer_setup(&brmctx->ip4_other_query.delay_timer, NULL, 0);
> +	timer_setup(&brmctx->ip4_own_query.timer, NULL, 0);
>  #if IS_ENABLED(CONFIG_IPV6)
> -	timer_setup(&brmctx->ip6_mc_router_timer,
> -		    br_ip6_multicast_local_router_expired, 0);
> -	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);
> -	timer_setup(&brmctx->ip6_own_query.timer,
> -		    br_ip6_multicast_query_expired, 0);
> +	timer_setup(&brmctx->ip6_mc_router_timer, NULL, 0);
> +	timer_setup(&brmctx->ip6_other_query.timer, NULL, 0);
> +	timer_setup(&brmctx->ip6_other_query.delay_timer, NULL, 0);
> +	timer_setup(&brmctx->ip6_own_query.timer, NULL, 0);
>  #endif
> +
> +	/* for non-vlan multicast timer callbacks:
> +	 * they are set on br_multicast_open() instead
> +	 */
> +	if (!vlan)
> +		return;
> +
> +	spin_lock_bh(&br->multicast_lock);
> +	br_multicast_reset_timer_cbs(&vlan->br_mcast_ctx);
> +	spin_unlock_bh(&br->multicast_lock);

It wasn't clear to me why special treatment is needed for the per-VLAN
context and it's not explained in the commit message either. I suspect
that it's a workaround for the fact that br_multicast_reset_timer_cbs()
is only called for VLANs that were actually added on the bridge (i.e.,
"brentry"), whereas the "other query" timers are used even when the VLAN
entry is not used for filtering.

This is related to my comment on patch #9 and it should probably be
handled in br_multicast_toggle_vlan(), which is called both when
multicast snooping is toggled on a specific VLAN and on all the VLANs
(i.e., "mcast_vlan_snooping").

>  }

Re: [PATCH net-next v4 04/14] net: bridge: mcast: track active state, IGMP/MLD querier appearance

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:38AM +0100, Linus Lüssing wrote:
> @@ -4245,7 +4355,7 @@ void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx)
>  	brmctx->ip4_other_query.timer.function =
>  		br_ip4_multicast_querier_expired;
>  	brmctx->ip4_other_query.delay_timer.function =
> -		br_multicast_query_delay_expired;
> +		br_ip4_multicast_query_delay_expired;
>  	brmctx->ip4_own_query.timer.function =
>  		br_ip4_multicast_query_expired;
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -4254,7 +4364,7 @@ void br_multicast_reset_timer_cbs(struct net_bridge_mcast *brmctx)
>  	brmctx->ip6_other_query.timer.function =
>  		br_ip6_multicast_querier_expired;
>  	brmctx->ip6_other_query.delay_timer.function =
> -		br_multicast_query_delay_expired;
> +		br_ip6_multicast_query_delay_expired;
>  	brmctx->ip6_own_query.timer.function =
>  		br_ip6_multicast_query_expired;
>  #endif

Patch looks fine to me, but it will probably need to change a little
because of this hunk and my earlier comments

Re: [PATCH net-next v4 07/14] net: bridge: mcast: track active state, own MLD querier disappearance

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:41AM +0100, Linus Lüssing wrote:
> This change ensures that the new multicast active state variable is
> immediately unset if our internal IGMP/MLD querier was elected and
> now disabled.
> 
> If no IGMP/MLD querier exists on the link then we can't reliably receive
> IGMP/MLD reports and in turn can't ensure the completeness of our MDB
> anymore either.
> 
> No functional change for the fast/data path yet.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Please take care of the comment I left last time regarding the subject:

https://lore.kernel.org/netdev/20260208160907.GG154003@shredder/

Re: [PATCH net-next v4 08/14] net: bridge: mcast: track active state, if snooping is enabled

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:42AM +0100, 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.
> 
> No functional change for the fast/data path yet.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net-next v4 10/14] net: bridge: mcast: track active state, bridge up/down

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:44AM +0100, Linus Lüssing wrote:
> @@ -4478,6 +4487,9 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
>  	timer_shutdown(&brmctx->ip6_other_query.delay_timer);
>  	timer_shutdown(&brmctx->ip6_own_query.timer);
>  #endif
> +
> +	/* bridge interface is down, set multicast state to inactive */
> +	br_multicast_update_active(brmctx);
>  }

This suffers from the same problem that I already mentioned.
__br_multicast_stop() is only called for VLANs that were added on the
bridge. For the rest it does not update the active state. You can
reproduce with [1].

[1]
#!/bin/bash

for ns in ns0 ns1; do
	ip netns del $ns &> /dev/null
	ip netns add $ns
	ip -n $ns link set dev lo up
done

ip -n ns0 link add name veth0 type veth peer name veth1 netns ns1

ip -n ns0 link add name br0 up type bridge vlan_filtering 1 mcast_snooping 1 mcast_vlan_snooping 1
ip -n ns1 link add name br1 up type bridge vlan_filtering 1 mcast_snooping 1 mcast_vlan_snooping 1

ip -n ns0 link set dev veth0 up master br0
ip -n ns1 link set dev veth1 up master br1

bridge -n ns0 vlan add vid 10 dev veth0 master
bridge -n ns1 vlan add vid 10 dev veth1 master

sleep 5

bridge -n ns0 vlan global set vid 10 dev br0 mcast_snooping 1 mcast_query_response_interval 100 mcast_querier 1
bridge -n ns1 vlan global set vid 10 dev br1 mcast_snooping 1 mcast_query_response_interval 100 mcast_querier 0

sleep 5

bridge -n ns1 -j -p vlan global show dev br1 vid 10 | jq '.[]["vlans"][]["mcast_active_v4"]'

ip -n ns1 link set dev br1 down
sleep 5
bridge -n ns1 -j -p vlan global show dev br1 vid 10 | jq '.[]["vlans"][]["mcast_active_v4"]'

Re: [PATCH net-next v4 11/14] net: bridge: mcast: track active state, prepare for outside lock reads

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:45AM +0100, Linus Lüssing wrote:
> We are updating ip{4,6}_active and check all variables their state relies
> on while holding the bridge multicast spinlock. However we are going to
> read it without this lock in a follow-up commit on fast/data path, too.
> 
> As these variables are only booleans this shouldn't be a problem,
> ip{4,6}_active will be loaded and stored atomically. And those
> read sides should converge eventually. But to allow tooling to verify
> this use the READ_ONCE() and WRITE_ONCE macros.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/bridge/br_multicast.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index cb78f9555db6..6faa484dede7 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1085,9 +1085,10 @@ static void br_ip4_multicast_update_active(struct net_bridge_mcast *brmctx,
>  					   bool force_inactive)
>  {
>  	if (force_inactive)
> -		brmctx->ip4_active = false;
> +		WRITE_ONCE(brmctx->ip4_active, false);
>  	else
> -		brmctx->ip4_active = br_ip4_multicast_querier_exists(brmctx);
> +		WRITE_ONCE(brmctx->ip4_active,
> +			   br_ip4_multicast_querier_exists(brmctx));
>  }
>  
>  static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx,
> @@ -1095,23 +1096,26 @@ static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx,
>  {
>  #if IS_ENABLED(CONFIG_IPV6)
>  	if (force_inactive)
> -		brmctx->ip6_active = false;
> +		WRITE_ONCE(brmctx->ip6_active, false);
>  	else
> -		brmctx->ip6_active = br_ip6_multicast_querier_exists(brmctx);
> +		WRITE_ONCE(brmctx->ip6_active,
> +			   br_ip6_multicast_querier_exists(brmctx));
>  #endif
>  }
>  
>  static void br_multicast_notify_active(struct net_bridge_mcast *brmctx,
>  				       bool ip4_active_old, bool ip6_active_old)
>  {
> -	if (brmctx->ip4_active == ip4_active_old &&
> -	    brmctx->ip6_active == ip6_active_old)
> +	int ip4_active = brmctx->ip4_active;
> +	int ip6_active = brmctx->ip6_active;
> +
> +	if (ip4_active == ip4_active_old &&
> +	    ip6_active == ip6_active_old)
>  		return;
>  
>  	br_debug(brmctx->br, "mc_active changed, vid: %i: v4: %i->%i, v6: %i->%i\n",
>  		 brmctx->vlan ? brmctx->vlan->vid : -1,
> -		 ip4_active_old, brmctx->ip4_active,
> -		 ip6_active_old, brmctx->ip6_active);
> +		 ip4_active_old, ip4_active, ip6_active_old, ip6_active);
>  }

This hunk seems unnecessary? Pretty sure you can leave this function
as-is.

>  
>  /**
> @@ -4269,7 +4273,7 @@ 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;
> +	WRITE_ONCE(brmctx->ip4_active, 0);
>  	seqcount_spinlock_init(&brmctx->ip4_querier.seq, &br->multicast_lock);
>  	brmctx->multicast_igmp_version = 2;
>  #if IS_ENABLED(CONFIG_IPV6)
> @@ -4277,7 +4281,7 @@ void br_multicast_ctx_init(struct net_bridge *br,
>  	brmctx->ip6_querier.port_ifidx = 0;
>  	seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock);
>  #endif
> -	brmctx->ip6_active = 0;
> +	WRITE_ONCE(brmctx->ip6_active, 0);

Is this necessary? Who can read the active states while the multicast
context is still be initialized?

>  
>  	timer_setup(&brmctx->ip4_mc_router_timer, NULL, 0);
>  	timer_setup(&brmctx->ip4_other_query.timer, NULL, 0);
> -- 
> 2.53.0
> 

Re: [PATCH net-next v4 13/14] net: bridge: mcast: use combined active state in fast/data path

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:47AM +0100, Linus Lüssing wrote:
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 6faa484dede7..aa2edb63a97b 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1067,6 +1067,26 @@ 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,

Both can be 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_ip4_multicast_querier_exists(struct net_bridge_mcast *brmctx)

Same (br_ip6_multicast_querier_exists() as well)

>  {
>  	return __br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query,
> @@ -1081,6 +1101,21 @@ static bool br_ip6_multicast_querier_exists(struct net_bridge_mcast *brmctx)
>  }
>  #endif
>  
> +static bool
> +br_multicast_querier_exists(struct net_bridge_mcast *brmctx, u16 proto)

Same

> +{
> +	switch (proto) {
> +	case ETH_P_IP:
> +		return br_ip4_multicast_querier_exists(brmctx);
> +#if IS_ENABLED(CONFIG_IPV6)
> +	case ETH_P_IPV6:
> +		return br_ip6_multicast_querier_exists(brmctx);
> +#endif
> +	default:
> +		return false;
> +	}
> +}

[...]

> +br_multicast_snooping_active(const struct net_bridge_mcast *brmctx,
> +			     __be16 eth_proto,
> +			     const struct net_bridge_mdb_entry *mdb)
>  {
> -	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)
> -{
> -	switch (eth->h_proto) {
> -	case (htons(ETH_P_IP)):
> -		return __br_multicast_querier_exists(brmctx,
> -			&brmctx->ip4_other_query, false);
> +	switch (eth_proto) {
> +	case htons(ETH_P_IP):
> +		return READ_ONCE(brmctx->ip4_active);
>  #if IS_ENABLED(CONFIG_IPV6)

This can be removed given that 'ipv6_active' is always available

> -	case (htons(ETH_P_IPV6)):
> -		return __br_multicast_querier_exists(brmctx,
> -			&brmctx->ip6_other_query, true);
> +	case htons(ETH_P_IPV6):
> +		return READ_ONCE(brmctx->ip6_active);
>  #endif
>  	default:
>  		return !!mdb && br_group_is_l2(&mdb->addr);
> @@ -1458,9 +1437,10 @@ static inline bool br_multicast_is_router(struct net_bridge_mcast *brmctx,
>  	return false;
>  }

Re: [PATCH net-next v4 14/14] net: bridge: mcast: add inactive state assertions

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:48AM +0100, Linus Lüssing wrote:
> To avoid packetloss and as it is very hard from a user's perspective to
> debug multicast snooping related issues it is even more crucial to properly
> switch from an active to an inactive multicast snooping state than the
> other way around.
> 
> Therefore adding a few kernel warnings if any of our assertions to be in
> an inactive state would fail.

More assertions will be needed to cover the following cases:

1. Per-VLAN multicast snooping is turned off.
2. Multicast snooping is disabled on a specific VLAN.

__br_multicast_stop() is not called in these cases with the per-VLAN
context when the VLAN is not configured on the bridge interface.

Re: [PATCH net-next v4 10/14] net: bridge: mcast: track active state, bridge up/down

[Ido Schimmel]

On Sat, Mar 07, 2026 at 05:45:44AM +0100, Linus Lüssing wrote:
> @@ -4431,6 +4437,9 @@ 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
> +
> +	/* bridge interface is up, maybe set multicast state to active */
> +	br_multicast_update_active(brmctx);
>  }
>  
>  static void br_multicast_open_locked(struct net_bridge *br)
> @@ -4478,6 +4487,9 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx)
>  	timer_shutdown(&brmctx->ip6_other_query.delay_timer);
>  	timer_shutdown(&brmctx->ip6_own_query.timer);
>  #endif
> +
> +	/* bridge interface is down, set multicast state to inactive */

Noticed this while reviewing patch #14: The comments about the interface
being up / down are inaccurate when __br_multicast_{open,stop}() are
called for a per-VLAN multicast context