Message-ID: <20200406182002.GD2354@nanopsycho.orion>
Date:   Mon, 6 Apr 2020 20:20:02 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        Ido Schimmel <idosch@...sch.org>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: Changing devlink port flavor dynamically for DSA

Mon, Apr 06, 2020 at 08:11:18PM CEST, f.fainelli@...il.com wrote:
>
>
>On 4/6/2020 11:04 AM, Jiri Pirko wrote:
>> Sun, Apr 05, 2020 at 10:42:29PM CEST, f.fainelli@...il.com wrote:
>>> Hi all,
>>>
>>> On a BCM7278 system, we have two ports of the switch: 5 and 8, that
>>> connect to separate Ethernet MACs that the host/CPU can control. In
>>> premise they are both interchangeable because the switch supports
>>> configuring the management port to be either 5 or 8 and the Ethernet
>>> MACs are two identical instances.
>>>
>>> The Ethernet MACs are scheduled differently across the memory controller
>>> (they have different bandwidth and priority allocations) so it is
>>> desirable to select an Ethernet MAC capable of sustaining bandwidth and
>>> latency for host networking. Our current (in the downstream kernel) use
>>> case is to expose port 5 solely as a control end-point to the user and
>>> leave it to the user how they wish to use the Ethernet MAC behind port
>>> 5. Some customers use it to bridge Wi-Fi traffic, some simply keep it
>>> disabled. Port 5 of that switch does not make use of Broadcom tags in
>>> that case, since ARL-based forwarding works just fine.
>>>
>>> The current Device Tree representation that we have for that system
>>> makes it possible for either port to be elected as the CPU port from a
>>> DSA perspective as they both have an "ethernet" phandle property that
>>> points to the appropriate Ethernet MAC node, because of that the DSA
>>> framework treats them as CPU ports.
>>>
>>> My current line of thinking is to permit a port to be configured as
>>> either "cpu" or "user" flavor and do that through devlink. This can
>>> create some challenges but hopefully this also paves the way for finally
>>> supporting "multi-CPU port" configurations. I am thinking something like
>>> this would be how I would like it to be configured:
>>>
>>> # First configure port 8 as the new CPU port
>>> devlink port set pci/0000:01:00.0/8 type cpu
>>> # Now unmap port 5 from being a CPU port
>>> devlink port set pci/0000:01:00.0/1 type eth
>> 
>> You are mixing "type" and "flavour".
>> 
>> Flavours: cpu/physical. I guess that is what you wanted to set, correct?
>
>Correct, flavor is really what we want to change here.
>
>> 
>> I'm not sure, it would make sense. The CPU port is still CPU port, it is
>> just not used. You can never make is really "physical", am I correct? 
>
>True, although with DSA as you may know if we have a DSA_PORT_TYPE_CPU
>(or DSA_PORT_TYPE_DSA), then we do not create a corresponding net_device
>instance because that would duplicate the Ethernet MAC net_device. This
>is largely the reason for suggesting doing this via devlink (so that we
>do not rely on a net_device handle). So by changing from a
>DSA_PORT_TYPE_CPU flavor to DSA_PORT_TYPE_USER, this means you would now
>see a corresponding net_device instance. Conversely when you migrate
>from DSA_PORT_TYPE_USER to DSA_PORT_TYPE_CPU, the corresponding
>net_device would be removed.

Wait, why would you ever want to have a netdevice for CPU port (changed
to "user")? What am I missing? Is there a usecase, some multi-person
port?


>
>Or maybe we finally bite the bullet and create net_device representors
>for all port types...
>
>> 
>> 
>> btw, we already implement port "type" setting. To "eth" and "ib". This
>> is how you can change the type of fabric for mlx4 driver.
>> 
>> 
>>>
>>> and this would do a simple "swap" of all user ports being now associated
>>> with port 8, and no longer with port 5, thus permitting port 5 from
>>> becoming a standard user port. Or maybe, we need to do this as an atomic
>>> operation in order to avoid a switch being configured with no CPU port
>>> anymore, so something like this instead:
>>>
>>> devlink port set pci/0000:01:00.0/5 type eth mgmt pci/0000:01:00.0/8
>>>
>>> The latter could also be used to define groups of ports within a switch
>>> that has multiple CPU ports, e.g.:
>>>
>>> # Ports 1 through 4 "bound" to CPU port 5:
>>>
>>> for i in $(seq 0 3)
>>> do
>>> 	devlink port set pci/0000:01:00.0/$i type eth mgmt pci/0000:01:00.0/5
>>> done
>>>
>>> # Ports 7 bound to CPU port 8:
>>>
>>> devlink port set pci/0000:01:00.0/1 type eth mgmt pci/0000:01:00.0/8
>> 
>> It is basically a mapping of physical port to CPU port, isn't it?
>> 
>> How about something like?
>> devlink port set pci/0000:01:00.0/1 cpu_master pci/0000:01:00.0/5
>> devlink port set pci/0000:01:00.0/2 cpu_master pci/0000:01:00.0/5
>> devlink port set pci/0000:01:00.0/7 cpu_master pci/0000:01:00.0/8
>> 
>> If CPU port would have 0 mapped ports, it would mean it is disabled.
>> What do you think?
>
>Yes, this makes sense.
>-- 
>Florian

Powered by blists - more mailing lists