netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Petr Machata <petrm@nvidia.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	<netdev@vger.kernel.org>
Cc: Ido Schimmel <idosch@nvidia.com>, Petr Machata <petrm@nvidia.com>,
	Danielle Ratson <danieller@nvidia.com>, <mlxsw@nvidia.com>
Subject: [PATCH net-next 00/11] mlxsw: Manage RIF across PVID changes
Date: Thu, 13 Jul 2023 18:15:23 +0200	[thread overview]
Message-ID: <cover.1689262695.git.petrm@nvidia.com> (raw)

The mlxsw driver currently makes the assumption that the user applies
configuration in a bottom-up manner. Thus netdevices need to be added to
the bridge before IP addresses are configured on that bridge or SVI added
on top of it. Enslaving a netdevice to another netdevice that already has
uppers is in fact forbidden by mlxsw for this reason. Despite this safety,
it is rather easy to get into situations where the offloaded configuration
is just plain wrong.

As an example, take a front panel port, configure an IP address: it gets a
RIF. Now enslave the port to the bridge, and the RIF is gone. Remove the
port from the bridge again, but the RIF never comes back. There is a number
of similar situations, where changing the configuration there and back
utterly breaks the offload.

The situation is going to be made better by implementing a range of replays
and post-hoc offloads.

In this patch set, address the ordering issues related to creation of
bridge RIFs. Currently, mlxsw has several shortcomings with regards to RIF
handling due to PVID changes:

- In order to cause RIF for a bridge device to be created, the user is
  expected first to set PVID, then to add an IP address. The reverse
  ordering is disallowed, which is not very user-friendly.

- When such bridge gets a VLAN upper whose VID was the same as the existing
  PVID, and this VLAN netdevice gets an IP address, a RIF is created for
  this netdevice. The new RIF is then assigned to the 802.1Q FID for the
  given VID. This results in a working configuration. However, then, when
  the VLAN netdevice is removed again, the RIF for the bridge itself is
  never reassociated to the PVID.

- PVID cannot be changed once the bridge has uppers. Presumably this is
  because the driver does not manage RIFs properly in face of PVID changes.
  However, as the previous point shows, it is still possible to get into
  invalid configurations.

This patch set addresses these issues and relaxes some of the ordering
requirements that mlxsw had. The patch set proceeds as follows:

- In patch #1, pass extack to mlxsw_sp_br_ban_rif_pvid_change()

- To relax ordering between setting PVID and adding an IP address to a
  bridge, mlxsw must be able to request that a RIF is created with a given
  VLAN ID, instead of trying to deduce it from the current netdevice
  settings, which do not reflect the user-requested values yet. This is
  done in patches #2 and #3.

- Similarly, mlxsw_sp_inetaddr_bridge_event() will need to make decisions
  based on the user-requested value of PVID, not the current value. Thus in
  patches #4 and #5, add a new argument which carries the requested PVID
  value.

- Finally in patch #6 relax the ban on PVID changes when a bridge has
  uppers. Instead, add the logic necessary for creation of a RIF as a
  result of PVID change.

- Relevant selftests are presented afterwards. In patch #7 a preparatory
  helper is added to lib.sh. Patches #8, #9, #10 and #11 include selftests
  themselves.

Petr Machata (11):
  mlxsw: spectrum_switchdev: Pass extack to
    mlxsw_sp_br_ban_rif_pvid_change()
  mlxsw: spectrum_router: Pass struct mlxsw_sp_rif_params to fid_get
  mlxsw: spectrum_router: Take VID for VLAN FIDs from RIF params
  mlxsw: spectrum_router: Adjust mlxsw_sp_inetaddr_vlan_event() coding
    style
  mlxsw: spectrum_router: mlxsw_sp_inetaddr_bridge_event: Add an
    argument
  mlxsw: spectrum_switchdev: Manage RIFs on PVID change
  selftests: forwarding: lib: Add ping6_, ping_test_fails()
  selftests: router_bridge: Add tests to remove and add PVID
  selftests: router_bridge_vlan: Add PVID change test
  selftests: router_bridge_vlan_upper_pvid: Add a new selftest
  selftests: router_bridge_pvid_vlan_upper: Add a new selftest

 .../ethernet/mellanox/mlxsw/spectrum_router.c | 169 +++++++++++++++--
 .../ethernet/mellanox/mlxsw/spectrum_router.h |   4 +
 .../mellanox/mlxsw/spectrum_switchdev.c       |  32 +---
 .../testing/selftests/net/forwarding/Makefile |   2 +
 tools/testing/selftests/net/forwarding/lib.sh |  18 ++
 .../selftests/net/forwarding/router_bridge.sh |  50 +++++
 .../router_bridge_pvid_vlan_upper.sh          | 155 ++++++++++++++++
 .../net/forwarding/router_bridge_vlan.sh      | 100 ++++++++--
 .../router_bridge_vlan_upper_pvid.sh          | 171 ++++++++++++++++++
 9 files changed, 643 insertions(+), 58 deletions(-)
 create mode 100755 tools/testing/selftests/net/forwarding/router_bridge_pvid_vlan_upper.sh
 create mode 100755 tools/testing/selftests/net/forwarding/router_bridge_vlan_upper_pvid.sh

-- 
2.40.1


             reply	other threads:[~2023-07-13 16:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-13 16:15 Petr Machata [this message]
2023-07-13 16:15 ` [PATCH net-next 01/11] mlxsw: spectrum_switchdev: Pass extack to mlxsw_sp_br_ban_rif_pvid_change() Petr Machata
2023-07-13 16:15 ` [PATCH net-next 02/11] mlxsw: spectrum_router: Pass struct mlxsw_sp_rif_params to fid_get Petr Machata
2023-07-13 16:15 ` [PATCH net-next 03/11] mlxsw: spectrum_router: Take VID for VLAN FIDs from RIF params Petr Machata
2023-07-13 16:15 ` [PATCH net-next 04/11] mlxsw: spectrum_router: Adjust mlxsw_sp_inetaddr_vlan_event() coding style Petr Machata
2023-07-13 16:15 ` [PATCH net-next 05/11] mlxsw: spectrum_router: mlxsw_sp_inetaddr_bridge_event: Add an argument Petr Machata
2023-07-13 16:15 ` [PATCH net-next 06/11] mlxsw: spectrum_switchdev: Manage RIFs on PVID change Petr Machata
2023-07-13 16:15 ` [PATCH net-next 07/11] selftests: forwarding: lib: Add ping6_, ping_test_fails() Petr Machata
2023-07-13 16:15 ` [PATCH net-next 08/11] selftests: router_bridge: Add tests to remove and add PVID Petr Machata
2023-07-13 16:15 ` [PATCH net-next 09/11] selftests: router_bridge_vlan: Add PVID change test Petr Machata
2023-07-13 16:15 ` [PATCH net-next 10/11] selftests: router_bridge_vlan_upper_pvid: Add a new selftest Petr Machata
2023-07-13 16:15 ` [PATCH net-next 11/11] selftests: router_bridge_pvid_vlan_upper: " Petr Machata
2023-07-14  9:30 ` [PATCH net-next 00/11] mlxsw: Manage RIF across PVID changes patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1689262695.git.petrm@nvidia.com \
    --to=petrm@nvidia.com \
    --cc=danieller@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=mlxsw@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).