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

From: Yinjun Zhang <yinjun.zhang@corigine.com>
To: Saeed Mahameed <saeed@kernel.org>
Cc: Jakub Kicinski <kuba@kernel.org>,
	Simon Horman <simon.horman@corigine.com>,
	David Miller <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	Michael Chan <michael.chan@broadcom.com>,
	Andy Gospodarek <andy@greyhouse.net>,
	Gal Pressman <gal@nvidia.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	Edward Cree <ecree.xilinx@gmail.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Nole Zhang <peng.zhang@corigine.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	oss-drivers <oss-drivers@corigine.com>
Subject: RE: [PATCH net-next 0/3] nfp: support VF multi-queues configuration
Date: Tue, 25 Oct 2022 10:41:41 +0000	[thread overview]
Message-ID: <DM6PR13MB370566F6E88DB8A258B93F29FC319@DM6PR13MB3705.namprd13.prod.outlook.com> (raw)
In-Reply-To: <20221025075141.v5rlybjvj3hgtdco@sx1>

On Tue, 25 Oct 2022 08:51:41 +0100, Saeed Mahameed wrote:
> On 20 Oct 09:35, Yinjun Zhang wrote:
> >On Wed, Oct 19, 2022 at 06:01:06PM -0700, Jakub Kicinski wrote:
> >> On Wed, 19 Oct 2022 16:09:40 +0200 Simon Horman wrote:
> >> > this short series adds the max_vf_queue generic devlink device
> parameter,
> >> > the intention of this is to allow configuration of the number of queues
> >> > associated with VFs, and facilitates having VFs with different queue
> >> > counts.
> >> >
> >> > The series also adds support for multi-queue VFs to the nfp driver
> >> > and support for the max_vf_queue feature described above.
> >>
> >> I appreciate CCing a wider group this time, but my concerns about using
> >> devlink params for resource allocation still stand. I don't remember
> >> anyone refuting that.
> >>
> >> https://lore.kernel.org/all/20220921063448.5b0dd32b@kernel.org/
> >
> >Sorry this part was neglected, we'll take a look into the resource APIs.
> >Thanks.
> >
> 
> 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.

> 
> Alternatively you should also consider limiting vf msix, as we did in mlx5
> https://patchwork.kernel.org/project/linux-
> rdma/cover/20210314124256.70253-1-leon@kernel.org/

Msix is not the limitation in our case.