Subject: Re: [patch net-next RFC 0/6] Introduce devlink interface and first drivers to use it

From: Doug Ledford <[email-masked]>
Date: Mon, 8 Feb 2016 14:00:14 -0500

On 02/07/2016 03:18 PM, roopa wrote:
> On 2/3/16, 2:47 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <[email-masked]>
>>
>> There a is need for some userspace API that would allow to expose things
>> that are not directly related to any device class like net_device of
>> ib_device, but rather chip-wide/switch-ASIC-wide stuff.
>>
>> Use cases:
>> 1) get/set of port type (Ethernet/InfiniBand)
>> 2) monitoring of hardware messages to and from chip
>> 3) setting up port splitters - split port into multiple ones and squash again,
>>    enables usage of splitter cable
>> 4) setting up shared buffers - shared among multiple ports within one chip
>>
>> First patch of this set introduces a new generic Netlink based interface,
>> called "devlink". It is similar to nl80211 model and it is heavily
>> influenced by it, including the API definition. The devlink introduction patch
>> implements use cases 1) and 2). Other 2 are in development atm and will
>> be addressed by follow-ups.
>>
>> It is very convenient for drivers to use devlink, as you can see in other
>> patches in this set.
>>
>> Counterpart for devlink is userspace tool called "dl". Command line interface
>> and outputs are derived from "ip" tool so it should be easy for users to get
>> used to it.
>>
>> It is available here:
>> https://github.com/jpirko/devlink
> 
> Jiri, thanks for this series!. Something like this is definitely needed for chip specific data. But i thought we were going to limit it to chip specific global attributes that cannot be set on a port.
>>
>> Example usage:
>> butter:~$ dl help
>> Usage: dl [ OPTIONS ] OBJECT { COMMAND | help }
>> where  OBJECT := { dev | port | monitor }
>>        OPTIONS := { -v/--verbose }
>>
>> butter:~$ dl dev show
>> 0: devlink0: bus pci dev 0000:01:00.0
>>
>> butter:~$ dl port help
>> Usage: dl port show [DEV/PORT_INDEX]
>> Usage: dl port set DEV/PORT_INDEX [ type { eth | ib | auto} ]
> 
> I don't think we should include port specific attributes in this api. ports are netdevs and they should still continue to use 'ip link set'.

That's not true.  InfiniBand ports don't have a netdev (unless you load
IPoIB, and then it's an emulated netdev, but the actual port itself has
none).  So as a general rule, you can never run ip link set on an
InfiniBand port.  You can run ip link set on the IPoIB device on that
port, but the features/bits you can set that way are specific to the
IPoIB device and not the parent IB device.

With that in mind, there has been discussions around the SRIOV settings
on InfiniBand ports.  If you have an Ethernet device, there are well
defined means of setting the parameters of the VFs via the PF.  Since an
InfiniBand device doesn't have a netdev, we are currently rolling our
own methods.  A couple solutions have been floated, neither one has been
coded up.  One was to use netlink directly on the InfiniBand PF to set
VF features, but that goes back to the problem that there is no netdev
for the IB port, so we would have to either register a fake dev or
something.  The other idea was to make IPoIB devices mandatory if you
have SRIOV IB devices, and then use the IPoIB device as the netdev to
configure and then add some new entry points for configuring IB devices
via an IPoIB netdev.  There are obvious differences, such as the VF GID
length would not come even close to being the same as the IPoIB dev's
netdev MAC length, possibly multiple P_Keys need configured on the VF
while the IPoIB dev has a single fixed P_Key per device, etc.  But the
whole making IPoIB mandatory and then using one class of PF device to
configure a different class of VF device is obviously just a really ugly
hack.

> So, why not just introduce a rtnetlink ops abstraction for switch ports ?. Example:
> 
> static struct rtnl_link_ops swport_link_ops __read_mostly = {
>         .kind           = 'swport',
>         .setup          = swport_setup,
>         .validate       = swport_validate,
> };
> 
> IFLA_INFO_KIND = swport
> IFLA_INFO_DATA = {
>             IFLA_SWPORT_KIND = 'mlx'
>             IFLA_SWPORT_TYPE, /* u16 */
>             IFLA_SWPORT_DESIRED_TYPE, /* u16 */
> 
>             ... more ...
> }
> 
> IFLA_INFO_DATA can be redirected to the specific switch driver at the switchdev ops/api layer.
> 
> ip link set dev swp1 swport_type eth swport_desired_type <blah>
> 
> Taking this one step further, the port splitter management can be done in user-space:
> And, users/switch port management infra can use 'ip link add type swport ...' to create required switch ports.
> 
> Thanks,
> Roopa
> 
> 
> 
>              
> 

-- 
Doug Ledford <[email-masked]>
              GPG KeyID: 0E572FDD