netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: netdev <netdev@vger.kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>, Jiri Pirko <jiri@nvidia.com>,
	Chris Healy <cphealy@gmail.com>
Subject: Re: [PATCH net-next RFC v1 0/4] Add per port devlink regions
Date: Mon, 21 Sep 2020 04:50:35 +0200	[thread overview]
Message-ID: <20200921025035.GB3702050@lunn.ch> (raw)
In-Reply-To: <20200920233340.ddps7yxoqlbvmv7m@skbuf>

On Sun, Sep 20, 2020 at 11:33:41PM +0000, Vladimir Oltean wrote:
> On Sat, Sep 19, 2020 at 04:43:28PM +0200, Andrew Lunn wrote:
> >
> > DSA only instantiates devlink ports for switch ports which are used.
> > For this hardware, only 4 user ports and the CPU port have devlink
> > ports, which explains the discontinuous port regions.
> 
> This is not so much a choice, as it is a workaround of the fact that
> dsa_port_setup(), which registers devlink ports with devlink, is called
> after ds->ops->setup(), so you can't register your port regions from
> the same place as the global regions now.

Correct.

> So you're doing it from ds->ops->port_enable(), which is the DSA wrapper
> for .ndo_open(). So, consequently, your port regions will only be
> registered when the port is up, and will be unregistered when it goes
> down. Is that what you want? I understand that users probably think they
> want to debug only the ports that they actively use, but I've heard of
> at least one problem in the past which was caused by invalid settings
> (flooding in that case) on a port that was down. Sure, this is probably
> a rare situation, but as I said, somebody trying to use port regions to
> debug something like that is probably going to have a hard time, because
> it isn't an easy surgery to figure the probe ordering out.

I did intially create the port instances at the same time as the
global ones, and it died a horrible death. And i was aiming to
register a region for each port, not just those which are used.

This splits into two problems.

1) Devlink has no concept of a port which is unused. We simply don't
register unused ports. So we need to add a new devlink_port_flavour:
DEVLINK_PORT_FLAVOUR_UNUSED. That seems easy enough.

2) We need to rearrange the order the core sets stuff up, such that it
registers devlink ports before calling the DSA driver setup()
method. I think that is possible after a quick look at the code.

	Andrew

      parent reply	other threads:[~2020-09-21  2:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-19 14:43 [PATCH net-next RFC v1 0/4] Add per port devlink regions Andrew Lunn
2020-09-19 14:43 ` [PATCH net-next RFC v1 1/4] net: devlink: Add support for port regions Andrew Lunn
2020-09-20 23:45   ` Vladimir Oltean
2020-09-21  0:23     ` Vladimir Oltean
2020-09-21  3:02       ` Andrew Lunn
2020-09-21 10:09         ` Vladimir Oltean
2020-09-19 14:43 ` [PATCH net-next RFC v1 2/4] net: dsa: Add devlink port regions support to DSA Andrew Lunn
2020-09-20 23:23   ` Vladimir Oltean
2020-09-21  2:32     ` Andrew Lunn
2020-09-19 14:43 ` [PATCH net-next RFC v1 3/4] net: dsa: Add helper for converting devlink port to ds and port Andrew Lunn
2020-09-20 23:52   ` Vladimir Oltean
2020-09-26 17:28     ` Andrew Lunn
2020-09-26 17:45       ` Vladimir Oltean
2020-09-19 14:43 ` [PATCH net-next RFC v1 4/4] net: dsa: mv88e6xxx: Add per port devlink regions Andrew Lunn
2020-09-20 23:59   ` Vladimir Oltean
2020-09-20 23:33 ` [PATCH net-next RFC v1 0/4] " Vladimir Oltean
2020-09-20 23:44   ` Florian Fainelli
2020-09-21  2:50   ` Andrew Lunn [this message]

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=20200921025035.GB3702050@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=cphealy@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=vladimir.oltean@nxp.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).