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

Xueming(Steven) Li xuemingl at nvidia.com
Wed Jan 20 06:51:33 CET 2021



>-----Original Message-----
>From: Ajit Khaparde <ajit.khaparde at broadcom.com>
>Sent: Tuesday, January 19, 2021 3:02 AM
>To: Xueming(Steven) Li <xuemingl at nvidia.com>
>Cc: NBU-Contact-Thomas Monjalon <thomas at monjalon.net>; Ferruh Yigit
><ferruh.yigit at intel.com>; Andrew Rybchenko
><andrew.rybchenko at oktetlabs.ru>; Olivier Matz <olivier.matz at 6wind.com>;
>dpdk-dev <dev at dpdk.org>; Slava Ovsiienko <viacheslavo at nvidia.com>; Asaf
>Penso <asafp at nvidia.com>
>Subject: Re: [dpdk-dev] [PATCH v4 7/9] devarg: change representor ID to
>bitmap
>
>On Mon, Jan 18, 2021 at 3:18 AM Xueming Li <xuemingl at nvidia.com> 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 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>
>Not just in this patch, there is a lot of info in the commit logs.
>Unless I missed it, I don't see an update to
>doc/guides/prog_guide/switch_representation.rst
>Can you please update that.

Thanks, updated in next version.
>
>> ---
>>  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/ethdev_private.c       |  5 ++-
>>  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 +
>>  10 files changed, 126 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/bnxt/bnxt_reps.c b/drivers/net/bnxt/bnxt_reps.c
>> index f7bbf77d3f..34febc2d1e 100644
>> --- a/drivers/net/bnxt/bnxt_reps.c
>> +++ b/drivers/net/bnxt/bnxt_reps.c
>> @@ -186,7 +186,8 @@ int bnxt_representor_init(struct rte_eth_dev
>*eth_dev, void *params)
>>
>>         eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
>>                                         RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
>> -       eth_dev->data->representor_id = rep_params->vf_id;
>> +       eth_dev->data->representor_id = rte_eth_representor_id_encode(
>> +               0, 0, RTE_ETH_REPRESENTOR_VF, rep_params->vf_id);
>>
>>         rte_eth_random_addr(vf_rep_bp->dflt_mac_addr);
>>         memcpy(vf_rep_bp->mac_addr, vf_rep_bp->dflt_mac_addr,
>> diff --git a/drivers/net/enic/enic_vf_representor.c
>b/drivers/net/enic/enic_vf_representor.c
>> index c2c03c0281..632317af15 100644
>> --- a/drivers/net/enic/enic_vf_representor.c
>> +++ b/drivers/net/enic/enic_vf_representor.c
>> @@ -674,7 +674,8 @@ int enic_vf_representor_init(struct rte_eth_dev
>*eth_dev, void *init_params)
>>         eth_dev->dev_ops = &enic_vf_representor_dev_ops;
>>         eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
>>                                         RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
>> -       eth_dev->data->representor_id = vf->vf_id;
>> +       eth_dev->data->representor_id = rte_eth_representor_id_encode(
>> +               0, 0, RTE_ETH_REPRESENTOR_VF, vf->vf_id);
>>         eth_dev->data->mac_addrs = rte_zmalloc("enic_mac_addr_vf",
>>                 sizeof(struct rte_ether_addr) *
>>                 ENIC_UNICAST_PERFECT_FILTERS, 0);
>> diff --git a/drivers/net/i40e/i40e_vf_representor.c
>b/drivers/net/i40e/i40e_vf_representor.c
>> index 9e40406a3d..d90d0fdb9d 100644
>> --- a/drivers/net/i40e/i40e_vf_representor.c
>> +++ b/drivers/net/i40e/i40e_vf_representor.c
>> @@ -510,7 +510,8 @@ i40e_vf_representor_init(struct rte_eth_dev *ethdev,
>void *init_params)
>>
>>         ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR |
>>                                         RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
>> -       ethdev->data->representor_id = representor->vf_id;
>> +       ethdev->data->representor_id = rte_eth_representor_id_encode(
>> +                       0, 0, RTE_ETH_REPRESENTOR_VF, representor->vf_id);
>>
>>         /* Setting the number queues allocated to the VF */
>>         ethdev->data->nb_rx_queues = vf->vsi->nb_qps;
>> diff --git a/drivers/net/ixgbe/ixgbe_vf_representor.c
>b/drivers/net/ixgbe/ixgbe_vf_representor.c
>> index 8185f0d3bb..e15b794761 100644
>> --- a/drivers/net/ixgbe/ixgbe_vf_representor.c
>> +++ b/drivers/net/ixgbe/ixgbe_vf_representor.c
>> @@ -196,7 +196,8 @@ ixgbe_vf_representor_init(struct rte_eth_dev
>*ethdev, void *init_params)
>>                 return -ENODEV;
>>
>>         ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
>> -       ethdev->data->representor_id = representor->vf_id;
>> +       ethdev->data->representor_id = rte_eth_representor_id_encode(
>> +                       0, 0, RTE_ETH_REPRESENTOR_VF, representor->vf_id);
>>
>>         /* Set representor device ops */
>>         ethdev->dev_ops = &ixgbe_vf_representor_dev_ops;
>> diff --git a/drivers/net/mlx5/linux/mlx5_os.c
>b/drivers/net/mlx5/linux/mlx5_os.c
>> index caead107b0..4d7940bcca 100644
>> --- a/drivers/net/mlx5/linux/mlx5_os.c
>> +++ b/drivers/net/mlx5/linux/mlx5_os.c
>> @@ -1025,7 +1025,9 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
>>  #endif
>>         /* representor_id field keeps the unmodified VF index. */
>>         priv->representor_id = switch_info->representor ?
>> -                              switch_info->port_name : -1;
>> +               rte_eth_representor_id_encode(0, 0, RTE_ETH_REPRESENTOR_VF,
>> +                                             switch_info->port_name) :
>> +               -1;
>>         /*
>>          * Look for sibling devices in order to reuse their switch domain
>>          * if any, otherwise allocate one.
>> diff --git a/lib/librte_ethdev/ethdev_private.c
>b/lib/librte_ethdev/ethdev_private.c
>> index 57473b5a39..0b3b4aa871 100644
>> --- a/lib/librte_ethdev/ethdev_private.c
>> +++ b/lib/librte_ethdev/ethdev_private.c
>> @@ -106,10 +106,13 @@ rte_eth_devargs_process_list(char *str, uint16_t
>*list, uint16_t *len_list,
>>  }
>>
>>  /*
>> - * representor format:
>> + * Parse representor ports, expand and update representor port ID.
>> + * Representor format:
>>   *   #: range or single number of VF representor - legacy
>>   *   [[c#]pf#]vf#: VF port representor/s
>>   *   [[c#]pf#]sf#: SF port representor/s
>> + *
>> + * See RTE_ETH_REPR() for representor ID format.
>>   */
>>  int
>>  rte_eth_devargs_parse_representor_ports(char *str, void *data)
>> diff --git a/lib/librte_ethdev/rte_class_eth.c
>b/lib/librte_ethdev/rte_class_eth.c
>> index efe6149df5..994db96960 100644
>> --- a/lib/librte_ethdev/rte_class_eth.c
>> +++ b/lib/librte_ethdev/rte_class_eth.c
>> @@ -66,8 +66,8 @@ eth_representor_cmp(const char *key __rte_unused,
>>         int ret;
>>         char *values;
>>         const struct rte_eth_dev_data *data = opaque;
>> -       struct rte_eth_devargs representors;
>> -       uint16_t index;
>> +       struct rte_eth_devargs eth_da;
>> +       uint16_t index, c, p, f;
>>
>>         if ((data->dev_flags & RTE_ETH_DEV_REPRESENTOR) == 0)
>>                 return -1; /* not a representor port */
>> @@ -76,17 +76,39 @@ eth_representor_cmp(const char *key __rte_unused,
>>         values = strdup(value);
>>         if (values == NULL)
>>                 return -1;
>> -       memset(&representors, 0, sizeof(representors));
>> -       ret = rte_eth_devargs_parse_representor_ports(values, &representors);
>> +       memset(&eth_da, 0, sizeof(eth_da));
>> +       ret = rte_eth_devargs_parse_representor_ports(values, &eth_da);
>>         free(values);
>>         if (ret != 0)
>>                 return -1; /* invalid devargs value */
>>
>> +       /* Set default values. */
>> +       if (eth_da.nb_mh_controllers == 0) {
>> +               eth_da.nb_mh_controllers = 1;
>> +               eth_da.mh_controllers[0] = 0;
>> +       }
>> +       if (eth_da.nb_ports == 0) {
>> +               eth_da.nb_ports = 1;
>> +               eth_da.ports[0] = 0;
>> +       }
>> +       if (eth_da.nb_representor_ports == 0) {
>> +               eth_da.nb_representor_ports = 1;
>> +               eth_da.representor_ports[0] = 0;
>> +       }
>>         /* Return 0 if representor id is matching one of the values. */
>> -       for (index = 0; index < representors.nb_representor_ports; index++)
>> -               if (data->representor_id ==
>> -                               representors.representor_ports[index])
>> -                       return 0;
>> +       for (c = 0; c < eth_da.nb_mh_controllers; ++c) {
>> +               for (p = 0; p < eth_da.nb_ports; ++p) {
>> +                       for (f = 0; f < eth_da.nb_representor_ports; ++f) {
>> +                               index = rte_eth_representor_id_encode(
>> +                                       eth_da.mh_controllers[c],
>> +                                       eth_da.ports[p],
>> +                                       eth_da.type,
>> +                                       eth_da.representor_ports[f]);
>> +                               if (data->representor_id == index)
>> +                                       return 0;
>> +                       }
>> +               }
>> +       }
>>         return -1; /* no match */
>>  }
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index 2ac51ac149..276f42e54b 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -5556,6 +5556,32 @@ rte_eth_devargs_parse(const char *dargs, struct
>rte_eth_devargs *eth_da)
>>         return result;
>>  }
>>
>> +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;
>> +}
>> +
>>  static int
>>  eth_dev_handle_port_list(const char *cmd __rte_unused,
>>                 const char *params __rte_unused,
>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h
>b/lib/librte_ethdev/rte_ethdev_driver.h
>> index 8e04634660..0d8893693e 100644
>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>> @@ -1218,6 +1218,59 @@ struct rte_eth_devargs {
>>         enum rte_eth_representor_type type; /* type of representor */
>>  };
>>
>> +#define RTE_NO_REPRESENTOR_ID UINT16_MAX /**< No representor ID.
>*/
>> +
>> +/**
>> + * PMD helper function to encode representor ID
>> + *
>> + * The compact format is used for device iterator that comparing
>> + * ethdev representor ID with target devargs.
>> + *
>> + * xxxx xxxx xxxx xxxx
>> + * |||| |LLL LLLL LLLL vf/sf id
>> + * |||| L 1:sf, 0:vf
>> + * ||LL pf id
>> + * LL controller(host) id
>> + *
>> + * @param controller
>> + *  Controller ID.
>> + * @param pf
>> + *  PF port ID.
>> + * @param type
>> + *  Representor type.
>> + * @param representor_port
>> + *  Representor port ID.
>> + *
>> + * @return
>> + *   Encoded representor ID.
>> + */
>> +__rte_internal
>> +uint16_t
>> +rte_eth_representor_id_encode(uint16_t controller, uint16_t pf,
>> +                             enum rte_eth_representor_type type,
>> +                             uint16_t representor_port);
>> +
>> +/**
>> + * PMD helper function to parse representor ID
>> + *
>> + * @param representor_id
>> + *  Representor ID.
>> + * @param controller
>> + *  Parsed controller ID.
>> + * @param pf
>> + *  Parsed PF port ID.
>> + * @param type
>> + *  Parsed representor type.
>> + *
>> + * @return
>> + *   Parsed representor port ID.
>> + */
>> +__rte_internal
>> +uint16_t
>> +rte_eth_representor_id_parse(const uint16_t representor_id,
>> +                            uint16_t *controller, uint16_t *pf,
>> +                            enum rte_eth_representor_type *type);
>> +
>>  /**
>>   * PMD helper function to parse ethdev arguments
>>   *
>> diff --git a/lib/librte_ethdev/version.map b/lib/librte_ethdev/version.map
>> index d3f5410806..44edaed507 100644
>> --- a/lib/librte_ethdev/version.map
>> +++ b/lib/librte_ethdev/version.map
>> @@ -257,6 +257,8 @@ INTERNAL {
>>         rte_eth_dev_release_port;
>>         rte_eth_dev_internal_reset;
>>         rte_eth_devargs_parse;
>> +       rte_eth_representor_id_encode;
>> +       rte_eth_representor_id_parse;
>>         rte_eth_dma_zone_free;
>>         rte_eth_dma_zone_reserve;
>>         rte_eth_hairpin_queue_peer_bind;
>> --
>> 2.25.1
>>


More information about the dev mailing list