Re: [dpdk-dev] [PATCH v3 1/2] common/mlx5: add provider query port support to glue library

[Thomas Monjalon <thomas@monjalon.net>]

01/07/2021 09:15, Viacheslav Ovsiienko:
> In order to get E-Switch vport identifiers the mlx5 PMD relies
> on two approaches:
>   - use port query API if it is provided by rdma-core library
>   - otherwise, deduce vport ids from the related VF index
> The latter is not reliable and may not work with newer kernel
> driver versions and in some configurations (LAG), causing
> the E-Switch malfunction. Hence, engaging the port query API
> is highly desirable.
> 
> The port attributes query API was supported by vendor rdma-core
> versions only (in OFED). In Upstream, the rdma-core mlx5 provider
> introduced the port attributes query API since version v35.0 - the
> mlx5dv_query_port routine, that differs from vendor API. In order
> to support this change in the rdma-core the conditional compilation
> flag HAVE_MLX5DV_DR_DEVX_PORT_V35 is introduced by the this patch.
> 
> In the OFED rdma-core version the new compatible mlx5dv_query_port
> routine was introduced as well, replacing the existing proprietary
> mlx5dv_query_devx_port routine. The proprietary routine is still
> controlled in PMD code with HAVE_MLX5DV_DR_DEVX_PORT conditional
> flag.
> 
> Currently, the OFED rdma-core library contains both versions of
> port query API. And this version is a transitional one, there are

"this version" of what?

> the plans to remove the proprietary mlx5dv_query_devx_port routine
> and the HAVE_MLX5DV_DR_DEVX_PORT flag in PMD will not work anymore.
> 
> With this patch applied the mlx5 PMD can be compiled and operates
> over all rdma-core versions:
>   - Upstream rdma-core before v35 (deduces vport id from VF name)
>                            (* E-Switch may not operate correctly)
>   - Upstream rdma-core v35 and above (uses common port query API)
>   - rdma-core OFED before 5.5 (uses proprietary port query API)
>   - rdma-core OFED 5.5 and above (uses common port query API)

This is the kind of explanation which should come first,
with exact function and flag names, before and after.

> Despite this patch is not a bugfix (it follows the introduced API
> variation in underlying library), it covers the major compatibility
> issue and is highly desired to be ported to DPDK LTS.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
[...]
> --- a/drivers/common/mlx5/linux/meson.build
> +++ b/drivers/common/mlx5/linux/meson.build
> @@ -93,6 +93,8 @@ has_sym_args = [
>              'IBV_WQ_FLAG_RX_END_PADDING' ],
>          [ 'HAVE_MLX5DV_DR_DEVX_PORT', 'infiniband/mlx5dv.h',
>              'mlx5dv_query_devx_port' ],
> +	[ 'HAVE_MLX5DV_DR_DEVX_PORT_V35', 'infiniband/mlx5dv.h',
> +	    'mlx5dv_query_port' ],

It seems there is an indentation issue.

>          [ 'HAVE_IBV_DEVX_OBJ', 'infiniband/mlx5dv.h',
>              'mlx5dv_devx_obj_create' ],
>          [ 'HAVE_IBV_FLOW_DEVX_COUNTERS', 'infiniband/mlx5dv.h',
[...]
> +		 * pf0vf2, we can deduce the VF number as 2, and set vport index
> +		 * as 3 (2+1). This assigning schema should be changed if the
> +		 * multiple E-Switch instances per PF configurations or/and PCI
> +		 * subfunctions are added.

I don't understand this last sentence.