On Mon, Oct 26, 2026 at 10:13:48AM -0700, Stephen Hemminger wrote:
> On Mon, 26 Oct 2026 17:04:00 +0100
> Dariusz Sosnowski <[email protected]> wrote:
>
> > mlx5 networking driver on Linux implemented LSC interrupts
> > by listening on netlink events.
> > This method is unfortunately not reliable enough,
> > because of linkwatch mechanism. It can delay link up/down events
> > for up to ~1 second. This introduces unnecessary delay
> > when mlx5 ports are used with DPDK bonding driver for examples.
> >
> > This patch addresses that by migrating LSC checks
> > to polling on link status using ETHTOOL_GLINK command.
> > This ethtool command, implemented by mlx5 since kernel 4.14,
> > will always get the current link status.
> > As a side effect it is also flushes the pending link watch events.
> >
> > Fixes: 17f95513adca ("net/mlx5: fix link status change detection")
> > Cc: [email protected]
> > Cc: [email protected]
> >
> > Signed-off-by: Cosmin Ratiu <[email protected]>
> > Signed-off-by: Dariusz Sosnowski <[email protected]>
> > ---
>
> Agree that 1s delay is bad for bonding, there are historical reasons for
> link watch. With some old NIC's the PHY would stream up/down on loose
> cables causing DoS on the machine. If you are using LSC for bonding
> and implementing LSC in driver, you might want to consider some rate
> limiting.
>
> AI review showed some issues.
>
> [PATCH] net/mlx5: move to polling for LSC interrupts
>
> Applies cleanly to main (a635140). Moves Linux LSC detection from
> netlink RTM_NEWLINK to a 100ms EAL alarm polling ETHTOOL_GLINK.
>
> Warning
>
> 1. Poller stores a partial, non-atomic dev_link.
>
> lsc_intr_check_links() copies dev->data->dev_link, flips only
> link_status, and writes the struct back with a plain assignment.
> mlx5_link_update() (now using rte_eth_linkstatus_set(), an atomic
> exchange on val64) can run concurrently from an application thread,
> so the read-modify-write can lose that update. On link up, speed and
> duplex are left at whatever the last link-down stored. The poller
> also ignores IFF_UP while mlx5_link_update() reports
> IFF_UP && carrier, so the two paths can disagree.
>
> Keep the cheap carrier probe, and on change refresh the full record
> through the existing path:
>
> ret = mlx5_link_carrier_get(dev);
> if (ret < 0 || ret == dev->data->dev_link.link_status ||
> mlx5_link_update(dev, 0) <= 0) {
> devs[i] = NULL;
> continue;
> }
I'll address in v2
>
> 2. Polling cost on the host.
>
> ETHTOOL_GLINK runs under rtnl_lock in the kernel. This is 10 Hz per
> started port with lsc=1, including representors in switchdev setups,
> per DPDK process. The adaptive period only reacts to latency seen by
> the DPDK interrupt thread, not to rtnl contention imposed on the rest
> of the system. Was this measured with a large number of representors?
I ran some tests on a following environment:
- Ubuntu 24.04
- Intel(R) Xeon(R) Platinum 8380 CPU
- NVIDIA ConnectX-7
- 1 PF, 127 VFs
Polling time of all 128 ports (time spent in mlx5_lsc_intr_cb()):
- only ETHTOOL_GLINK on each port:
- total: ~0.71 ms
- per port: ~5.5 us
- ETHTOOL_GLINK, followed by mlx5_link_update() on each port:
- total: ~19.72 ms
- per port: ~154 us
>
> The delay 17f95513adca worked around was IFF_RUNNING lagging the IB
> port event. With the query now done via ETHTOOL_GLINK (reads carrier
> and syncs linkwatch), going back to IBV_EVENT_PORT_ACTIVE/ERR on the
> existing shared async handler and querying carrier there would give
> immediate events without polling. Please explain why that was not
> chosen, or use it.
On mlx5 ethernet devices, IBV_EVENT_PORT_ACTIVE/ERR events are generated
based on netdev carrier status [1].
As a result, these are subject to the same delay as netlink notifications.
There is also IBV_EVENT_DEVICE_SPEED_CHANGE, which is generated on port up/down
events
and is generated immediately, but:
- it is only available from Linux v7.0;
- it is a device-wide event; it would require a scan of all related ports
to know which one is affected;
[1]:
https://github.com/torvalds/linux/blob/v7.3/drivers/infiniband/hw/mlx5/main.c#L3082
>
> 3. Not stable material as is.
>
> A new 300 line polling framework using rte_bitset (24.11 only) is
> not a backport candidate for 23.11, and reducing a ~1s notification
> delay is not a regression fix. Suggest splitting:
> - small fix: use ETHTOOL_GLINK in mlx5_link_update_unlocked_gs/gset
> (Fixes: 17f95513adca, Cc: stable)
> - the LSC mechanism change, without Fixes/Cc stable
>
> Info
>
> 4. Windows stub logs ERR on every dev_start with lsc=1, while the PCI
> driver still advertises RTE_PCI_DRV_INTR_LSC. The old path logged
> INFO. Make the stub silent; mlx5_dev_start() already logs. Also
> "failed configure" -> "failed to configure" in that message.
>
> 5. Leftovers: the rte_io_wmb() in the mlx5_dev_spawn() error path
> ordered the removed nl_ih_port_id store and can go; <stdbool.h> in
> mlx5_lsc_intr.h is unused.
>
> 6. mlx5_link_update() now reports only link_status changes instead
> of a memcmp of the whole struct. Fine, but mention it in the commit
> message.
>
> 7. Commit message: "for examples" -> "for example",
> "it is also flushes" -> "it also flushes".
Points 3-7: I'll address in v2
