<div dir="auto">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 </div> <div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Nov 2, 2026, 16:37 Bruce Richardson <<a href="mailto:bruce.richardson@intel.com">bruce.richardson@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <<a href="mailto:bruce.richardson@intel.com" target="_blank" rel="noreferrer">bruce.richardson@intel.com</a>> > > --- > > 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 <<a href="mailto:ciara.loftus@intel.com" target="_blank" rel="noreferrer">ciara.loftus@intel.com</a>> 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. </blockquote></div>