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

Fri, Mar 29, 2019 at 07:59:26PM CET, jakub.kicinski@...ronome.com 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.

Well we have switchib driver which does not have any netdevice and still
the ports are part of a switch. In other words, this is not
ethernet-specific attribute, therefore devlink is the right fit.

>
>> 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?

Hmm, we'll have multiple non-switch port flavours and once your vf/pf
patchset  hits the tree we'll have multipkle switch port flavours.
So makes sense to have switch id. Also, you can have multiple switches
within one asic.


>
>> >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.

I have to gather that info. Not so many things are shared. There is one
extra switch to mix 2 pfs together. I know about some IB features that
also mix 2 pfs.

>
>The situation with pipelined devices is somewhat murky.  Didn't Or add
>some from of PCIe-side looped queue to forward between PFs?

I have no clue. Ccing Or. Or?


>
>Presumably DSA would lean the opposite way with multiple ASICs
>reporting the same ID?

Yes, sounds right.

Powered by blists - more mailing lists