netdev - Re: [PATCH net-next 0/3] nfp: support VF multi-queues configuration

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]

Message-ID: <20221026142221.7vp4pkk6qgbwcrjk@sx1>
Date:   Wed, 26 Oct 2022 15:22:21 +0100
From:   Saeed Mahameed <saeed@...nel.org>
To:     Yinjun Zhang <yinjun.zhang@...igine.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Simon Horman <simon.horman@...igine.com>,
        David Miller <davem@...emloft.net>,
        Paolo Abeni <pabeni@...hat.com>,
        Michael Chan <michael.chan@...adcom.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        Gal Pressman <gal@...dia.com>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>,
        Edward Cree <ecree.xilinx@...il.com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Andrew Lunn <andrew@...n.ch>,
        Nole Zhang <peng.zhang@...igine.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        oss-drivers <oss-drivers@...igine.com>
Subject: Re: [PATCH net-next 0/3] nfp: support VF multi-queues configuration

On 25 Oct 11:39, Yinjun Zhang wrote:
>On Date: Tue, 25 Oct 2022 12:05:14 +0100, Saeed Mahameed wrote:
>> On 25 Oct 10:41, Yinjun Zhang wrote:
>> >On Tue, 25 Oct 2022 08:51:41 +0100, Saeed Mahameed wrote:
>> >> The problem with this is that this should be a per function parameter,
>> >> devlink params or resources is not the right place for this as this
>> >> should be a configuration of a specific devlink object that is not the
>> >> parent device (namely devlink port function), otherwise we will have to
>> >> deal with ugly string parsing to address the specific vf attributes.
>> >>
>> >> let's use devlink port:
>> >> https://www.kernel.org/doc/html/latest/networking/devlink/devlink-
>> >> port.html
>> >>
>> >> devlink ports have attributes and we should extend attributes to act like
>> >> devlink parameters.
>> >>
>> >>    devlink port function set DEV/PORT_INDEX [ queue_count count ] ...
>> >>
>> >> https://man7.org/linux/man-pages/man8/devlink-port.8.html
>> >
>> >Although the vf-max-queue is a per-VF property, it's configured from PF's
>> >perspective, so that the overall queue resource can be reallocated among
>> VFs.
>> >So a devlink object attached to the PF is used to configure, and resource
>> seems
>> >more appropriate than param.
>> >
>>
>> devlink port function is an object that's exposed on the PF. It will give
>> you a handle on the PF side to every sub-function (vf/sf) exposed via the
>> PF.
>
>Sorry, I thought you meant each VF creates a devlink obj. So still one devlink obj
>and each VF registers a devlink port, right? But the configuration is supposed to
>be done before VFs are created, it maybe not appropriate to register ports before
>relevant VFs are created I think.
>

Usually you create the VFs unbound, configure them and then bind them.
otherwise a query will have to query any possible VF which for some vendors
can be thousands ! it's better to work on created but not yet deployed vfs

>>
>> can you provide an example of how you imagine the reosurce vf-max-queue
>> api
>> will look like ?
>
>Two options,
>one is from VF's perspective, you need configure one by one, very straightforward:
>```
>pci/xxxx:xx:xx.x:
>  name max_q size 128 unit entry
>    resources:
>      name VF0 size 1 unit entry size_min 1 size_max 128 size_gran 1
>      name VF1 size 1 unit entry size_min 1 size_max 128 size_gran 1
>      ...

the above semantics are really weird, 
VF0 can't be a sub-resource of max_q ! 

sorry i can't think of a way where devlink resoruce semantics can work for
VF resource allocation.

Unless a VF becomes a resource and it's q_table becomes a sub resource of that
VF, which means you will have to register each vf as a resource individually.

Note that i called the resource "q_table" and not "max_queues",
since semantically max_queues is a parameter where q_table can be looked at
as a sub-resource of the VF, the q_table size decides the max_queues a VF
will accept, so there you go ! 
arghh weird.. just make it an attribute for devlink port function and name it
max_q as god intended it to be ;). Fix your FW to allow changing VF maxqueue for
unbound VFs if needed.


>```
>another is from queue's perspective, several class is supported, not very flexible:
>```
>pci/xxxx:xx:xx.x:
>  name max_q_class size 128 unit entry
>    resources:
>      # means how many VFs possess max-q-number of 16/8/..1 respectively
>      name _16 size 0 unit entry size_min 0 size_max 128 size_gran 1
>      name _8 size 0 unit entry size_min 0 size_max 128 size_gran 1
>      ...
>      name _1 size 0 unit entry size_min 0 size_max 128 size_gran 1
>```

weirder.