Re: [RFC PATCH net] net: dsa: tear down devlink port regions when tearing down the devlink port on error

On Sun, Sep 05, 2021 at 04:01:32PM +0300, Leon Romanovsky wrote:

On Sun, Sep 05, 2021 at 02:07:35PM +0300, Vladimir Oltean wrote:

quoted

On Sun, Sep 05, 2021 at 01:47:51PM +0300, Leon Romanovsky wrote:

quoted

On Sun, Sep 05, 2021 at 01:31:25PM +0300, Vladimir Oltean wrote:

quoted

On Sun, Sep 05, 2021 at 01:25:03PM +0300, Leon Romanovsky wrote:

quoted

On Sun, Sep 05, 2021 at 11:45:18AM +0300, Vladimir Oltean wrote:

quoted

On Sun, Sep 05, 2021 at 10:07:45AM +0300, Leon Romanovsky wrote:

quoted

On Fri, Sep 03, 2021 at 02:17:38AM +0300, Vladimir Oltean wrote:

<...>

quoted

quoted

That sentence means that your change is OK and you did it right by not
changing devlink port to hold not-working ports.

You're with me so far.

There is a second part. The ports with 'status = "disabled"' in the
device tree still get devlink ports registered, but with the
DEVLINK_PORT_FLAVOUR_UNUSED flavour and no netdev. These devlink ports
still have things like port regions exported.

What we do for ports that have failed to probe is to reinit their
devlink ports as DEVLINK_PORT_FLAVOUR_UNUSED, and their port regions, so
they effectively behave as though they were disabled in the device tree.

Yes, and this part require DSA knowledge that I don't have, because you
suggest fallback for any error during devlink port register,

Again, fallback but not during devlink port register. The devlink port
was registered just fine, but our plans changed midway. If you want to
create a net device with an associated devlink port, first you need to
create the devlink port and then the net device, then you need to link
the two using devlink_port_type_eth_set, at least according to my
understanding.

So the failure is during the creation of the **net device**, we now have a
devlink port which was originally intended to be of the Ethernet type
and have a physical flavour, but it will not be backed by any net device,
because the creation of that just failed. So the question is simply what
to do with that devlink port.

I lost you here, from known to me from the NIC, the **net devices** are
created with devlink_alloc() API call and devlink_port_register comes
later. It means that net device is created (or not) before devlink port
code.

Anyway, it is really not important to me as long as changes won't touch
net/core/devlink.c.

Unless I am mistaken, there is a DEVLINK_PORT_TYPE_WARN_TIMEOUT of 3600
seconds until you can associate the devlink_port with the net_device via
devlink_port_type_eth_set, _after_ you've registered the net_device.
Is that incorrect, or is that not what the timeout is for?