On Mon, Jan 18, 2021 at 3:18 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 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 > Acked-by: Viacheslav Ovsiienko 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. > --- > 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(ð_da, 0, sizeof(eth_da)); > + ret = rte_eth_devargs_parse_representor_ports(values, ð_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 >