[PATCH net-next 0/4] net: bridge: vlan: fixes for vlan mcast contexts

[PATCH net-next 0/4] net: bridge: vlan: fixes for vlan mcast contexts

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov <nikolay@nvidia.com>

Hi,
These are four fixes for vlan multicast contexts. The first patch enables
mcast ctx snooping when adding already existing master vlans to be
consistent with the rest of the code. The second patch accounts for the
mcast ctx router ports when allocating skb for notification. The third
one fixes two suspicious rcu usages due to wrong vlan group helper, and
the fourth updates host vlan mcast state along with port mcast state.

Thanks,
 Nik


Nikolay Aleksandrov (4):
  net: bridge: vlan: enable mcast snooping for existing master vlans
  net: bridge: vlan: account for router port lists when notifying
  net: bridge: mcast: use the correct vlan group helper
  net: bridge: mcast: toggle also host vlan state in
    br_multicast_toggle_vlan

 net/bridge/br_mdb.c          | 30 ++++++++++++++++++++++++++++++
 net/bridge/br_multicast.c    | 10 +++++++---
 net/bridge/br_private.h      |  7 +------
 net/bridge/br_vlan.c         |  1 +
 net/bridge/br_vlan_options.c | 17 +++++++++--------
 5 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.31.1

[PATCH net-next 1/4] net: bridge: vlan: enable mcast snooping for existing master vlans

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov <nikolay@nvidia.com>

We always create a vlan with enabled mcast snooping, so when the user
turns on per-vlan mcast contexts they'll get consistent behaviour with
the current situation, but one place wasn't updated when a bridge/master
vlan which already exists (created due to port vlans) is being added as
real bridge vlan (BRIDGE_VLAN_INFO_BRENTRY). We need to enable mcast
snooping for that vlan when that happens.

Fixes: 7b54aaaf53cb ("net: bridge: multicast: add vlan state initialization and control")
Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
---
 net/bridge/br_vlan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index cbc922681a76..e25e288e7a85 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -694,6 +694,7 @@ static int br_vlan_add_existing(struct net_bridge *br,
 		vlan->flags |= BRIDGE_VLAN_INFO_BRENTRY;
 		vg->num_vlans++;
 		*changed = true;
+		br_multicast_toggle_one_vlan(vlan, true);
 	}
 
 	if (__vlan_add_flags(vlan, flags))
-- 
2.31.1

[PATCH net-next 2/4] net: bridge: vlan: account for router port lists when notifying

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov <nikolay@nvidia.com>

When sending a global vlan notification we should account for the number
of router ports when allocating the skb, otherwise we might end up
losing notifications.

Fixes: dc002875c22b ("net: bridge: vlan: use br_rports_fill_info() to export mcast router ports")
Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
---
 net/bridge/br_mdb.c          | 30 ++++++++++++++++++++++++++++++
 net/bridge/br_private.h      |  1 +
 net/bridge/br_vlan_options.c | 17 +++++++++--------
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 389ff3c1e9d9..0281453f7766 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -37,6 +37,36 @@ br_ip6_rports_get_timer(struct net_bridge_mcast_port *pmctx,
 #endif
 }
 
+static size_t __br_rports_one_size(void)
+{
+	return nla_total_size(sizeof(u32)) + /* MDBA_ROUTER_PORT */
+	       nla_total_size(sizeof(u32)) + /* MDBA_ROUTER_PATTR_TIMER */
+	       nla_total_size(sizeof(u8)) +  /* MDBA_ROUTER_PATTR_TYPE */
+	       nla_total_size(sizeof(u32)) + /* MDBA_ROUTER_PATTR_INET_TIMER */
+	       nla_total_size(sizeof(u32)) + /* MDBA_ROUTER_PATTR_INET6_TIMER */
+	       nla_total_size(sizeof(u32));  /* MDBA_ROUTER_PATTR_VID */
+}
+
+size_t br_rports_size(const struct net_bridge_mcast *brmctx)
+{
+	struct net_bridge_mcast_port *pmctx;
+	size_t size = nla_total_size(0); /* MDBA_ROUTER */
+
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(pmctx, &brmctx->ip4_mc_router_list,
+				 ip4_rlist)
+		size += __br_rports_one_size();
+
+#if IS_ENABLED(CONFIG_IPV6)
+	hlist_for_each_entry_rcu(pmctx, &brmctx->ip6_mc_router_list,
+				 ip6_rlist)
+		size += __br_rports_one_size();
+#endif
+	rcu_read_unlock();
+
+	return size;
+}
+
 int br_rports_fill_info(struct sk_buff *skb,
 			const struct net_bridge_mcast *brmctx)
 {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 9b1bf98a2c5a..df0fa246c80c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -952,6 +952,7 @@ int br_multicast_dump_querier_state(struct sk_buff *skb,
 				    const struct net_bridge_mcast *brmctx,
 				    int nest_attr);
 size_t br_multicast_querier_state_size(void);
+size_t br_rports_size(const struct net_bridge_mcast *brmctx);
 
 static inline bool br_group_is_l2(const struct br_ip *group)
 {
diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c
index 49dec53a4a74..a3b8a086284b 100644
--- a/net/bridge/br_vlan_options.c
+++ b/net/bridge/br_vlan_options.c
@@ -362,7 +362,7 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range,
 	return false;
 }
 
-static size_t rtnl_vlan_global_opts_nlmsg_size(void)
+static size_t rtnl_vlan_global_opts_nlmsg_size(const struct net_bridge_vlan *v)
 {
 	return NLMSG_ALIGN(sizeof(struct br_vlan_msg))
 		+ nla_total_size(0) /* BRIDGE_VLANDB_GLOBAL_OPTIONS */
@@ -382,6 +382,8 @@ static size_t rtnl_vlan_global_opts_nlmsg_size(void)
 		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_QUERIER */
 		+ nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_ROUTER */
 		+ br_multicast_querier_state_size() /* BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE */
+		+ nla_total_size(0) /* BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS */
+		+ br_rports_size(&v->br_mcast_ctx) /* BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS */
 #endif
 		+ nla_total_size(sizeof(u16)); /* BRIDGE_VLANDB_GOPTS_RANGE */
 }
@@ -398,7 +400,12 @@ static void br_vlan_global_opts_notify(const struct net_bridge *br,
 	/* right now notifications are done only with rtnl held */
 	ASSERT_RTNL();
 
-	skb = nlmsg_new(rtnl_vlan_global_opts_nlmsg_size(), GFP_KERNEL);
+	/* need to find the vlan due to flags/options */
+	v = br_vlan_find(br_vlan_group(br), vid);
+	if (!v)
+		return;
+
+	skb = nlmsg_new(rtnl_vlan_global_opts_nlmsg_size(v), GFP_KERNEL);
 	if (!skb)
 		goto out_err;
 
@@ -411,11 +418,6 @@ static void br_vlan_global_opts_notify(const struct net_bridge *br,
 	bvm->family = AF_BRIDGE;
 	bvm->ifindex = br->dev->ifindex;
 
-	/* need to find the vlan due to flags/options */
-	v = br_vlan_find(br_vlan_group(br), vid);
-	if (!v)
-		goto out_kfree;
-
 	if (!br_vlan_global_opts_fill(skb, vid, vid_range, v))
 		goto out_err;
 
@@ -425,7 +427,6 @@ static void br_vlan_global_opts_notify(const struct net_bridge *br,
 
 out_err:
 	rtnl_set_sk_err(dev_net(br->dev), RTNLGRP_BRVLAN, err);
-out_kfree:
 	kfree_skb(skb);
 }
 
-- 
2.31.1

[PATCH net-next 3/4] net: bridge: mcast: use the correct vlan group helper

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov <nikolay@nvidia.com>

When dereferencing the port vlan group we should use the rcu helper
instead of the one relying on rtnl. In br_multicast_pg_to_port_ctx the
entry cannot disappear as we hold the multicast lock and rcu as explained
in the comment above it.
For the same reason we're ok in br_multicast_start_querier.

 =============================
 WARNING: suspicious RCU usage
 5.14.0-rc5+ #429 Tainted: G        W
 -----------------------------
 net/bridge/br_private.h:1478 suspicious rcu_dereference_protected() usage!

 other info that might help us debug this:

 rcu_scheduler_active = 2, debug_locks = 1
 3 locks held by swapper/2/0:
  #0: ffff88822be85eb0 ((&p->timer)){+.-.}-{0:0}, at: call_timer_fn+0x5/0x2da
  #1: ffff88810b32f260 (&br->multicast_lock){+.-.}-{3:3}, at: br_multicast_port_group_expired+0x28/0x13d [bridge]
  #2: ffffffff824f6c80 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire.constprop.0+0x0/0x22 [bridge]

 stack backtrace:
 CPU: 2 PID: 0 Comm: swapper/2 Kdump: loaded Tainted: G        W         5.14.0-rc5+ #429
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-4.fc34 04/01/2014
 Call Trace:
  <IRQ>
  dump_stack_lvl+0x45/0x59
  nbp_vlan_group+0x3e/0x44 [bridge]
  br_multicast_pg_to_port_ctx+0xd6/0x10d [bridge]
  br_multicast_star_g_handle_mode+0xa1/0x2ce [bridge]
  ? netlink_broadcast+0xf/0x11
  ? nlmsg_notify+0x56/0x99
  ? br_mdb_notify+0x224/0x2e9 [bridge]
  ? br_multicast_del_pg+0x1dc/0x26d [bridge]
  br_multicast_del_pg+0x1dc/0x26d [bridge]
  br_multicast_port_group_expired+0xaa/0x13d [bridge]
  ? __grp_src_delete_marked.isra.0+0x35/0x35 [bridge]
  ? __grp_src_delete_marked.isra.0+0x35/0x35 [bridge]
  call_timer_fn+0x134/0x2da
  __run_timers+0x169/0x193
  run_timer_softirq+0x19/0x2d
  __do_softirq+0x1bc/0x42a
  __irq_exit_rcu+0x5c/0xb3
  irq_exit_rcu+0xa/0x12
  sysvec_apic_timer_interrupt+0x5e/0x75
  </IRQ>
  asm_sysvec_apic_timer_interrupt+0x12/0x20
 RIP: 0010:default_idle+0xc/0xd
 Code: e8 14 40 71 ff e8 10 b3 ff ff 4c 89 e2 48 89 ef 31 f6 5d 41 5c e9 a9 e8 c2 ff cc cc cc cc 0f 1f 44 00 00 e8 7f 55 65 ff fb f4 <c3> 0f 1f 44 00 00 55 65 48 8b 2c 25 40 6f 01 00 53 f0 80 4d 02 20
 RSP: 0018:ffff88810033bf00 EFLAGS: 00000206
 RAX: ffffffff819cf828 RBX: ffff888100328000 RCX: 0000000000000001
 RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff819cfa2d
 RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
 R10: ffff8881008302c0 R11: 00000000000006db R12: 0000000000000000
 R13: 0000000000000002 R14: 0000000000000000 R15: 0000000000000000
  ? __sched_text_end+0x4/0x4
  ? default_idle_call+0x15/0x7b
  default_idle_call+0x4d/0x7b
  do_idle+0x124/0x2a2
  cpu_startup_entry+0x1d/0x1f
  secondary_startup_64_no_verify+0xb0/0xbb

Fixes: 74edfd483de8 ("net: bridge: multicast: add helper to get port mcast context from port group")
Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
---
 net/bridge/br_multicast.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index e411dd814c58..c9f7f56eaf9b 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -221,7 +221,7 @@ br_multicast_pg_to_port_ctx(const struct net_bridge_port_group *pg)
 	 * can safely be used on return
 	 */
 	rcu_read_lock();
-	vlan = br_vlan_find(nbp_vlan_group(pg->key.port), pg->key.addr.vid);
+	vlan = br_vlan_find(nbp_vlan_group_rcu(pg->key.port), pg->key.addr.vid);
 	if (vlan && !br_multicast_port_ctx_vlan_disabled(&vlan->port_mcast_ctx))
 		pmctx = &vlan->port_mcast_ctx;
 	else
@@ -4329,7 +4329,8 @@ static void br_multicast_start_querier(struct net_bridge_mcast *brmctx,
 		if (br_multicast_ctx_is_vlan(brmctx)) {
 			struct net_bridge_vlan *vlan;
 
-			vlan = br_vlan_find(nbp_vlan_group(port), brmctx->vlan->vid);
+			vlan = br_vlan_find(nbp_vlan_group_rcu(port),
+					    brmctx->vlan->vid);
 			if (!vlan ||
 			    br_multicast_port_ctx_state_stopped(&vlan->port_mcast_ctx))
 				continue;
-- 
2.31.1

[PATCH net-next 4/4] net: bridge: mcast: toggle also host vlan state in br_multicast_toggle_vlan

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov <nikolay@nvidia.com>

When changing vlan mcast state by br_multicast_toggle_vlan it iterates
over all ports and enables/disables the port mcast ctx based on the new
state, but I forgot to update the host vlan (bridge master vlan entry)
with the new state so it will be left out. Also that function is not
used outside of br_multicast.c, so make it static.

Fixes: f4b7002a7076 ("net: bridge: add vlan mcast snooping knob")
Signed-off-by: Nikolay Aleksandrov <nikolay@nvidia.com>
---
 net/bridge/br_multicast.c | 5 ++++-
 net/bridge/br_private.h   | 6 ------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index c9f7f56eaf9b..16e686f5b9e9 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4074,7 +4074,7 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
 	}
 }
 
-void br_multicast_toggle_vlan(struct net_bridge_vlan *vlan, bool on)
+static void br_multicast_toggle_vlan(struct net_bridge_vlan *vlan, bool on)
 {
 	struct net_bridge_port *p;
 
@@ -4089,6 +4089,9 @@ void br_multicast_toggle_vlan(struct net_bridge_vlan *vlan, bool on)
 			continue;
 		br_multicast_toggle_one_vlan(vport, on);
 	}
+
+	if (br_vlan_is_brentry(vlan))
+		br_multicast_toggle_one_vlan(vlan, on);
 }
 
 int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index df0fa246c80c..21b292eb2b3e 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -938,7 +938,6 @@ void br_multicast_port_ctx_init(struct net_bridge_port *port,
 				struct net_bridge_mcast_port *pmctx);
 void br_multicast_port_ctx_deinit(struct net_bridge_mcast_port *pmctx);
 void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on);
-void br_multicast_toggle_vlan(struct net_bridge_vlan *vlan, bool on);
 int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on,
 				      struct netlink_ext_ack *extack);
 bool br_multicast_toggle_global_vlan(struct net_bridge_vlan *vlan, bool on);
@@ -1370,11 +1369,6 @@ static inline void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan,
 {
 }
 
-static inline void br_multicast_toggle_vlan(struct net_bridge_vlan *vlan,
-					    bool on)
-{
-}
-
 static inline int br_multicast_toggle_vlan_snooping(struct net_bridge *br,
 						    bool on,
 						    struct netlink_ext_ack *extack)
-- 
2.31.1

Re: [PATCH net-next 0/4] net: bridge: vlan: fixes for vlan mcast contexts

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Mon, 16 Aug 2021 17:57:03 +0300 you wrote:
> From: Nikolay Aleksandrov <nikolay@nvidia.com>
> 
> Hi,
> These are four fixes for vlan multicast contexts. The first patch enables
> mcast ctx snooping when adding already existing master vlans to be
> consistent with the rest of the code. The second patch accounts for the
> mcast ctx router ports when allocating skb for notification. The third
> one fixes two suspicious rcu usages due to wrong vlan group helper, and
> the fourth updates host vlan mcast state along with port mcast state.
> 
> [...]

Here is the summary with links:
  - [net-next,1/4] net: bridge: vlan: enable mcast snooping for existing master vlans
    https://git.kernel.org/netdev/net-next/c/b92dace38f8f
  - [net-next,2/4] net: bridge: vlan: account for router port lists when notifying
    https://git.kernel.org/netdev/net-next/c/05d6f38ec0a5
  - [net-next,3/4] net: bridge: mcast: use the correct vlan group helper
    https://git.kernel.org/netdev/net-next/c/3f0d14efe2fa
  - [net-next,4/4] net: bridge: mcast: toggle also host vlan state in br_multicast_toggle_vlan
    https://git.kernel.org/netdev/net-next/c/affce9a774ca

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html