Re: [Bridge] [PATCH v2 net] bridge: Add extack warning when enabling STP in netns.

2023-07-14 Thread patchwork-bot+netdevbpf 
Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller :

On Wed, 12 Jul 2023 08:44:49 -0700 you wrote:
> When we create an L2 loop on a bridge in netns, we will see packets storm
> even if STP is enabled.
> 
>   # unshare -n
>   # ip link add br0 type bridge
>   # ip link add veth0 type veth peer name veth1
>   # ip link set veth0 master br0 up
>   # ip link set veth1 master br0 up
>   # ip link set br0 type bridge stp_state 1
>   # ip link set br0 up
>   # sleep 30
>   # ip -s link show br0
>   2: br0:  mtu 1500 qdisc noqueue state UP 
> mode DEFAULT group default qlen 1000
>   link/ether b6:61:98:1c:1c:b5 brd ff:ff:ff:ff:ff:ff
>   RX: bytes  packets  errors  dropped missed  mcast
>   956553768  12861249 0   0   0   12861249  <-. Keep
>   TX: bytes  packets  errors  dropped carrier collsns |  increasing
>   1027834119510   0   0   0 <-'   rapidly
> 
> [...]

Here is the summary with links:
  - [v2,net] bridge: Add extack warning when enabling STP in netns.
https://git.kernel.org/netdev/net/c/56a16035bb6e

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

Re: [Bridge] [PATCH v2 net] bridge: Add extack warning when enabling STP in netns.

2023-07-12 Thread Ido Schimmel 
On Wed, Jul 12, 2023 at 08:44:49AM -0700, Kuniyuki Iwashima wrote:
> When we create an L2 loop on a bridge in netns, we will see packets storm
> even if STP is enabled.
> 
>   # unshare -n
>   # ip link add br0 type bridge
>   # ip link add veth0 type veth peer name veth1
>   # ip link set veth0 master br0 up
>   # ip link set veth1 master br0 up
>   # ip link set br0 type bridge stp_state 1
>   # ip link set br0 up
>   # sleep 30
>   # ip -s link show br0
>   2: br0:  mtu 1500 qdisc noqueue state UP 
> mode DEFAULT group default qlen 1000
>   link/ether b6:61:98:1c:1c:b5 brd ff:ff:ff:ff:ff:ff
>   RX: bytes  packets  errors  dropped missed  mcast
>   956553768  12861249 0   0   0   12861249  <-. Keep
>   TX: bytes  packets  errors  dropped carrier collsns |  increasing
>   1027834119510   0   0   0 <-'   rapidly
> 
> This is because llc_rcv() drops all packets in non-root netns and BPDU
> is dropped.
> 
> Let's add extack warning when enabling STP in netns.
> 
>   # unshare -n
>   # ip link add br0 type bridge
>   # ip link set br0 type bridge stp_state 1
>   Warning: bridge: STP does not work in non-root netns.
> 
> Note this commit will be reverted later when we namespacify the whole LLC
> infra.
> 
> Fixes: e730c15519d0 ("[NET]: Make packet reception network namespace safe")
> Suggested-by: Harry Coin 
> Link: 
> https://lore.kernel.org/netdev/0f531295-e289-022d-5add-5ceffa0df...@quietfountain.com/
> Suggested-by: Ido Schimmel 
> Signed-off-by: Kuniyuki Iwashima 

Reviewed-by: Ido Schimmel

Re: [Bridge] [PATCH v2 net] bridge: Add extack warning when enabling STP in netns.

2023-07-12 Thread Nikolay Aleksandrov 
On 12/07/2023 18:44, Kuniyuki Iwashima wrote:
> When we create an L2 loop on a bridge in netns, we will see packets storm
> even if STP is enabled.
> 
>   # unshare -n
>   # ip link add br0 type bridge
>   # ip link add veth0 type veth peer name veth1
>   # ip link set veth0 master br0 up
>   # ip link set veth1 master br0 up
>   # ip link set br0 type bridge stp_state 1
>   # ip link set br0 up
>   # sleep 30
>   # ip -s link show br0
>   2: br0:  mtu 1500 qdisc noqueue state UP 
> mode DEFAULT group default qlen 1000
>   link/ether b6:61:98:1c:1c:b5 brd ff:ff:ff:ff:ff:ff
>   RX: bytes  packets  errors  dropped missed  mcast
>   956553768  12861249 0   0   0   12861249  <-. Keep
>   TX: bytes  packets  errors  dropped carrier collsns |  increasing
>   1027834119510   0   0   0 <-'   rapidly
> 
> This is because llc_rcv() drops all packets in non-root netns and BPDU
> is dropped.
> 
> Let's add extack warning when enabling STP in netns.
> 
>   # unshare -n
>   # ip link add br0 type bridge
>   # ip link set br0 type bridge stp_state 1
>   Warning: bridge: STP does not work in non-root netns.
> 
> Note this commit will be reverted later when we namespacify the whole LLC
> infra.
> 
> Fixes: e730c15519d0 ("[NET]: Make packet reception network namespace safe")
> Suggested-by: Harry Coin 
> Link: 
> https://lore.kernel.org/netdev/0f531295-e289-022d-5add-5ceffa0df...@quietfountain.com/
> Suggested-by: Ido Schimmel 
> Signed-off-by: Kuniyuki Iwashima 
> ---
> v2:
>   - Just add extack instead of returning error (Ido Schimmel)
> 
> v1: https://lore.kernel.org/netdev/20230711235415.92166-1-kun...@amazon.com/
> ---
>  net/bridge/br_stp_if.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index 75204d36d7f9..b65962682771 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -201,6 +201,9 @@ int br_stp_set_enabled(struct net_bridge *br, unsigned 
> long val,
>  {
>   ASSERT_RTNL();
>  
> + if (!net_eq(dev_net(br->dev), _net))
> + NL_SET_ERR_MSG_MOD(extack, "STP does not work in non-root 
> netns");
> +
>   if (br_mrp_enabled(br)) {
>   NL_SET_ERR_MSG_MOD(extack,
>  "STP can't be enabled if MRP is already 
> enabled");

Thanks,
Acked-by: Nikolay Aleksandrov

[Bridge] [PATCH v2 net] bridge: Add extack warning when enabling STP in netns.

2023-07-12 Thread Kuniyuki Iwashima via Bridge 
When we create an L2 loop on a bridge in netns, we will see packets storm
even if STP is enabled.

  # unshare -n
  # ip link add br0 type bridge
  # ip link add veth0 type veth peer name veth1
  # ip link set veth0 master br0 up
  # ip link set veth1 master br0 up
  # ip link set br0 type bridge stp_state 1
  # ip link set br0 up
  # sleep 30
  # ip -s link show br0
  2: br0:  mtu 1500 qdisc noqueue state UP 
mode DEFAULT group default qlen 1000
  link/ether b6:61:98:1c:1c:b5 brd ff:ff:ff:ff:ff:ff
  RX: bytes  packets  errors  dropped missed  mcast
  956553768  12861249 0   0   0   12861249  <-. Keep
  TX: bytes  packets  errors  dropped carrier collsns |  increasing
  1027834119510   0   0   0 <-'   rapidly

This is because llc_rcv() drops all packets in non-root netns and BPDU
is dropped.

Let's add extack warning when enabling STP in netns.

  # unshare -n
  # ip link add br0 type bridge
  # ip link set br0 type bridge stp_state 1
  Warning: bridge: STP does not work in non-root netns.

Note this commit will be reverted later when we namespacify the whole LLC
infra.

Fixes: e730c15519d0 ("[NET]: Make packet reception network namespace safe")
Suggested-by: Harry Coin 
Link: 
https://lore.kernel.org/netdev/0f531295-e289-022d-5add-5ceffa0df...@quietfountain.com/
Suggested-by: Ido Schimmel 
Signed-off-by: Kuniyuki Iwashima 
---
v2:
  - Just add extack instead of returning error (Ido Schimmel)

v1: https://lore.kernel.org/netdev/20230711235415.92166-1-kun...@amazon.com/
---
 net/bridge/br_stp_if.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 75204d36d7f9..b65962682771 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -201,6 +201,9 @@ int br_stp_set_enabled(struct net_bridge *br, unsigned long 
val,
 {
ASSERT_RTNL();
 
+   if (!net_eq(dev_net(br->dev), _net))
+   NL_SET_ERR_MSG_MOD(extack, "STP does not work in non-root 
netns");
+
if (br_mrp_enabled(br)) {
NL_SET_ERR_MSG_MOD(extack,
   "STP can't be enabled if MRP is already 
enabled");
-- 
2.30.2