Hi,
This series by Shay enables a LAG over functions that do not represent a
physical port - a "virtual LAG" (virt LAG) of VFs or SFs acting as nested
e-switch managers. New FW lets a VF/SF front the physical uplinks through
a multi-uplink e-switch; such a function reports num_lag_ports < 2 and
drives port selection via the e-switch (port_select_eswitch) rather than
the HW port-selection flow table. A virt LAG is therefore a
software-managed, shared-FDB MPESW LAG: the driver builds the LAG demux
and vport-LAG in software and skips the FW LAG commands.
The patches fall into three parts.
LAG code cleanup (patches 5, 6, 7, 8, 9)
- Finish removing the fixed MLX5_MAX_PORTS sizing the first series
began: convert the remaining structures to dynamic, port-count-based
allocation and inline iteration. A virt LAG's member count is
unrelated to the physical port count.
- Rename the LAG "pf" naming to "func", since a member is now any
function (PF, VF or SF), not necessarily a PF.
- Resolve the queue-affinity member indices only in that mode, so the
other port-select modes are not rejected for indices they never use.
Virtual LAG core (patches 1, 2, 3, 4, 10, 15)
- Distinguish VFs/SFs from their PF in the SW system image GUID, so they
group into their own LAG instead of with the PF.
- Select the MPESW master from the live member snapshot: VF/SF members
are dynamic and not marked by the devcom PAIR event, so the master is
chosen from the current membership (tie-broken by PCI BDF for VFs,
sfnum for SFs).
- Skip the phys-LAG prerequisites (port_select_flow_table,
create_lag_when_not_master_up, ...) for a virt LAG, and require
port_select_eswitch instead.
- Treat a virt LAG as a software-managed LAG (demux + vport-LAG in SW).
- Do not add a device to an already-active LAG.
- Enable LAG over functions that do not represent a physical port - the
gate that admits a virt LAG.
E-switch and representors (patches 11, 12, 13, 14)
- A dedicated match-metadata allocator for nested VF/SF e-switches, so
VFs/SFs sharing an FDB get unique source-port metadata.
- Match the master egress bounce rule by vport metadata.
- Drop the master egress rule while a software vport LAG is installed:
the SW emulation is a rule in the slave's egress ACL and cannot act on
a packet the master egress rule redirects after the FDB. Restore it
when the vport LAG is removed.
- Register a VF/SF e-switch-manager self-port with a switch-id so it
groups with the representors it manages.
Regards,
Tariq
Answers to sashiko comments:
net/mlx5: LAG, treat virt LAG as a software managed LAG
> -static int mlx5_lag_is_sw_lag(struct mlx5_core_dev *dev)
> +int mlx5_lag_is_sw_managed(struct mlx5_core_dev *dev)
> {
> - return mlx5_lag_is_sd(dev);
> + struct mlx5_lag *ldev = mlx5_lag_dev(dev);
Q: If dev is NULL, does mlx5_lag_dev(dev) NULL-deref dev->priv.lag? When
called from mlx5_lag_multipath_check_prereq() via the FIB notifier,
pf->dev can be NULL due to concurrent removal in mlx5_ldev_remove_mdev().
__mlx5_lag_is_sd() was safe (lockless pointer compare). Does this need a
NULL check?
[SD] mlx5_lag_is_sw_managed() assumes a valid dev pointer, like the rest
of the mlx5_lag_is_* APIs. It is true mlx5_lag_multipath_check_prereq()
can provide an invalid device pointer, but that is an existing bug there
which should be handled in its own patch.
net/mlx5: LAG, decouple iteration end-of-walk from MLX5_MAX_PORTS
> - if (i < MLX5_MAX_PORTS) {
> + if (i < ldev->ports) {
> slave = bond_slave_get_rcu(ndev_tmp);
Q: Could this loop-termination change break LAG activation, since
physical ports share the XArray ldev->pfs with VFs/SFs?
[SD] No. mlx5 creates and populates an ldev per sw_guid, and PFs have a
different sw_guid than VFs/SFs. Hence PFs cannot share an ldev with
VFs/SFs.
> - err = xa_alloc(&ldev->pfs, &idx, pf, XA_LIMIT(0, MLX5_MAX_PORTS - 1),
> + err = xa_alloc(&ldev->pfs, &idx, pf, XA_LIMIT(0, U8_MAX - 1),
Q: Physical members and Socket Direct secondaries share the xarray index
space. Can a physical member get an index >= MLX5_MAX_PORTS and make
mlx5_ldev_add_netdev() write beyond tracker.netdev_state[]?
[SD] Such HW doesn't exist; it needs LAG over SD with more than
MLX5_MAX_PORTS / 2 ports, and when it does MLX5_MAX_PORTS will be
increased. To protect a new HW running an old driver, a check that
num_lag_ports times the SD group size fits in MLX5_MAX_PORTS will be added
to mlx5_lag_is_supported() separately.
net/mlx5: LAG: replace fixed MLX5_MAX_PORTS arrays with dynamic allocation
> + ldev = mlx5_lag_dev(dev);
> + num_ports = ldev ? max_t(int, ldev->ports, 1) : 1;
> + mdev = kcalloc(num_ports, sizeof(mdev[0]), GFP_KERNEL);
Q: Could mlx5_lag_query_cong_counters() keep a freed ldev across this
sleeping allocation if teardown removes the association and drops the
last LAG reference before the query resumes?
[SD] No. The query runs on the IB device of dev, and dev leaves the LAG
only in its own teardown, after its IB device is unregistered.
net/mlx5: LAG, Drop MLX5_MAX_PORTS scratch array from device rescan
> + mlx5_lag_for_each(i, 0, ldev, filter) {
> + pf = mlx5_lag_pf(ldev, i);
> + ...
> + mlx5_rescan_drivers_locked(pf->dev);
Q: Is it safe to iterate the LAG PFs and deref pf->dev while ldev->lock is
dropped?
[SD] Valid. Membership can be removed by SD cleanup while ldev->lock
is dropped; this will be fixed in a separate patch.
Q: Does using the sparse XArray index directly as a dense-array index
cause OOB writes elsewhere? With Socket Direct the XArray index (i) for a
primary port can exceed ldev->ports, and it is used directly in
mlx5_infer_tx_affinity_mapping() (ports[idx]) and
mlx5_handle_changelowerstate_event() (tracker->netdev_state[idx]) - could
these overflow the heap/stack buffers?
[SD] All these buffers aren't used in LAG over SD; only MPESW is, and
MPESW doesn't use these APIs.
net/mlx5: E-switch, add match-metadata allocator for nested VF/SF eswitches
> + if (!mlx5_core_is_pf(esw->dev) && !mlx5_virt_lag_nonmember(esw->dev))
> + return mlx5_nested_esw_match_metadata_alloc(esw);
Q: mlx5_virt_lag_nonmember() is false when virt LAG is unsupported, so a
standalone non-PF eswitch without virt LAG support takes the nested path
and fails switchdev with -ENOSPC. Could it keep the PFNUM allocator?
[SD] A non-PF eswitch manager without virt LAG support doesn't exist at
the moment.
net/mlx5: LAG, enable LAG over functions that do not represent a physical port
> - if (group_id)
> + if (group_id || ldev->virt_lag)
> return 0;
Q: mlx5_lag_set_vports_agg_speed() iterates ports only, so a virt LAG gets
no aggregate vport speed, and its members skip the port-change notifier.
Is there an alternative virt LAG speed policy?
[SD] This is on purpose.
Q: members_show() uses mlx5_ldev_for_each(), so the debugfs members file
is empty for a virt LAG. Could it list all members?
[SD] Left as is for now, since all VFs in the system share the LAG.
Shay Drory (15):
net/mlx5: Distinguish VFs/SFs in the SW system image GUID for LAG
grouping
net/mlx5: LAG, select the MPESW master from the live member snapshot
net/mlx5: LAG, skip phys LAG prerequisites for a virt LAG
net/mlx5: LAG, treat virt LAG as a software managed LAG
net/mlx5: LAG, decouple iteration end-of-walk from MLX5_MAX_PORTS
net/mlx5: LAG, rename PF naming to fn
net/mlx5: LAG, resolve affinity indices only in queue-affinity mode
net/mlx5: LAG: replace fixed MLX5_MAX_PORTS arrays with dynamic
allocation
net/mlx5: LAG, drop MLX5_MAX_PORTS scratch array from device rescan
net/mlx5: LAG, do not add a device to an already-active LAG
net/mlx5: E-switch, add match-metadata allocator for nested VF/SF
eswitches
net/mlx5: E-switch, match master egress rule by vport metadata
net/mlx5: E-switch, drop master egress rule under a SW vport LAG
net/mlx5e: Register eswitch-manager VF/SF self-port with switch-id
net/mlx5: LAG, enable LAG over functions that do not represent a
physical port
.../ethernet/mellanox/mlx5/core/en/devlink.c | 5 +
.../net/ethernet/mellanox/mlx5/core/eswitch.h | 3 +-
.../mellanox/mlx5/core/eswitch_offloads.c | 154 ++++-
.../ethernet/mellanox/mlx5/core/lag/debugfs.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/lag/lag.c | 604 ++++++++++--------
.../net/ethernet/mellanox/mlx5/core/lag/lag.h | 71 +-
.../net/ethernet/mellanox/mlx5/core/lag/mp.c | 29 +-
.../ethernet/mellanox/mlx5/core/lag/mpesw.c | 129 ++--
.../ethernet/mellanox/mlx5/core/lag/mpesw.h | 2 +-
.../mellanox/mlx5/core/lag/port_sel.c | 25 +-
.../mellanox/mlx5/core/lag/shared_fdb.c | 50 +-
.../net/ethernet/mellanox/mlx5/core/lib/sd.c | 32 +-
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 1 +
.../ethernet/mellanox/mlx5/core/sf/dev/dev.h | 12 +
.../net/ethernet/mellanox/mlx5/core/vport.c | 41 +-
include/linux/mlx5/driver.h | 2 +-
16 files changed, 744 insertions(+), 418 deletions(-)
base-commit: 7b6ab4dbc156e2f96419715130b3d5738a306c93
--
2.44.0