On Fri, 2021-10-01 at 14:39 +0300, Andrew Rybchenko wrote: > Hello PMD maintainers, > > please, review the patch. > > It is especially important for net/mlx5 since changes there are > not trivial. > > Thanks, > Andrew. > > On 9/13/21 2:26 PM, Andrew Rybchenko wrote: > > From: Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru> > > > > Getting a list of representors from a representor does not make sense. > > Instead, a parent device should be used. > > > > To this end, extend the rte_eth_dev_data structure to include the port ID > > of the backing device for representors. > > > > Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktio...@oktetlabs.ru> > > Signed-off-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > > Acked-by: Haiyue Wang <haiyue.w...@intel.com> > > Acked-by: Beilei Xing <beilei.x...@intel.com> > > --- > > The new field is added into the hole in rte_eth_dev_data structure. > > The patch does not change ABI, but extra care is required since ABI > > check is disabled for the structure because of the libabigail bug [1]. > > It should not be a problem anyway since 21.11 is a ABI breaking release. > > > > Potentially it is bad for out-of-tree drivers which implement > > representors but do not fill in a new parert_port_id field in > > rte_eth_dev_data structure. Get ID by name will not work. > > > > mlx5 changes should be reviwed by maintainers very carefully, since > > we are not sure if we patch it correctly. > > > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=28060 > > > > v5: > > - try to improve name: backer_port_id instead of parent_port_id > > - init new field to RTE_MAX_ETHPORTS on allocation to avoid > > zero port usage by default > > > > v4: > > - apply mlx5 review notes: remove fallback from generic ethdev > > code and add fallback to mlx5 code to handle legacy usecase > > > > v3: > > - fix mlx5 build breakage > > > > v2: > > - fix mlx5 review notes > > - try device port ID first before parent in order to address > > backward compatibility issue > > > > drivers/net/bnxt/bnxt_reps.c | 1 + > > drivers/net/enic/enic_vf_representor.c | 1 + > > drivers/net/i40e/i40e_vf_representor.c | 1 + > > drivers/net/ice/ice_dcf_vf_representor.c | 1 + > > drivers/net/ixgbe/ixgbe_vf_representor.c | 1 + > > drivers/net/mlx5/linux/mlx5_os.c | 13 +++++++++++++ > > drivers/net/mlx5/windows/mlx5_os.c | 13 +++++++++++++ > > lib/ethdev/ethdev_driver.h | 6 +++--- > > lib/ethdev/rte_class_eth.c | 2 +- > > lib/ethdev/rte_ethdev.c | 9 +++++---- > > lib/ethdev/rte_ethdev_core.h | 6 ++++++ > > 11 files changed, 46 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/bnxt/bnxt_reps.c b/drivers/net/bnxt/bnxt_reps.c > > index bdbad53b7d..0d50c0f1da 100644 > > --- a/drivers/net/bnxt/bnxt_reps.c > > +++ b/drivers/net/bnxt/bnxt_reps.c > > @@ -187,6 +187,7 @@ 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->backer_port_id = rep_params->parent_dev->data->port_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 79dd6e5640..fedb09ecd6 100644 > > --- a/drivers/net/enic/enic_vf_representor.c > > +++ b/drivers/net/enic/enic_vf_representor.c > > @@ -662,6 +662,7 @@ int enic_vf_representor_init(struct rte_eth_dev > > *eth_dev, void *init_params) > > 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->backer_port_id = pf->port_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 0481b55381..d65b821a01 100644 > > --- a/drivers/net/i40e/i40e_vf_representor.c > > +++ b/drivers/net/i40e/i40e_vf_representor.c > > @@ -514,6 +514,7 @@ 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->backer_port_id = pf->dev_data->port_id; > > > > /* Setting the number queues allocated to the VF */ > > ethdev->data->nb_rx_queues = vf->vsi->nb_qps; > > diff --git a/drivers/net/ice/ice_dcf_vf_representor.c > > b/drivers/net/ice/ice_dcf_vf_representor.c > > index 970461f3e9..e51d0aa6b9 100644 > > --- a/drivers/net/ice/ice_dcf_vf_representor.c > > +++ b/drivers/net/ice/ice_dcf_vf_representor.c > > @@ -418,6 +418,7 @@ ice_dcf_vf_repr_init(struct rte_eth_dev > > *vf_rep_eth_dev, void *init_param) > > > > vf_rep_eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; > > vf_rep_eth_dev->data->representor_id = repr->vf_id; > > + vf_rep_eth_dev->data->backer_port_id = repr->dcf_eth_dev->data->port_id; > > > > vf_rep_eth_dev->data->mac_addrs = &repr->mac_addr; > > > > diff --git a/drivers/net/ixgbe/ixgbe_vf_representor.c > > b/drivers/net/ixgbe/ixgbe_vf_representor.c > > index d5b636a194..9fa75984fb 100644 > > --- a/drivers/net/ixgbe/ixgbe_vf_representor.c > > +++ b/drivers/net/ixgbe/ixgbe_vf_representor.c > > @@ -197,6 +197,7 @@ ixgbe_vf_representor_init(struct rte_eth_dev *ethdev, > > void *init_params) > > > > ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; > > ethdev->data->representor_id = representor->vf_id; > > + ethdev->data->backer_port_id = representor->pf_ethdev->data->port_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 470b16cb9a..1cddaaba1a 100644 > > --- a/drivers/net/mlx5/linux/mlx5_os.c > > +++ b/drivers/net/mlx5/linux/mlx5_os.c > > @@ -1677,6 +1677,19 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev, > > if (priv->representor) { > > eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; > > eth_dev->data->representor_id = priv->representor_id; > > + MLX5_ETH_FOREACH_DEV(port_id, &priv->pci_dev->device) { > > + struct mlx5_priv *opriv = > > + rte_eth_devices[port_id].data->dev_private; > > + if (opriv && > > + opriv->master && > > + opriv->domain_id == priv->domain_id && > > + opriv->sh == priv->sh) { > > + eth_dev->data->backer_port_id = port_id; > > + break; > > + } > > + } > > + if (port_id >= RTE_MAX_ETHPORTS) > > + eth_dev->data->backer_port_id = eth_dev->data->port_id; > > } > > priv->mp_id.port_id = eth_dev->data->port_id; > > strlcpy(priv->mp_id.name, MLX5_MP_NAME, RTE_MP_MAX_NAME_LEN); > > diff --git a/drivers/net/mlx5/windows/mlx5_os.c > > b/drivers/net/mlx5/windows/mlx5_os.c > > index 26fa927039..a9c244c7dc 100644 > > --- a/drivers/net/mlx5/windows/mlx5_os.c > > +++ b/drivers/net/mlx5/windows/mlx5_os.c > > @@ -543,6 +543,19 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev, > > if (priv->representor) { > > eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; > > eth_dev->data->representor_id = priv->representor_id; > > + MLX5_ETH_FOREACH_DEV(port_id, &priv->pci_dev->device) { > > + struct mlx5_priv *opriv = > > + rte_eth_devices[port_id].data->dev_private; > > + if (opriv && > > + opriv->master && > > + opriv->domain_id == priv->domain_id && > > + opriv->sh == priv->sh) { > > + eth_dev->data->backer_port_id = port_id; > > + break; > > + } > > + } > > + if (port_id >= RTE_MAX_ETHPORTS) > > + eth_dev->data->backer_port_id = eth_dev->data->port_id; > > } > > /* > > * Store associated network device interface index. This index > > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > > index 40e474aa7e..b940e6cb38 100644 > > --- a/lib/ethdev/ethdev_driver.h > > +++ b/lib/ethdev/ethdev_driver.h > > @@ -1248,8 +1248,8 @@ struct rte_eth_devargs { > > * For backward compatibility, if no representor info, direct > > * map legacy VF (no controller and pf). > > * > > - * @param ethdev > > - * Handle of ethdev port. > > + * @param port_id > > + * Port ID of the backing device. > > * @param type > > * Representor type. > > * @param controller > > @@ -1266,7 +1266,7 @@ struct rte_eth_devargs { > > */ > > __rte_internal > > int > > -rte_eth_representor_id_get(const struct rte_eth_dev *ethdev, > > +rte_eth_representor_id_get(uint16_t port_id, > > enum rte_eth_representor_type type, > > int controller, int pf, int representor_port, > > uint16_t *repr_id); > > diff --git a/lib/ethdev/rte_class_eth.c b/lib/ethdev/rte_class_eth.c > > index 1fe5fa1f36..eda216ced5 100644 > > --- a/lib/ethdev/rte_class_eth.c > > +++ b/lib/ethdev/rte_class_eth.c > > @@ -95,7 +95,7 @@ eth_representor_cmp(const char *key __rte_unused, > > c = i / (np * nf); > > p = (i / nf) % np; > > f = i % nf; > > - if (rte_eth_representor_id_get(edev, > > + if (rte_eth_representor_id_get(edev->data->backer_port_id, > > eth_da.type, > > eth_da.nb_mh_controllers == 0 ? -1 : > > eth_da.mh_controllers[c], > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c > > index daf5ca9242..7c9b0d6b3b 100644 > > --- a/lib/ethdev/rte_ethdev.c > > +++ b/lib/ethdev/rte_ethdev.c > > @@ -524,6 +524,7 @@ rte_eth_dev_allocate(const char *name) > > eth_dev = eth_dev_get(port_id); > > strlcpy(eth_dev->data->name, name, sizeof(eth_dev->data->name)); > > eth_dev->data->port_id = port_id; > > + eth_dev->data->backer_port_id = RTE_MAX_ETHPORTS; > > eth_dev->data->mtu = RTE_ETHER_MTU; > > pthread_mutex_init(ð_dev->data->flow_ops_mutex, NULL); > > > > @@ -5996,7 +5997,7 @@ rte_eth_devargs_parse(const char *dargs, struct > > rte_eth_devargs *eth_da) > > } > > > > int > > -rte_eth_representor_id_get(const struct rte_eth_dev *ethdev, > > +rte_eth_representor_id_get(uint16_t port_id, > > enum rte_eth_representor_type type, > > int controller, int pf, int representor_port, > > uint16_t *repr_id) > > @@ -6012,7 +6013,7 @@ rte_eth_representor_id_get(const struct rte_eth_dev > > *ethdev, > > return -EINVAL; > > > > /* Get PMD representor range info. */ > > - ret = rte_eth_representor_info_get(ethdev->data->port_id, NULL); > > + ret = rte_eth_representor_info_get(port_id, NULL); > > if (ret == -ENOTSUP && type == RTE_ETH_REPRESENTOR_VF && > > controller == -1 && pf == -1) { > > /* Direct mapping for legacy VF representor. */ > > @@ -6027,7 +6028,7 @@ rte_eth_representor_id_get(const struct rte_eth_dev > > *ethdev, > > if (info == NULL) > > return -ENOMEM; > > info->nb_ranges_alloc = n; > > - ret = rte_eth_representor_info_get(ethdev->data->port_id, info); > > + ret = rte_eth_representor_info_get(port_id, info); > > if (ret < 0) > > goto out; > > > > @@ -6046,7 +6047,7 @@ rte_eth_representor_id_get(const struct rte_eth_dev > > *ethdev, > > continue; > > if (info->ranges[i].id_end < info->ranges[i].id_base) { > > RTE_LOG(WARNING, EAL, "Port %hu invalid representor ID > > Range %u - %u, entry %d\n", > > - ethdev->data->port_id, info->ranges[i].id_base, > > + port_id, info->ranges[i].id_base, > > info->ranges[i].id_end, i); > > continue; > > > > diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h > > index edf96de2dc..48b814e8a1 100644 > > --- a/lib/ethdev/rte_ethdev_core.h > > +++ b/lib/ethdev/rte_ethdev_core.h > > @@ -185,6 +185,12 @@ struct rte_eth_dev_data { > > /**< Switch-specific identifier. > > * Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags. > > */ > > + uint16_t backer_port_id; > > + /**< Port ID of the backing device. > > + * This device will be used to query representor > > + * info and calculate representor IDs. > > + * Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags. > > + */ > > > > pthread_mutex_t flow_ops_mutex; /**< rte_flow ops mutex. */ > > uint64_t reserved_64s[4]; /**< Reserved for future fields */ > > >
Reviewed-by: Xueming Li <xuemi...@nvidia.com>
