Re: [PATCH v2] app/testpmd: add explicit link autonegotiation control

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon,  9 Nov 2026 15:59:35 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:

> When a particular speed was specified for a port in testpmd, the flags
> passed to the underlying API call always included the
> RTE_ETH_LINK_SPEED_FIXED flag. This flag is not a mandatory part of the
> bitmap when setting speeds, and is used to disable autonegotiation.
> It's possible to have a link report only a single speed while
> still have negotiation enabled, so testpmd needs to be extended to
> support setting a speed both with and without the flag.
> 
> This patch does so by adding optional "autoneg on|off" setting after
> speed and duplex settings specified by the user. This new option adjusts
> the presence or absence of the FIXED speed bit in the ethdev
> configuration structure "rte_eth_conf.link_speeds" bitmap. For backward
> compatibility, the default value is "autoneg off" for fixed speeds if
> not specified, meaning that the RTE_ETH_LINK_SPEED_FIXED flag is set on
> the speed value by default.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> V2:
> * changed syntax to more closely match that of ethtool, adding
>   autoneg on|off at the end of the command.
> * increased compatibility by ensuring that when no autoneg value is
>   specified, a numeric speed sets the fixed flag (autoneg off), while an
>   "auto" speed keeps autoneg on.
> ---

Looked ok to me, but AI decided that it was pickier than me.

Review: [PATCH v2] app/testpmd: add explicit link autonegotiation control

Applied on 26.11-rc2, testpmd builds with -Dwerror=true. Parser
behaviour checked against lib/cmdline: the 7-token and 9-token
instances share a prefix, but match_inst() rejects the short
instance when trailing tokens remain, so there is no ambiguity.
Semantics match rte_ethdev.h (FIXED is bit 0, AUTONEG is 0).

Warning
-------

link_speed_fixed_bit() returns a uint32_t bit, but both callers
immediately reduce it to bool:

	link_speed_fixed_bit(res->value1, autoneg) != 0, &link_speed) < 0)

Make it a predicate and drop the "!= 0" at both call sites:

	static bool
	link_speed_is_fixed(const char *speed, const char *autoneg)
	{
		if (autoneg == NULL)
			return strcmp(speed, "auto") != 0;

		return strcmp(autoneg, "off") == 0;
	}

	if (parse_and_check_speed_duplex(res->value1, res->value2,
			link_speed_is_fixed(res->value1, autoneg), &link_speed) < 0)

Info
----

testpmd_funcs.rst: "NOTE: Combining ``auto`` speed with ``autoneg
off`` is an error." is plain text. The file uses the ``.. note::``
directive everywhere else (17 instances); use it here too.

Review-Result: WARNING