All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Casey Leedom <leedom@chelsio.com>
Cc: Jakub Kicinski <kubakici@wp.pl>,
	Dustin Byford <dustin@cumulusnetworks.com>,
	Roopa Prabhu <roopa@cumulusnetworks.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"linville@tuxdriver.com" <linville@tuxdriver.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"vidya.chowdary@gmail.com" <vidya.chowdary@gmail.com>,
	"olson@cumulusnetworks.com" <olson@cumulusnetworks.com>,
	Manoj Malviya <manojmalviya@chelsio.com>,
	Santosh Rastapur <santosh@chelsio.com>,
	"yuval.mintz@qlogic.com" <yuval.mintz@qlogic.com>,
	"odedw@mellanox.com" <odedw@mellanox.com>,
	"ariela@mellanox.com" <ariela@mellanox.com>,
	"galp@mellanox.com" <galp@mellanox.com>,
	"jeffrey.t.kirsher@intel.com" <jeffrey.t.kirsher@intel.com>
Subject: Re: [PATCH net-next 1/3] net: ethtool: add support for forward error correction modes
Date: Fri, 7 Jul 2017 00:33:24 +0200	[thread overview]
Message-ID: <20170706223324.GA24237@lunn.ch> (raw)
In-Reply-To: <MWHPR12MB16008B0A63E099591C26C7E8C8D50@MWHPR12MB1600.namprd12.prod.outlook.com>

On Thu, Jul 06, 2017 at 09:53:46PM +0000, Casey Leedom wrote:
> | From: Jakub Kicinski <kubakici@wp.pl>
> | Sent: Thursday, July 6, 2017 12:02 PM
> |
> | IMHO if something gets replugged all the settings should be reset.
> | I feel that it's not entirely unlike replugging a USB adapter.  Perhaps
> | we should introduce some (devlink) notifications for SFP module events
> | so userspace can apply whatever static user config it has?
> 
> Absolutely a valid approach.  As are all of the ones I outlined.
> 
> But, and far more importantly, ideally _*ANY*_ such decision is made at an
> architectural level to apply to all Link Parameters and Vendor Products.
> The last thing a user wants to deal with is a hodge-podge of different
> policies for different adapters from different vendors.

Yes.

SFP needs to becomes a Linux device, similar to Copper PHYs are Linux
devices. With some core code which all drivers can use, implement
ethtool --dump-module-eeprom, report speeds to the MAC using
adjust_link, etc..

> how do users conceive of a "Port"?

For a user, it is something they configure via /etc/network/interfaces
and then use ifup/ifdown on.

>  I.e. when a user requests that a particular
> Link Parameter be applied to a Port, are they thinking that it only applies
> to the current instantaneous combination of Adapter Transceiver Module Cage
> + Transceiver Module?  Or do they conceptualize a "Port" as being a higher
> level entity?
> 
> Or, let's make it Very Concrete with a specific example:
> 
>  1. User applies some set of Link Parameters.
> 
>  2. User attempts to bring Link up but it doesn't come up.

So these are effectively one step for the user, since the
configuration goes into /etc/network/interfaces, and it is only when
ifup is used is it applied. If the configuration is not valid, at this
point in time, i would expect ifup to give an error message.

>  3. User decides to try a different cable on the grounds that the first
>     cable may be bad.
> 
>  4. New cable is accidentally of a completely different type with completely
>     different subsequent Physical Port Capabilities, not capable of supporting
>     the user's selected Link Parameters.

And this is where it gets interesting, as you say. We are into a
hotplug model.

I think you also need to define 'cable' here. I assume you are not
talking about a piece of CAT 5 or glass fibre. You mean something
which is active. You are putting a different module into the SFP cage.

The extreme model would be, if you pull the module out, the whole
netdev is hot-unplugged. Plug a different modules in, the netdev is
hot-plugged. The user has to ifup it again, and would get an error
message if the configuration is invalid.

But i think this is too extreme.

I think the sfp device needs to give a hotplug event on unplug/plug.
A hot-unplug would result in an ifdown. And within the kernel, the
netdev is set down. If there is an "allow-hotplug" statement in
/etc/network/interfaces, on hot-plug, udev would try to ifup and get
an error and it will stay down. Without the "allow-hotplug" the
interface remains configured down until the user does an ifup and
would see an error message if the configuration is invalid.

    Andrew

  parent reply	other threads:[~2017-07-06 22:33 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-24 19:19 [PATCH net-next 0/3] ethtool: support for forward error correction mode setting on a link Roopa Prabhu
2017-06-24 19:19 ` [PATCH net-next 1/3] net: ethtool: add support for forward error correction modes Roopa Prabhu
2017-06-25 13:38   ` Gal Pressman
2017-06-28  5:46     ` Dustin Byford
2017-06-29 15:49       ` Gal Pressman
2017-06-27 10:22   ` Jakub Kicinski
2017-06-28  6:27     ` Dustin Byford
2017-06-28  6:41       ` Jakub Kicinski
2017-06-28 13:41     ` Andrew Lunn
2017-06-28 21:47       ` Dustin Byford
2017-06-29  1:00         ` Jakub Kicinski
2017-07-06 18:53           ` Casey Leedom
2017-07-06 19:02             ` Jakub Kicinski
2017-07-06 21:06               ` Wyborny, Carolyn
2017-07-06 21:53               ` Casey Leedom
2017-07-06 22:16                 ` Wyborny, Carolyn
2017-07-06 22:36                   ` Andrew Lunn
2017-07-06 22:37                   ` Casey Leedom
2017-07-06 22:33                 ` Andrew Lunn [this message]
2017-07-06 22:47                   ` Casey Leedom
2017-07-06 23:15                     ` Andrew Lunn
2017-07-06 23:27                       ` Jakub Kicinski
2017-07-06 23:39                       ` Casey Leedom
2017-07-07  0:56                         ` Andrew Lunn
2017-07-07  1:38                           ` Dave Olson
2017-07-06 22:43                 ` Jakub Kicinski
2017-07-06 22:57                   ` Casey Leedom
2017-06-29 13:30         ` Andrew Lunn
2017-06-24 19:19 ` [PATCH net-next 2/3] cxgb4: core hardware/firmware support for Forward Error Correction on a link Roopa Prabhu
2017-06-27  3:16   ` David Miller
2017-06-24 19:19 ` [PATCH net-next 3/3] cxgb4: ethtool forward error correction management support Roopa Prabhu
2017-06-24 21:47 ` [PATCH net-next 0/3] ethtool: support for forward error correction mode setting on a link Andrew Lunn

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=20170706223324.GA24237@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=ariela@mellanox.com \
    --cc=davem@davemloft.net \
    --cc=dustin@cumulusnetworks.com \
    --cc=galp@mellanox.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=kubakici@wp.pl \
    --cc=leedom@chelsio.com \
    --cc=linville@tuxdriver.com \
    --cc=manojmalviya@chelsio.com \
    --cc=netdev@vger.kernel.org \
    --cc=odedw@mellanox.com \
    --cc=olson@cumulusnetworks.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=santosh@chelsio.com \
    --cc=vidya.chowdary@gmail.com \
    --cc=yuval.mintz@qlogic.com \
    /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.