Re: [PATCH] net/mlx5: move to polling for LSC interrupts

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 26 Oct 2026 17:04:00 +0100
Dariusz Sosnowski <dsosnowski@nvidia.com> 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: dmitry.kozliuk@gmail.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---

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;
	}

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?

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.

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".