Hi Haiyue,

>-----Original Message-----
>From: Wang, Haiyue <haiyue.w...@intel.com>
>Sent: Tuesday, January 19, 2021 3:37 PM
>To: Xueming(Steven) Li <xuemi...@nvidia.com>
>Cc: dev@dpdk.org; Slava Ovsiienko <viachesl...@nvidia.com>; Asaf Penso
><as...@nvidia.com>; Ajit Khaparde <ajit.khapa...@broadcom.com>;
>Somnath Kotur <somnath.ko...@broadcom.com>; Daley, John
><johnd...@cisco.com>; Hyong Youb Kim <hyon...@cisco.com>; Xing, Beilei
><beilei.x...@intel.com>; Guo, Jia <jia....@intel.com>; Matan Azrad
><ma...@nvidia.com>; Shahaf Shuler <shah...@nvidia.com>; NBU-Contact-
>Thomas Monjalon <tho...@monjalon.net>; Yigit, Ferruh
><ferruh.yi...@intel.com>; Andrew Rybchenko
><andrew.rybche...@oktetlabs.ru>; Ray Kinsella <m...@ashroe.eu>; Neil
>Horman <nhor...@tuxdriver.com>
>Subject: RE: [PATCH v5 7/9] devarg: change representor ID to bitmap
>
>> -----Original Message-----
>> From: Xueming Li <xuemi...@nvidia.com>
>> Sent: Tuesday, January 19, 2021 15:15
>> Cc: dev@dpdk.org; Viacheslav Ovsiienko <viachesl...@nvidia.com>;
>> xuemi...@nvidia.com; Asaf Penso <as...@nvidia.com>; Ajit Khaparde
>> <ajit.khapa...@broadcom.com>; Somnath Kotur
>> <somnath.ko...@broadcom.com>; Daley, John <johnd...@cisco.com>;
>Hyong
>> Youb Kim <hyon...@cisco.com>; Xing, Beilei <beilei.x...@intel.com>;
>> Guo, Jia <jia....@intel.com>; Wang, Haiyue <haiyue.w...@intel.com>;
>> Matan Azrad <ma...@nvidia.com>; Shahaf Shuler <shah...@nvidia.com>;
>> Thomas Monjalon <tho...@monjalon.net>; Yigit, Ferruh
>> <ferruh.yi...@intel.com>; Andrew Rybchenko
>> <andrew.rybche...@oktetlabs.ru>; Ray Kinsella <m...@ashroe.eu>; Neil
>> Horman <nhor...@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 <xuemi...@nvidia.com>
>> Acked-by: Viacheslav Ovsiienko <viachesl...@nvidia.com>
>> Acked-by: Thomas Monjalon <tho...@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

Reply via email to