[dpdk-dev] [PATCH v5 7/9] devarg: change representor ID to bitmap

[Xueming(Steven) Li]

Hi Haiyue,

>-----Original Message-----
>From: Wang, Haiyue <haiyue.wang at intel.com>
>Sent: Tuesday, January 19, 2021 3:37 PM
>To: Xueming(Steven) Li <xuemingl at nvidia.com>
>Cc: dev at dpdk.org; Slava Ovsiienko <viacheslavo at nvidia.com>; Asaf Penso
><asafp at nvidia.com>; Ajit Khaparde <ajit.khaparde at broadcom.com>;
>Somnath Kotur <somnath.kotur at broadcom.com>; Daley, John
><johndale at cisco.com>; Hyong Youb Kim <hyonkim at cisco.com>; Xing, Beilei
><beilei.xing at intel.com>; Guo, Jia <jia.guo at intel.com>; Matan Azrad
><matan at nvidia.com>; Shahaf Shuler <shahafs at nvidia.com>; NBU-Contact-
>Thomas Monjalon <thomas at monjalon.net>; Yigit, Ferruh
><ferruh.yigit at intel.com>; Andrew Rybchenko
><andrew.rybchenko at oktetlabs.ru>; Ray Kinsella <mdr at ashroe.eu>; Neil
>Horman <nhorman at tuxdriver.com>
>Subject: RE: [PATCH v5 7/9] devarg: change representor ID to bitmap
>
>> -----Original Message-----
>> From: Xueming Li <xuemingl at nvidia.com>
>> Sent: Tuesday, January 19, 2021 15:15
>> Cc: dev at dpdk.org; Viacheslav Ovsiienko <viacheslavo at nvidia.com>;
>> xuemingl at nvidia.com; Asaf Penso <asafp at nvidia.com>; Ajit Khaparde
>> <ajit.khaparde at broadcom.com>; Somnath Kotur
>> <somnath.kotur at broadcom.com>; Daley, John <johndale at cisco.com>;
>Hyong
>> Youb Kim <hyonkim at cisco.com>; Xing, Beilei <beilei.xing at intel.com>;
>> Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>;
>> Matan Azrad <matan at nvidia.com>; Shahaf Shuler <shahafs at nvidia.com>;
>> Thomas Monjalon <thomas at monjalon.net>; Yigit, Ferruh
>> <ferruh.yigit at intel.com>; Andrew Rybchenko
>> <andrew.rybchenko at oktetlabs.ru>; Ray Kinsella <mdr at ashroe.eu>; Neil
>> Horman <nhorman at tuxdriver.com>
>> Subject: [PATCH v5 7/9] devarg: change representor ID to bitmap
>>
>> 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 was compared with devarg.
>> Since ethdev representor port didn't contain controller index and PF
>> index information, callback returned representor from other PF or
>> controller.
>>
>> This patch changes representor ID to bitmap so that the ethdev
>> representor comparer callback returns correct ethdev by comparing full
>> representor information including: controller index, PF index,
>> representor type, SF or VF index.
>>
>> Representor ID bitmap definition:
>>  xxxx xxxx xxxx xxxx
>>  |||| |LLL LLLL LLLL vf/sf id
>>  |||| L 1:sf, 0:vf
>>  ||LL pf id
>>  LL controller(host) id
>>
>> This approach keeps binary compatibility with all drivers, VF
>> representor id matches with simple id for non-bonding and
>> non-multi-host configurations.
>>
>> In the future, the representor ID field and each section should extend
>> to bigger width to support more devices.
>>
>> Signed-off-by: Xueming Li <xuemingl at nvidia.com>
>> Acked-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
>> Acked-by: Thomas Monjalon <thomas at monjalon.net>
>> ---
>>  drivers/net/bnxt/bnxt_reps.c             |  3 +-
>>  drivers/net/enic/enic_vf_representor.c   |  3 +-
>>  drivers/net/i40e/i40e_vf_representor.c   |  3 +-
>>  drivers/net/ixgbe/ixgbe_vf_representor.c |  3 +-
>>  drivers/net/mlx5/linux/mlx5_os.c         |  4 +-
>>  lib/librte_ethdev/rte_class_eth.c        | 38 +++++++++++++----
>>  lib/librte_ethdev/rte_ethdev.c           | 26 ++++++++++++
>>  lib/librte_ethdev/rte_ethdev_driver.h    | 53 ++++++++++++++++++++++++
>>  lib/librte_ethdev/version.map            |  2 +
>>  9 files changed, 122 insertions(+), 13 deletions(-)
>>
>
>
>> +uint16_t
>> +rte_eth_representor_id_encode(uint16_t controller, uint16_t pf,
>> +			      enum rte_eth_representor_type type,
>> +			      uint16_t representor_port)
>> +{
>> +	return (((controller & 3) << 14) |
>> +		((pf & 3) << 12) |
>> +		(!!(type == RTE_ETH_REPRESENTOR_SF) << 11) |
>> +		(representor_port & 0x7ff));
>> +}
>> +
>> +uint16_t
>> +rte_eth_representor_id_parse(const uint16_t representor_id,
>> +			     uint16_t *controller, uint16_t *pf,
>> +			     enum rte_eth_representor_type *type) {
>> +	if (controller)
>> +		*controller = (representor_id >> 14) & 3;
>> +	if (pf)
>> +		*pf = (representor_id >> 12) & 3;
>> +	if (type)
>> +		*type = ((representor_id >> 11) & 1) ?
>> +			RTE_ETH_REPRESENTOR_SF :
>RTE_ETH_REPRESENTOR_VF;
>> +	return representor_id & 0x7ff;
>> +}
>> +
>
>Rename 'parse' to 'decode' ? Since not parse from string. ;-) The these two
>functions are pair style.
>
>rte_eth_representor_id_encode / rte_eth_representor_id_decode ?

Nice catch, thanks!
>
>> 2.25.1