diff mbox series

[v5] ethdev: fix representor port ID search by name

Message ID 20210913112633.2836730-1-andrew.rybchenko@oktetlabs.ru (mailing list archive)
State Superseded
Delegated to: Ferruh Yigit
Headers show
Series [v5] ethdev: fix representor port ID search by name | expand

Checks

Context Check Description
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing fail Testing issues
ci/Intel-compilation success Compilation OK
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/checkpatch warning coding style issues

Commit Message

Andrew Rybchenko Sept. 13, 2021, 11:26 a.m. UTC
From: Viacheslav Galaktionov <viacheslav.galaktionov@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.galaktionov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Haiyue Wang <haiyue.wang@intel.com>
Acked-by: Beilei Xing <beilei.xing@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(-)

Comments

Aman Singh Sept. 29, 2021, 11:13 a.m. UTC | #1
On 9/13/2021 4:56 PM, Andrew Rybchenko wrote:
> From: Viacheslav Galaktionov <viacheslav.galaktionov@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.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Haiyue Wang <haiyue.wang@intel.com>
> Acked-by: Beilei Xing <beilei.xing@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.
Did we change name of new field from parert_port_id to backer_port_id.
>
> 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(&eth_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 */
Andrew Rybchenko Sept. 30, 2021, 12:03 p.m. UTC | #2
On 9/29/21 2:13 PM, Singh, Aman Deep wrote:
> 
> On 9/13/2021 4:56 PM, Andrew Rybchenko wrote:
>> From: Viacheslav Galaktionov <viacheslav.galaktionov@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.galaktionov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Acked-by: Haiyue Wang <haiyue.wang@intel.com>
>> Acked-by: Beilei Xing <beilei.xing@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.
> Did we change name of new field from parert_port_id to backer_port_id.

Yes, see v5 changelog below.
It is done to address review notes from Ferruh on v4.

>>
>> 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

[snip]
Aman Singh Sept. 30, 2021, 12:51 p.m. UTC | #3
On 9/30/2021 5:33 PM, Andrew Rybchenko wrote:
> On 9/29/21 2:13 PM, Singh, Aman Deep wrote:
>> On 9/13/2021 4:56 PM, Andrew Rybchenko wrote:
>>> From: Viacheslav Galaktionov <viacheslav.galaktionov@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.galaktionov@oktetlabs.ru>
>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> Acked-by: Haiyue Wang <haiyue.wang@intel.com>
>>> Acked-by: Beilei Xing <beilei.xing@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.
>> Did we change name of new field from parert_port_id to backer_port_id.
> Yes, see v5 changelog below.
> It is done to address review notes from Ferruh on v4.

Maybe I did not put it clearly, my bad. Just wanted, in above lines also 
the usage
of "parert_port_id" should be changed.

>
>>> 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
> [snip]
>
Andrew Rybchenko Sept. 30, 2021, 1:40 p.m. UTC | #4
On 9/30/21 3:51 PM, Singh, Aman Deep wrote:
> 
> On 9/30/2021 5:33 PM, Andrew Rybchenko wrote:
>> On 9/29/21 2:13 PM, Singh, Aman Deep wrote:
>>> On 9/13/2021 4:56 PM, Andrew Rybchenko wrote:
>>>> From: Viacheslav Galaktionov <viacheslav.galaktionov@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.galaktionov@oktetlabs.ru>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Acked-by: Haiyue Wang <haiyue.wang@intel.com>
>>>> Acked-by: Beilei Xing <beilei.xing@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.
>>> Did we change name of new field from parert_port_id to backer_port_id.
>> Yes, see v5 changelog below.
>> It is done to address review notes from Ferruh on v4.
> 
> Maybe I did not put it clearly, my bad. Just wanted, in above lines also
> the usage
> of "parert_port_id" should be changed.

Thanks, I'll fix it in v6, but I think it does not worse to
respin it since it is not a part of description. Just extra
information.

>>
>>>> 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
>> [snip]
>>
Andrew Rybchenko Oct. 1, 2021, 11:39 a.m. UTC | #5
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.galaktionov@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.galaktionov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Haiyue Wang <haiyue.wang@intel.com>
> Acked-by: Beilei Xing <beilei.xing@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(&eth_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 */
>
Thomas Monjalon Oct. 5, 2021, 9:56 p.m. UTC | #6
13/09/2021 13:26, Andrew Rybchenko:
> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> 
> Getting a list of representors from a representor does not make sense.
> Instead, a parent device should be used.

I don't understand which issue it is fixing.
This function was not working before if not using the backer port?
Is it fixing a specific PMD?
Andrew Rybchenko Oct. 7, 2021, 10:20 a.m. UTC | #7
On 10/6/21 12:56 AM, Thomas Monjalon wrote:
> 13/09/2021 13:26, Andrew Rybchenko:
>> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
>>
>> Getting a list of representors from a representor does not make sense.
>> Instead, a parent device should be used.
> 
> I don't understand which issue it is fixing.
> This function was not working before if not using the backer port?

The function, rte_eth_representor_id_get(), is used in
eth_representor_cmp() which is required in ethdev class
iterator to search ethdev port ID by name (representor case).
Before the patch the function is called on the representor
itself it tries to get representors info to match.

It was found by OvS+DPDK testing:
 1. OvS hotplugs representor
 2. OvS tries to find DPDK port ID for just hotplugged
    representor and fails (if PMD does not provider
    representors info on the representor itself)

> Is it fixing a specific PMD?

It is a generic fix for PMD which do *not* provide representors
info on the representor itself.
Thomas Monjalon Oct. 7, 2021, 12:39 p.m. UTC | #8
07/10/2021 12:20, Andrew Rybchenko:
> On 10/6/21 12:56 AM, Thomas Monjalon wrote:
> > 13/09/2021 13:26, Andrew Rybchenko:
> >> From: Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
> >>
> >> Getting a list of representors from a representor does not make sense.
> >> Instead, a parent device should be used.
> > 
> > I don't understand which issue it is fixing.
> > This function was not working before if not using the backer port?
> 
> The function, rte_eth_representor_id_get(), is used in
> eth_representor_cmp() which is required in ethdev class
> iterator to search ethdev port ID by name (representor case).
> Before the patch the function is called on the representor
> itself it tries to get representors info to match.
> 
> It was found by OvS+DPDK testing:
>  1. OvS hotplugs representor
>  2. OvS tries to find DPDK port ID for just hotplugged
>     representor and fails (if PMD does not provider
>     representors info on the representor itself)
> 
> > Is it fixing a specific PMD?
> 
> It is a generic fix for PMD which do *not* provide representors
> info on the representor itself.

This is the key information.
Please reword the commit log.
Xueming(Steven) Li Oct. 8, 2021, 8:39 a.m. UTC | #9
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.galaktionov@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.galaktionov@oktetlabs.ru>
> > Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Acked-by: Haiyue Wang <haiyue.wang@intel.com>
> > Acked-by: Beilei Xing <beilei.xing@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(&eth_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 <xuemingl@nvidia.com>
diff mbox series

Patch

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(&eth_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 */