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

On Sun, 5 Sep 2021 14:07:35 +0300 Vladimir Oltean wrote:

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.

Is the failure you're referring to discovered inside the
register_netdevice() call?

The only thing I said about the devlink API in the commit description is
that it would have been nice to just flip the type and flavour of a
devlink port, post registration. That would avoid a lot of complications
in DSA. But that is obviously not possible, and my patch does not even
attempt to do it. What DSA does today, and will still do after the patch
we are discussing on, is to unregister that initial devlink port, and
create another one with the unused flavour, and register that one.

The reason why we even bother to re-register a devlink port a second
time for a port that failed to create and initialize its net_device is
basically for consistency with the ports that are statically disabled in
the device tree. Since devlink is a mechanism through which we gain
insight into the hardware, and disabled ports are still physically
present, just, you know, disabled and not used, their devlink ports
still exist and can be used for things like dumping port regions.
We treat ports that fail to find their PHY during probing as
'dynamically disabled', and the expectation is for them to behave just
the same as ports that were statically disabled through the device tree.

My change is entirely about how to properly structure the code such that
we unregister the port regions that a devlink port might have, before
unregistering the devlink port itself, and how to re-register those port
regions then, after the new devlink port was registered.