netdev - Re: [PATCH net-next RFC v1 0/4] Add per port devlink regions

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: <20200921025035.GB3702050@lunn.ch>
Date:   Mon, 21 Sep 2020 04:50:35 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...dia.com>,
        Chris Healy <cphealy@...il.com>
Subject: Re: [PATCH net-next RFC v1 0/4] Add per port devlink regions

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