Re: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5

Re: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5

[Leon Romanovsky]

On Fri, May 13, 2022 at 05:46:16PM +0000, Rohit Sajan Kumar wrote:
> Hey,
> 
> Sending this as a gentle reminder to review the patch sent earlier this week which can be found in this email chain.

Patches that sent in HTML format, to wrong addresses and not visible
in patchworks/ML, without target net-next/rdma-next/e.t.c., with wrong
title are generally ignored.

Why vhca_id that returned from MLX5_IB_METHOD_QUERY_PORT is not enough?

Anyway, sysfs file in IB driver for the property of mlx5_core is no-go.

Thanks

> 
> Thank you.
> 
> Best,
> Rohit.
> ________________________________
> From: Rohit Nair <rohit.sajan.kumar@oracle.com>
> Sent: Monday, May 9, 2022 4:00 PM
> To: matanb@mellanox.com <matanb@mellanox.com>; leonro@mellanox.com <leonro@mellanox.com>; dledford@redhat.com <dledford@redhat.com>; sean.hefty@intel.com <sean.hefty@intel.com>; hal.rosenstock@gmail.com <hal.rosenstock@gmail.com>; Rohit Sajan Kumar <rohit.sajan.kumar@oracle.com>; Manjunath Patil <manjunath.b.patil@oracle.com>; Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
> Cc: linux-rdma@vger.kernel.org <linux-rdma@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> Subject: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5
> 
> While collecting diagnostic information (Ex:wqdump) in virtual
> environment, we need vhca id to collect data belonging a particular VF.
> This patch adds a sysfs entry to show the vhca id inside guest.
> 
> Signed-off-by: Rohit Nair <rohit.sajan.kumar@oracle.com>
> ---
>  drivers/infiniband/hw/mlx5/main.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index 32a0ea8..dd935bc 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -2499,12 +2499,24 @@ static ssize_t board_id_show(struct device *device,
>  }
>  static DEVICE_ATTR_RO(board_id);
> 
> +static ssize_t vhca_id_show(struct device *device,
> +                           struct device_attribute *attr, char *buf)
> +{
> +       struct mlx5_ib_dev *dev =
> +               container_of(device, struct mlx5_ib_dev, ib_dev.dev);
> +       return sysfs_emit(buf, "%d [0x%x]\n",
> +                      MLX5_CAP_GEN(dev->mdev, vhca_id),
> +                      MLX5_CAP_GEN(dev->mdev, vhca_id));
> +}
> +static DEVICE_ATTR_RO(vhca_id);
> +
>  static struct attribute *mlx5_class_attributes[] = {
>          &dev_attr_hw_rev.attr,
>          &dev_attr_hca_type.attr,
>          &dev_attr_board_id.attr,
>          &dev_attr_fw_pages.attr,
>          &dev_attr_reg_pages.attr,
> +       &dev_attr_vhca_id.attr,
>          NULL,
>  };
> 
> --
> 1.8.3.1
> 

Re: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5

[Mark Bloch]

On 5/14/2022 08:09, Leon Romanovsky wrote:
> On Fri, May 13, 2022 at 05:46:16PM +0000, Rohit Sajan Kumar wrote:
>> Hey,
>>
>> Sending this as a gentle reminder to review the patch sent earlier this week which can be found in this email chain.
> 
> Patches that sent in HTML format, to wrong addresses and not visible
> in patchworks/ML, without target net-next/rdma-next/e.t.c., with wrong
> title are generally ignored.
> 
> Why vhca_id that returned from MLX5_IB_METHOD_QUERY_PORT is not enough?

The driver returns that only when in switchdev mode.
I don't see why that can't be changed but that's the state today.

> 
> Anyway, sysfs file in IB driver for the property of mlx5_core is no-go.

For vhca_id a nicer route is probably to use DEVX and and run a mlx5dv_devx_general_cmd()
where you query HCA_CAP and take vhca_id from there. The added bonus is that
there is no change required in rdma-core/kernel.

Mark 

> 
> Thanks
> 
>>
>> Thank you.
>>
>> Best,
>> Rohit.
>> ________________________________
>> From: Rohit Nair <rohit.sajan.kumar@oracle.com>
>> Sent: Monday, May 9, 2022 4:00 PM
>> To: matanb@mellanox.com <matanb@mellanox.com>; leonro@mellanox.com <leonro@mellanox.com>; dledford@redhat.com <dledford@redhat.com>; sean.hefty@intel.com <sean.hefty@intel.com>; hal.rosenstock@gmail.com <hal.rosenstock@gmail.com>; Rohit Sajan Kumar <rohit.sajan.kumar@oracle.com>; Manjunath Patil <manjunath.b.patil@oracle.com>; Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
>> Cc: linux-rdma@vger.kernel.org <linux-rdma@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
>> Subject: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5
>>
>> While collecting diagnostic information (Ex:wqdump) in virtual
>> environment, we need vhca id to collect data belonging a particular VF.
>> This patch adds a sysfs entry to show the vhca id inside guest.
>>
>> Signed-off-by: Rohit Nair <rohit.sajan.kumar@oracle.com>
>> ---
>>  drivers/infiniband/hw/mlx5/main.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
>> index 32a0ea8..dd935bc 100644
>> --- a/drivers/infiniband/hw/mlx5/main.c
>> +++ b/drivers/infiniband/hw/mlx5/main.c
>> @@ -2499,12 +2499,24 @@ static ssize_t board_id_show(struct device *device,
>>  }
>>  static DEVICE_ATTR_RO(board_id);
>>
>> +static ssize_t vhca_id_show(struct device *device,
>> +                           struct device_attribute *attr, char *buf)
>> +{
>> +       struct mlx5_ib_dev *dev =
>> +               container_of(device, struct mlx5_ib_dev, ib_dev.dev);
>> +       return sysfs_emit(buf, "%d [0x%x]\n",
>> +                      MLX5_CAP_GEN(dev->mdev, vhca_id),
>> +                      MLX5_CAP_GEN(dev->mdev, vhca_id));
>> +}
>> +static DEVICE_ATTR_RO(vhca_id);
>> +
>>  static struct attribute *mlx5_class_attributes[] = {
>>          &dev_attr_hw_rev.attr,
>>          &dev_attr_hca_type.attr,
>>          &dev_attr_board_id.attr,
>>          &dev_attr_fw_pages.attr,
>>          &dev_attr_reg_pages.attr,
>> +       &dev_attr_vhca_id.attr,
>>          NULL,
>>  };
>>
>> --
>> 1.8.3.1
>>

Re: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5

[Leon Romanovsky]

On Sun, May 15, 2022 at 10:36:07AM +0300, Mark Bloch wrote:
> 
> 
> On 5/14/2022 08:09, Leon Romanovsky wrote:
> > On Fri, May 13, 2022 at 05:46:16PM +0000, Rohit Sajan Kumar wrote:
> >> Hey,
> >>
> >> Sending this as a gentle reminder to review the patch sent earlier this week which can be found in this email chain.
> > 
> > Patches that sent in HTML format, to wrong addresses and not visible
> > in patchworks/ML, without target net-next/rdma-next/e.t.c., with wrong
> > title are generally ignored.
> > 
> > Why vhca_id that returned from MLX5_IB_METHOD_QUERY_PORT is not enough?
> 
> The driver returns that only when in switchdev mode.
> I don't see why that can't be changed but that's the state today.

My guess is that it is not needed outside of switchdev mode. This is why
I would like to know why current solution is not enough.

Thanks

Re: [PATCH] net/mlx5: Add sysfs entry for vhca to /sys/class/infiniband/mlx5

[Mark Bloch]

On 5/18/22 07:49, Leon Romanovsky wrote:
> On Sun, May 15, 2022 at 10:36:07AM +0300, Mark Bloch wrote:
>>
>>
>> On 5/14/2022 08:09, Leon Romanovsky wrote:
>>> On Fri, May 13, 2022 at 05:46:16PM +0000, Rohit Sajan Kumar wrote:
>>>> Hey,
>>>>
>>>> Sending this as a gentle reminder to review the patch sent earlier this week which can be found in this email chain.
>>>
>>> Patches that sent in HTML format, to wrong addresses and not visible
>>> in patchworks/ML, without target net-next/rdma-next/e.t.c., with wrong
>>> title are generally ignored.
>>>
>>> Why vhca_id that returned from MLX5_IB_METHOD_QUERY_PORT is not enough?
>>
>> The driver returns that only when in switchdev mode.
>> I don't see why that can't be changed but that's the state today.
> 
> My guess is that it is not needed outside of switchdev mode. This is why
> I would like to know why current solution is not enough.
> 

The commit message says this API will be used inside a guest.
So they have a PCI function inside a VM (could be VF/PF etc.)
and they would like to get the vhca_id of said function.

The current info is filled only when in switchdev mode and only
when run on the eswitch manager. If they want to run it on a VF
it doesn't matter if the eswitch manager is in switchdev mode or
not, as the VF doesn't know what mode is used.

The query port method was added as there is info (like metadata c0 value and more)
that is filled by the kernel and DEVX can't help with that.

As vhca_id can be retrieved via DEVX and requires no userspace/kernel changes
it seems like the best option.

> Thanks
> 

Mark