All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Ratheesh Kannoth <rkannoth@marvell.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	sgoutham@marvell.com, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, donald.hunter@gmail.com,
	jiri@resnulli.us, chuck.lever@oracle.com, matttbe@kernel.org,
	cjubran@nvidia.com, shshitrit@nvidia.com, dtatulea@nvidia.com,
	tariqt@nvidia.com, Saeed Mahameed <saeedm@nvidia.com>
Subject: Re: [PATCH v7 net-next 2/5] devlink: Implement devlink param multi attribute nested data values
Date: Tue, 24 Mar 2026 16:09:17 +0000	[thread overview]
Message-ID: <20260324160917.GB111839@horms.kernel.org> (raw)
In-Reply-To: <20260323035110.3908741-3-rkannoth@marvell.com>

On Mon, Mar 23, 2026 at 09:21:07AM +0530, Ratheesh Kannoth wrote:
> From: Saeed Mahameed <saeedm@nvidia.com>
> 
> Devlink param value attribute is not defined since devlink is handling
> the value validating and parsing internally, this allows us to implement
> multi attribute values without breaking any policies.
> 
> Devlink param multi-attribute values are considered to be dynamically
> sized arrays of u64 values, by introducing a new devlink param type
> DEVLINK_PARAM_TYPE_U64_ARRAY, driver and user space can set a variable
> count of u32 values into the DEVLINK_ATTR_PARAM_VALUE_DATA attribute.
> 
> Implement get/set parsing and add to the internal value structure passed
> to drivers.
> 
> This is useful for devices that need to configure a list of values for
> a specific configuration.
> 
> example:
> $ devlink dev param show pci/... name multi-value-param
> name multi-value-param type driver-specific
> values:
> cmode permanent value: 0,1,2,3,4,5,6,7
> 
> $ devlink dev param set pci/... name multi-value-param \
> 		value 4,5,6,7,0,1,2,3 cmode permanent
> 
> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

...

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c b/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c
> index 2eb666a46f39..e96dfe322c14 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/pcie_cong_event.c
> @@ -43,7 +43,6 @@ struct mlx5e_pcie_cong_event {
>  	struct mlx5e_pcie_cong_stats stats;
>  };
>  
> -
>  static const struct counter_desc mlx5e_pcie_cong_stats_desc[] = {
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_pcie_cong_stats,
>  			     pci_bw_inbound_high) },

The above hunk seems to have been included by mistake.

> @@ -259,7 +258,11 @@ mlx5e_pcie_cong_get_thresh_config(struct mlx5_core_dev *dev,
>  		MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH,
>  	};
>  	struct devlink *devlink = priv_to_devlink(dev);
> -	union devlink_param_value val[4];
> +	union devlink_param_value *val;
> +
> +	val = kcalloc(4, sizeof(*val), GFP_KERNEL);
> +	if (!val)
> +		return -ENOMEM;
>  
>  	for (int i = 0; i < 4; i++) {
>  		u32 id = ids[i];

The lines following this hunk are:

		int err;

		err = devl_param_driverinit_value_get(devlink, id, &val[i]);
		if (err)
			return err;

AI review points out that if the error condition above is reached,
then val will be leaked.

> @@ -275,6 +278,7 @@ mlx5e_pcie_cong_get_thresh_config(struct mlx5_core_dev *dev,
>  	config->outbound_low = val[2].vu16;
>  	config->outbound_high = val[3].vu16;
>  
> +	kfree(val);
>  	return 0;
>  }
>  

Overall I would suggest breaking the mlx5 driver changes out into a
separate preparatory patch, which precedes the devlink patch in the
patch-set.

...

-- 
pw-bot: changes-requested

  reply	other threads:[~2026-03-24 16:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-23  3:51 [PATCH v7 net-next 0/5] octeontx2-af: npc: Enhancements Ratheesh Kannoth
2026-03-23  3:51 ` [PATCH v7 net-next 1/5] octeontx2-af: npc: cn20k: debugfs enhancements Ratheesh Kannoth
2026-03-23  3:51 ` [PATCH v7 net-next 2/5] devlink: Implement devlink param multi attribute nested data values Ratheesh Kannoth
2026-03-24 16:09   ` Simon Horman [this message]
2026-03-23  3:51 ` [PATCH v7 net-next 3/5] octeontx2-af: npc: cn20k: add subbank search order control Ratheesh Kannoth
2026-03-23  3:51 ` [PATCH v7 net-next 4/5] octeontx2-af: npc: cn20k: dynamically allocate and free default MCAM entries Ratheesh Kannoth
2026-03-23  3:51 ` [PATCH v7 net-next 5/5] octeontx2-af: npc: Support for custom KPU profile from filesystem Ratheesh Kannoth
2026-03-24 16:36   ` Simon Horman
2026-03-25  3:14     ` Ratheesh Kannoth
2026-03-25 16:42       ` Simon Horman

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=20260324160917.GB111839@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=cjubran@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=dtatulea@nvidia.com \
    --cc=edumazet@google.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matttbe@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rkannoth@marvell.com \
    --cc=saeedm@nvidia.com \
    --cc=sgoutham@marvell.com \
    --cc=shshitrit@nvidia.com \
    --cc=tariqt@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.