netdev - Re: [patch net-next 00/12] net: expose switch ID via devlink

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]

Date:   Fri, 29 Mar 2019 14:29:11 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, mlxsw@...lanox.com,
        idosch@...lanox.com, andrew@...n.ch, vivien.didelot@...il.com,
        michael.chan@...adcom.com
Subject: Re: [patch net-next 00/12] net: expose switch ID via devlink

On 3/29/19 11:59 AM, Jakub Kicinski wrote:
> On Fri, 29 Mar 2019 07:49:05 +0100, Jiri Pirko wrote:
>> Thu, Mar 28, 2019 at 10:40:02PM CET, jakub.kicinski@...ronome.com wrote:
>>> On Thu, 28 Mar 2019 22:12:42 +0100, Jiri Pirko wrote:  
>>>> From: Jiri Pirko <jiri@...lanox.com>
>>>>
>>>> To provide visibility of the ports, this patchset exposes switch ID
>>>> for devlink ports, which are part of a switch. The rest of the ports
>>>> if any (in case of sr-iov for example) do not set switch ID.  
>>>
>>> I don't feel good about this patch set.  There is no visibility
>>> provided here.  Should the port flavour should be a sufficient  
>>
>> 1) this patch is mainly about avoiding need to define the ndo and moving
>>    the switch id definition to devlink port attr.
> 
> Sure, that you could achieve by putting the data in the netdevice
> structure as well..
> 
> What is the guiding principle here?  I'm trying to argue for leaving
> forwarding-related info in netdev code, and only have HW control in
> devlink.  I just don't see switch id being useful at devlink level in
> any way.
> 
>> 2) along with that, switch id is added as attribute. It tells the user
>>    that some devlink port is part of a switch with certain id. If port
>>    is not part of a switch (like upcoming hostport, cpu, dsa, etc),
>>    switch id is not set on that port
> 
> If the flavour already gives that information, why crowd the attributes
> for ports with switch id?
> 
>>> indication of whether netdev associated with that port can be 
>>> switched to or not?  CPU, DSA, and Host flavours can't be switched 
>>> to.  And the switchid can be an attribute of the devlink instance,
>>> if we want to expose it via devlink.  
>>
>> One devlink instance can have multiple switch ids in use as it may
>> contain multiple switches. Take mlx5 as an instance. Currently every PF
>> creates a separate devlink instance, however there are some features
>> shared. In this example, with proposed idea of aliasing, there would be
>> one devlink instance aliased between these 2 pf inctances, with 2
>> eswitches and 2 sets of switch ports each belonging to an eswitch -
>> distinguished by switch id.
> 
> Out of curiosity, what are the shared features?  It seems mlx5 drives 
> a lot of our API design, it'd be good if the community had a better
> understanding of it.
> 
> The situation with pipelined devices is somewhat murky.  Didn't Or add
> some from of PCIe-side looped queue to forward between PFs?
> 
> Presumably DSA would lean the opposite way with multiple ASICs
> reporting the same ID?

If you have multiple switches inter connected between each other to use
the "D" in DSA and form a fabric of switches, then you would expect each
port to be physically tied to a particular switch device/instance,
because, but how they will report the switch physical ID can be of the form:

<fabric>.<switch>

where fabric is dst->index and switch is ds->index (the switch within
the fabric).
-- 
Florian