mbox series

[net-next,0/2] mlxsw: Set port STP state on bridge enslavement

Message ID cover.1691498735.git.petrm@nvidia.com (mailing list archive)
Headers show
Series mlxsw: Set port STP state on bridge enslavement | expand

Message

Petr Machata Aug. 8, 2023, 1:18 p.m. UTC
When the first port joins a LAG that already has a bridge upper, an
instance of struct mlxsw_sp_bridge_port is created for the LAG to keep
track of it as a bridge port. The bridge_port's STP state is initialized to
BR_STATE_DISABLED. This made sense previously, because mlxsw would only
ever allow a port to join a LAG if the LAG had no uppers. Thus if a
bridge_port was instantiated, it must have been because the LAG as such is
joining a bridge, and the STP state is correspondingly disabled.

However as of commit 2c5ffe8d7226 ("mlxsw: spectrum: Permit enslavement to
netdevices with uppers"), mlxsw allows a port to join a LAG that is already
a member of a bridge. The STP state may be different than disabled in that
case. Initialize it properly by querying the actual state.

This bug may cause an issue as traffic on ports attached to a bridged LAG
gets dropped on ingress with discard_ingress_general counter bumped.

The above fix in patch #1. Patch #2 contains a selftest that would
sporadically reproduce the issue.

Petr Machata (2):
  mlxsw: Set port STP state on bridge enslavement
  selftests: mlxsw: router_bridge_lag: Add a new selftest

 .../mellanox/mlxsw/spectrum_switchdev.c       |  2 +-
 .../drivers/net/mlxsw/router_bridge_lag.sh    | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/drivers/net/mlxsw/router_bridge_lag.sh

Comments

Simon Horman Aug. 9, 2023, 2:50 p.m. UTC | #1
On Tue, Aug 08, 2023 at 03:18:14PM +0200, Petr Machata wrote:
> When the first port joins a LAG that already has a bridge upper, an
> instance of struct mlxsw_sp_bridge_port is created for the LAG to keep
> track of it as a bridge port. The bridge_port's STP state is initialized to
> BR_STATE_DISABLED. This made sense previously, because mlxsw would only
> ever allow a port to join a LAG if the LAG had no uppers. Thus if a
> bridge_port was instantiated, it must have been because the LAG as such is
> joining a bridge, and the STP state is correspondingly disabled.
> 
> However as of commit 2c5ffe8d7226 ("mlxsw: spectrum: Permit enslavement to
> netdevices with uppers"), mlxsw allows a port to join a LAG that is already
> a member of a bridge. The STP state may be different than disabled in that
> case. Initialize it properly by querying the actual state.
> 
> This bug may cause an issue as traffic on ports attached to a bridged LAG
> gets dropped on ingress with discard_ingress_general counter bumped.
> 
> The above fix in patch #1. Patch #2 contains a selftest that would
> sporadically reproduce the issue.
> 
> Petr Machata (2):
>   mlxsw: Set port STP state on bridge enslavement
>   selftests: mlxsw: router_bridge_lag: Add a new selftest

Reviewed-by: Simon Horman <horms@kernel.org>
patchwork-bot+netdevbpf@kernel.org Aug. 9, 2023, 10:40 p.m. UTC | #2
Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 8 Aug 2023 15:18:14 +0200 you wrote:
> When the first port joins a LAG that already has a bridge upper, an
> instance of struct mlxsw_sp_bridge_port is created for the LAG to keep
> track of it as a bridge port. The bridge_port's STP state is initialized to
> BR_STATE_DISABLED. This made sense previously, because mlxsw would only
> ever allow a port to join a LAG if the LAG had no uppers. Thus if a
> bridge_port was instantiated, it must have been because the LAG as such is
> joining a bridge, and the STP state is correspondingly disabled.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] mlxsw: Set port STP state on bridge enslavement
    https://git.kernel.org/netdev/net-next/c/a76ca8afd45a
  - [net-next,2/2] selftests: mlxsw: router_bridge_lag: Add a new selftest
    https://git.kernel.org/netdev/net-next/c/aae5bb8d18d8

You are awesome, thank you!