Re: [dpdk-dev] [PATCH v6 8/9] ethdev: representor iterator compare complete info

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 2/16/21 7:35 PM, Xueming(Steven) Li wrote:
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Monday, February 15, 2021 5:32 PM
>> To: Xueming(Steven) Li <xuemingl@nvidia.com>
>> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Asaf Penso <asafp@nvidia.com>; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> <nhorman@tuxdriver.com>
>> Subject: Re: [PATCH v6 8/9] ethdev: representor iterator compare complete info
>>
>> On 2/14/21 6:21 AM, Xueming Li wrote:
>>> The NIC can have multiple PCIe links and can be attached to multiple
>>> hosts, for example the same single NIC can be shared for multiple
>>> server units in the rack. On each PCIe link NIC can provide multiple
>>> PFs and VFs/SFs based on these ones. The full representor identifier
>>> consists of three indices - controller index, PF index, and VF or SF index (if any).
>>>
>>> SR-IOV and SubFunction are created on top of PF. PF index is
>>> introduced because there might be multiple PFs in the bonding
>>> configuration and only bonding device is probed.
>>>
>>> In eth representor comparator callback, ethdev representor ID was
>>> compared with devarg. Since controller index and PF index not
>>> compared, callback returned representor from other PF or controller.
>>>
>>> This patch adds new API to convert representor controller, pf and
>>> vf/sf index to representor ID. Representor comparer callback convert
>>> representor info into ID and compare with device representor ID.
>>>
>>> Signed-off-by: Xueming Li <xuemingl@nvidia.com>

[snip]

>>> +	}
>>> +	n = ret;
>>> +	size = sizeof(*info) + n * sizeof(info->ranges[0]);
>>> +	info = calloc(1, size);
>>> +	if (info == NULL)
>>> +		return -ENOMEM;
>>> +	ret = rte_eth_representor_info_get(ethdev->data->port_id, info);
>>> +	if (ret < 0)
>>> +		goto out;
>>> +
>>> +	/* Default controller and pf to caller. */
>>> +	if (controller == -1)
>>> +		controller = info->controller;
>>> +	if (pf == -1)
>>> +		pf = info->pf;
>>> +
>>> +	/* Locate representor ID. */
>>> +	for (i = 0; i < n; ++i) {
>>> +		if (info->ranges[i].type != type)
>>> +			continue;
>>> +		/* PMD hit: ignore controller if -1. */
>>> +		if (info->ranges[i].controller != -1 &&
>>> +		    info->ranges[i].controller != (uint16_t)controller)
>>> +			continue;
>>
>> I think it is incorrect to ignore controller in range if controller is specified in request. It must match.
> 
> This is a real requirement I'm facing from orchestration, before orchestration having knowledge on port
> status, it always send "pf#vf#" to OVS->DPDK, although "pf#" is meaningless in standard mode. It will take
> time for orchestration and OVS to evolve... PMD uses this option to decide ignore them or not.

I'll take a look at it on the next version, but it still sounds
wrong when something is specified, but ignored. May be I simply
misunderstand.

Thanks for working on the patch series.