Message-ID: <38f489d7-0c77-6470-35ff-8f86cf655495@mellanox.com>
Date:   Tue, 29 Oct 2019 18:06:28 +0000
From:   Yuval Avnery <yuvalav@...lanox.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Andy Gospodarek <andrew.gospodarek@...adcom.com>
CC:     Jiri Pirko <jiri@...nulli.us>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "shuah@...nel.org" <shuah@...nel.org>,
        Daniel Jurgens <danielj@...lanox.com>,
        Michael Chan <michael.chan@...adcom.com>
Subject: Re: [PATCH net-next 0/9] devlink vdev

On 2019-10-29 10:08 a.m., Jakub Kicinski wrote:
> On Fri, 25 Oct 2019 10:58:08 -0400, Andy Gospodarek wrote:
>> Thanks, Jakub, I'm happy to chime in based on our deployment experience.
>> We definitely understand the desire to be able to configure properties
>> of devices on the SmartNIC (the kind with general purpose cores not the
>> kind with only flow offload) from the server side.
> Thanks!
>
>> In addition to addressing NVMe devices, I'd also like to be be able to
>> create virtual or real serial ports as well as there is an interest in
>> *sometimes* being able to gain direct access to the SmartNIC console not
>> just a shell via ssh.  So my point is that there are multiple use-cases.
> Shelling into a NIC is the ultimate API backdoor. IMO we should try to
> avoid that as much as possible.
>
>> Arm are also _extremely_ interested in developing a method to enable
>> some form of SmartNIC discovery method and while lots of ideas have been
>> thrown around, discovery via devlink is a reasonable option.  So while
>> doing all this will be much more work than simply handling this case
>> where we set the peer or local MAC for a vdev, I think it will be worth
>> it to make this more usable for all^W more types of devices.  I also
>> agree that not everything on the other side of the wire should be a
>> port.
>>
>> So if we agree that addressing this device as a PCIe device then it
>> feels like we would be better served to query device capabilities and
>> depending on what capabilities exist we would be able to configure
>> properties for those.  In an ideal world, I could query a device using
>> devlink ('devlink info'?) and it would show me different devices that
>> are available for configuration on the SmartNIC and would also give me a
>> way to address them.  So while I like the idea of being able to address
>> and set parameters as shown in patch 05 of this series, I would like to
>> see a bit more flexibility to define what type of device is available
>> and how it might be configured.
> We shall see how this develops. For now sounds pretty high level.
> If the NIC needs to expose many "devices" that are independently
> controlled we should probably look at re-using the standard device
> model and not reinvent the wheel.
> If we need to configure particular aspects and resource allocation,
> we can add dedicated APIs as needed.
>
> What I definitely want to avoid is adding a catch-all API with unclear
> semantics which will become the SmartNIC dumping ground.
>
>> So if we took the devlink info command as an example (whether its the
>> proper place for this or not), it could look _like_ this:
>>
>> $ devlink dev info pci/0000:03:00.0
>> pci/0000:03:00.0:
>>    driver foo
>>    serial_number 8675309
>>    versions:
>> [...]
>>    capabilities:
>>        storage 0
>>        console 1
>>        mdev 1024
>>        [something else] [limit]
>>
>> (Additionally rather than putting this as part of 'info' the device
>> capabilities and limits could be part of the 'resource' section and
>> frankly may make more sense if this is part of that.)
>>
>> and then those capabilities would be something that could be set using the
>> 'vdev' or whatever-it-is-named interface:
>>
>> # devlink vdev show pci/0000:03:00.0
>> pci/0000:03:00.0/console/0: speed 115200 device /dev/ttySNIC0
> The speed in this console example makes no sense to me.
>
> The patches as they stand are about the peer side/other side of the
> port. So which side of the serial device is the speed set on? One can
> just read the speed from /dev/ttySNIC0. And link that serial device to
> the appropriate parent via sysfs. This is pure wheel reinvention.
>
The patches are not only about the other side,

They are about all the devices which are under the control of a 
privileged user.

In the case of SmartNic, those devices (vdevs) are on the host side, and 
the privileged user runs on the embedded CPU.

The vdev devices don't necessarily have relationship with a port. All 
attributes are optional except flavour.

>> pci/0000:03:00.0/mdev/0: hw_addr 02:00:00:00:00:00
>> [...]
>> pci/0000:03:00.0/mdev/1023: hw_addr 02:00:00:00:03:ff
>>
>> # devlink vdev set pci/0000:03:00.0/mdev/0 hw_addr 00:22:33:44:55:00
>>
>> Since these Arm/RISC-V based SmartNICs are going to be used in a variety
>> of different ways and will have a variety of different personalities
>> (not just different SKUs that vendors will offer but different ways in
>> which these will be deployed), I think it's critical that we consider
>> more than just the mdev/representer case from the start.

Powered by blists - more mailing lists