linux-kernel - Re: [PATCH net-next] net/mlx5: Support devlink port state for host PF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <722cba76-363e-4273-8150-2b50c9f591a5@redhat.com>
Date: Thu, 5 Feb 2026 16:57:06 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Tariq Toukan <tariqt@...dia.com>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>
Cc: Saeed Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>,
 Mark Bloch <mbloch@...dia.com>, netdev@...r.kernel.org,
 linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
 Gal Pressman <gal@...dia.com>, Moshe Shemesh <moshe@...dia.com>,
 Or Har-Toov <ohartoov@...dia.com>, Jiri Pirko <jiri@...dia.com>,
 Parav Pandit <parav@...dia.com>
Subject: Re: [PATCH net-next] net/mlx5: Support devlink port state for host PF

On 2/3/26 11:24 AM, Tariq Toukan wrote:
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index 4b7a1ce7f406..5fbfabe28bdb 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -1304,24 +1304,52 @@ static int mlx5_eswitch_load_ec_vf_vports(struct mlx5_eswitch *esw, u16 num_ec_v
>  	return err;
>  }
>  
> -static int host_pf_enable_hca(struct mlx5_core_dev *dev)
> +int mlx5_esw_host_pf_enable_hca(struct mlx5_core_dev *dev)
>  {
> -	if (!mlx5_core_is_ecpf(dev))
> +	struct mlx5_eswitch *esw = dev->priv.eswitch;
> +	struct mlx5_vport *vport;
> +	int err;
> +
> +	if (!mlx5_core_is_ecpf(dev) || !mlx5_esw_allowed(esw))
>  		return 0;

I was able to miss the AI feedback here:

---
The old host_pf_enable_hca() only checked mlx5_core_is_ecpf(dev) before
calling mlx5_cmd_host_pf_enable_hca(). The new function adds a check for
mlx5_esw_allowed(esw), which returns false when esw is NULL or when the
device is not an eswitch manager.

When called from mlx5_host_pf_init() in ecpf.c on an ECPF device that is
not an eswitch manager (the path when mlx5_ecpf_esw_admins_host_pf()
returns false), this new condition will cause the function to return 0
without enabling the HCA.

Is this behavior change intentional? The old code would enable the HCA
in this configuration, but the new code skips it.

The same concern applies to mlx5_esw_host_pf_disable_hca() below.
---

and indeed it looks relevant. I think you have to follow-up or send a
revert, whatever it's easier/faster.

> @@ -1347,7 +1375,7 @@ mlx5_eswitch_enable_pf_vf_vports(struct mlx5_eswitch *esw,
>  
>  	if (mlx5_esw_host_functions_enabled(esw->dev)) {
>  		/* Enable external host PF HCA */
> -		ret = host_pf_enable_hca(esw->dev);
> +		ret = mlx5_esw_host_pf_enable_hca(esw->dev);

Just FTR, more AI feedback here:

---
The old host_pf_disable_hca() was a void function. The new
mlx5_esw_host_pf_disable_hca() returns int and can fail, but the return
value is not checked here in the error path.

If mlx5_esw_host_pf_disable_hca() fails, it returns without setting
vport->pf_activated = false. This leaves pf_activated set to true even
though the HCA state may be inconsistent.

Later, mlx5_devlink_pf_port_fn_state_get() reads vport->pf_activated to
report state to userspace, which could then report incorrect state.

Should the return value be checked, or should the pf_activated flag be
updated unconditionally to reflect the intended state?

The same pattern appears in mlx5_eswitch_disable_pf_vf_vports().
---