why is auto negotiation disable a devarg rather than extending relevant part of ethdev? my opinion is every time a devarg like this gets added it means the ethdev abstraction is the real issue
On Mon, Nov 2, 2026, 16:37 Bruce Richardson <[email protected]> wrote: > On Mon, Nov 02, 2026 at 10:55:43AM +0000, Loftus, Ciara wrote: > > > Subject: [PATCH] net/ice: add explicit control of link auto-negotiation > > > > > > When selecting a fixed speed of the link, whether or not > > > autonegotiation (e.g. AN Clause 37, etc.), needs to be enabled depends > > > on the peer link. While commit 3dc2933866d9 ("net/ice: disable link > > > negotiation for fixed speed selection") would allow link setup when the > > > remote side also has a fixed link without AN37 auto-negotiation, it > > > causes the link to flap up and down when e.g. two ports are connected > > > back to back and only one is set to a fixed speed. > > > > > > Support both scenarios of auto-negotiation supported or unsupported on > > > the remote end by adding an explicit driver option for disabling > > > auto-negotiation when a fixed speed is selected. If no fixed speed > > > setting, then new option does nothing. > > > > > > Fixes: 3dc2933866d9 ("net/ice: disable link negotiation for fixed speed > > > selection") > > > > > > Signed-off-by: Bruce Richardson <[email protected]> > > > --- > > > doc/guides/nics/ice.rst | 10 ++++++++++ > > > doc/guides/rel_notes/release_26_11.rst | 2 ++ > > > drivers/net/intel/ice/ice_ethdev.c | 12 +++++++++++- > > > drivers/net/intel/ice/ice_ethdev.h | 1 + > > > 4 files changed, 24 insertions(+), 1 deletion(-) > > > > > > diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst > > > index ca3a94655b..af36341c9a 100644 > > > --- a/doc/guides/nics/ice.rst > > > +++ b/doc/guides/nics/ice.rst > > > @@ -107,6 +107,16 @@ Configuration > > > Runtime Configuration > > > ~~~~~~~~~~~~~~~~~~~~~ > > > > > > +``auto_neg_disable`` (default ``0``) > > > + Control hardware link negotiation when a fixed speed is requested > with > > > ``RTE_ETH_LINK_SPEED_FIXED``. > > > + By default, the firmware's negotiation settings are preserved, > > > + allowing a fixed-speed link to use negotiation such as Clause 37 > when > > > required by its peer. > > > + Set ``auto_neg_disable=1`` to disable Clause 28, Clause 37 and > Clause 73 > > > negotiation. > > > + This option has no effect when automatic speed selection is > requested. > > > + For example:: > > > + > > > + -a 80:00.0,auto_neg_disable=1 > > > + > > > > The approach looks good to me. > > Something my AI review flagged was the setting of "link.link_autoneg" > > in ice_link_update() - your patch doesn't update that setting, and it's > still > > just based off the dev_conf.link_speeds value, and doesn't take into > account > > the value of the devarg (unlike ice_apply_link_speed() which accounts for > > both). Perhaps it's not necessary, just something to check. > > Once that is addressed or ruled out: > > > > Acked-by: Ciara Loftus <[email protected]> > > Yes, good catch. That is fixed in v2, but I've dropped your ack as some of > the logic has been moved to a subfunction to avoid copy-pasting too much. >
