All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Nathan Rossi <nathan@nathanrossi.com>
Cc: netdev@vger.kernel.org, Nathan Rossi <nathan.rossi@digi.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH] net: phylink: Update SFP selected interface on advertising changes
Date: Thu, 26 Aug 2021 10:25:11 +0100	[thread overview]
Message-ID: <20210826092511.GQ22278@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210826071230.11296-1-nathan@nathanrossi.com>

On Thu, Aug 26, 2021 at 07:12:30AM +0000, Nathan Rossi wrote:
> From: Nathan Rossi <nathan.rossi@digi.com>
> 
> Currently changes to the advertising state via ethtool do not cause any
> reselection of the configured interface mode after the SFP is already
> inserted and initially configured.
> 
> While it is not typical to change the advertised link modes for an
> interface using an SFP in certain use cases it is desirable. In the case
> of a SFP port that is capable of handling both SFP and SFP+ modules it
> will automatically select between 1G and 10G modes depending on the
> supported mode of the SFP. However if the SFP module is capable of
> working in multiple modes (e.g. a SFP+ DAC that can operate at 1G or
> 10G), one end of the cable may be attached to a SFP 1000base-x port thus
> the SFP+ end must be manually configured to the 1000base-x mode in order
> for the link to be established.
> 
> This change causes the ethtool setting of advertised mode changes to
> reselect the interface mode so that the link can be established.

This may be a better solution than the phylink_helper_basex_speed()
approach used to select between 2500 and 1000 base-X, but the config
needs to be validated after selecting a different interface mode, as
per what phylink_sfp_config() does.

I also suspect that this will allow you to select e.g. 1000base-X but
there will be no way back to 10G modes as they will be masked out of
the advertising mask from that point on, as the 1000base-X interface
mode does not allow them.

So, I think the suggested code is problematical as it stands.

phylink_sfp_config() uses a multi-step approach to selecting the
interface mode for a reason - first step is to discover what the MAC
is capable of in _any_ interface mode using _NA to reduce the supported
and advertised mask down, and then to select the interface from that.
I'm not entirely convinced that is a good idea here yet though.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2021-08-26  9:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-26  7:12 [PATCH] net: phylink: Update SFP selected interface on advertising changes Nathan Rossi
2021-08-26  9:25 ` Russell King (Oracle) [this message]
2021-08-27  2:11   ` Nathan Rossi
2021-09-02  5:14 ` [PATCH v2] " Nathan Rossi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210826092511.GQ22278@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=nathan.rossi@digi.com \
    --cc=nathan@nathanrossi.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.