DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Singh, Aman Deep" <aman.deep.singh@intel.com>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Ajit Khaparde <ajit.khaparde@broadcom.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	John Daley <johndale@cisco.com>,
	Hyong Youb Kim <hyonkim@cisco.com>,
	"Beilei Xing" <beilei.xing@intel.com>,
	Qiming Yang <qiming.yang@intel.com>,
	Qi Zhang <qi.z.zhang@intel.com>,
	Haiyue Wang <haiyue.wang@intel.com>,
	Matan Azrad <matan@nvidia.com>,
	Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
	"Thomas Monjalon" <thomas@monjalon.net>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Cc: <dev@dpdk.org>,
	Viacheslav Galaktionov <viacheslav.galaktionov@oktetlabs.ru>
Subject: Re: [dpdk-dev] [PATCH v5] ethdev: fix representor port ID search by name
Date: Wed, 29 Sep 2021 16:43:18 +0530	[thread overview]
Message-ID: <2ddbed3f-6a65-6a6f-462a-4753860c9b2f@intel.com> (raw)
In-Reply-To: <20210913112633.2836730-1-andrew.rybchenko@oktetlabs.ru>


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

  reply	other threads:[~2021-09-29 11:13 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12 16:17 [dpdk-dev] [PATCH] " Andrew Rybchenko
2021-07-19  6:58 ` Xueming(Steven) Li
2021-07-19  8:46   ` Andrew Rybchenko
2021-07-19 11:54     ` Xueming(Steven) Li
2021-07-19 12:36       ` Andrew Rybchenko
2021-07-19 12:50         ` Xueming(Steven) Li
2021-07-20  8:59           ` Andrew Rybchenko
2021-07-29  4:13             ` Xueming(Steven) Li
2021-08-01  8:40               ` Andrew Rybchenko
2021-08-01 14:25                 ` Xueming(Steven) Li
2021-07-29  4:20 ` Xueming(Steven) Li
2021-08-01  8:50   ` Andrew Rybchenko
2021-08-01 14:15     ` Xueming(Steven) Li
2021-08-18 14:00 ` [dpdk-dev] [PATCH v2] " Andrew Rybchenko
2021-08-27  9:18   ` Xueming(Steven) Li
2021-08-27  9:48     ` Viacheslav Galaktionov
2021-08-28 13:22       ` Xueming(Steven) Li
2021-08-29  8:23         ` Andrew Rybchenko
2021-08-29 12:17           ` Xueming(Steven) Li
2021-08-31 15:42             ` Andrew Rybchenko
2021-08-20 12:18 ` [dpdk-dev] [PATCH v3] " Andrew Rybchenko
2021-08-31 15:41   ` Andrew Rybchenko
2021-08-31 16:06 ` [dpdk-dev] [PATCH v4] " Andrew Rybchenko
2021-08-31 16:32   ` Wang, Haiyue
2021-08-31 16:37     ` Andrew Rybchenko
2021-09-01  5:15   ` Xing, Beilei
2021-09-01 14:55   ` Ferruh Yigit
2021-09-06 16:16     ` Viacheslav Galaktionov
2021-09-13 11:26 ` [dpdk-dev] [PATCH v5] " Andrew Rybchenko
2021-09-29 11:13   ` Singh, Aman Deep [this message]
2021-09-30 12:03     ` Andrew Rybchenko
2021-09-30 12:51       ` Singh, Aman Deep
2021-09-30 13:40         ` Andrew Rybchenko
2021-10-01 11:39   ` Andrew Rybchenko
2021-10-08  8:39     ` Xueming(Steven) Li
2021-10-05 21:56   ` Thomas Monjalon
2021-10-07 10:20     ` Andrew Rybchenko
2021-10-07 12:39       ` Thomas Monjalon
2021-10-08  9:27 ` [dpdk-dev] [PATCH v6] " Andrew Rybchenko
2021-10-11  7:56   ` Slava Ovsiienko
2021-10-11 12:30 ` [dpdk-dev] [PATCH v7] " Andrew Rybchenko
2021-10-11 12:53 ` [dpdk-dev] [PATCH v8] " Andrew Rybchenko
2021-10-12 15:09   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ddbed3f-6a65-6a6f-462a-4753860c9b2f@intel.com \
    --to=aman.deep.singh@intel.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=haiyue.wang@intel.com \
    --cc=hyonkim@cisco.com \
    --cc=johndale@cisco.com \
    --cc=matan@nvidia.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=somnath.kotur@broadcom.com \
    --cc=thomas@monjalon.net \
    --cc=viacheslav.galaktionov@oktetlabs.ru \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).