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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210905150115.bw5cgv572my5cmep@skbuf>
Date:   Sun, 5 Sep 2021 18:01:15 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>
Subject: 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:
> > On Sun, Sep 05, 2021 at 01:47:51PM +0300, Leon Romanovsky wrote:
> > > On Sun, Sep 05, 2021 at 01:31:25PM +0300, Vladimir Oltean wrote:
> > > > On Sun, Sep 05, 2021 at 01:25:03PM +0300, Leon Romanovsky wrote:
> > > > > On Sun, Sep 05, 2021 at 11:45:18AM +0300, Vladimir Oltean wrote:
> > > > > > On Sun, Sep 05, 2021 at 10:07:45AM +0300, Leon Romanovsky wrote:
> > > > > > > On Fri, Sep 03, 2021 at 02:17:38AM +0300, Vladimir Oltean wrote:
> > > 
> > > <...>
> > > 
> > > > > 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?