Re: [PATCH net-next 2/4] net/mlx5: Expose serial numbers in devlink info

[Simon Horman <horms@kernel.org>]

On Tue, Mar 18, 2025 at 04:36:25PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Devlink info allows to expose serial number and board serial number
> Get the values from PCI VPD and expose it.
> 
> $ devlink dev info
> pci/0000:08:00.0:
>   driver mlx5_core
>   serial_number e4397f872caeed218000846daa7d2f49
>   board.serial_number MT2314XZ00YA

Hi Jiri,

I'm sorry if this is is somehow obvious, but what is
the difference between the serial number and board serial number
(yes, I do see that they are different numbers :)

>   versions:
>       fixed:
>         fw.psid MT_0000000894
>       running:
>         fw.version 28.41.1000
>         fw 28.41.1000
>       stored:
>         fw.version 28.41.1000
>         fw 28.41.1000
> auxiliary/mlx5_core.eth.0:
>   driver mlx5_core.eth
> pci/0000:08:00.1:
>   driver mlx5_core
>   serial_number e4397f872caeed218000846daa7d2f49
>   board.serial_number MT2314XZ00YA
>   versions:
>       fixed:
>         fw.psid MT_0000000894
>       running:
>         fw.version 28.41.1000
>         fw 28.41.1000
>       stored:
>         fw.version 28.41.1000
>         fw 28.41.1000
> auxiliary/mlx5_core.eth.1:
>   driver mlx5_core.eth
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> ---
>  .../net/ethernet/mellanox/mlx5/core/devlink.c | 49 +++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> index 73cd74644378..be0ae26d1582 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> @@ -35,6 +35,51 @@ static u16 mlx5_fw_ver_subminor(u32 version)
>  	return version & 0xffff;
>  }
>  
> +static int mlx5_devlink_serial_numbers_put(struct mlx5_core_dev *dev,
> +					   struct devlink_info_req *req,
> +					   struct netlink_ext_ack *extack)
> +{
> +	struct pci_dev *pdev = dev->pdev;
> +	unsigned int vpd_size, kw_len;
> +	char *str, *end;
> +	u8 *vpd_data;
> +	int start;
> +	int err;
> +
> +	vpd_data = pci_vpd_alloc(pdev, &vpd_size);
> +	if (IS_ERR(vpd_data))
> +		return 0;
> +
> +	start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size,
> +					     PCI_VPD_RO_KEYWORD_SERIALNO, &kw_len);
> +	if (start >= 0) {
> +		str = kstrndup(vpd_data + start, kw_len, GFP_KERNEL);
> +		if (!str) {
> +			err = -ENOMEM;
> +			goto end;
> +		}
> +		end = strchrnul(str, ' ');
> +		*end = '\0';
> +		err = devlink_info_board_serial_number_put(req, str);
> +		kfree(str);
> +	}
> +
> +	start = pci_vpd_find_ro_info_keyword(vpd_data, vpd_size, "V3", &kw_len);
> +	if (start >= 0) {
> +		str = kstrndup(vpd_data + start, kw_len, GFP_KERNEL);
> +		if (!str) {
> +			err = -ENOMEM;
> +			goto end;
> +		}
> +		err = devlink_info_serial_number_put(req, str);
> +		kfree(str);
> +	}
> +
> +end:
> +	kfree(vpd_data);
> +	return err;

Perhaps it can never happen, but Smatch flags that err may be used
uninitialised here. I believe that can theoretically occur if
neither of start condition above are met.

> +}
> +

...