Re: [PATCH net-next 08/10] net/mlx5e: Recreate netdev channels on data direct device unbind

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net/mlx5e: unhandled mlx5e_safe_switch_params() failure leaves queues using unbound data direct device
- [High] net/mlx5e: missing devmem revoke hook causes IOMMU faults for dmabuf bindings on unbind
--

commit e6199c36bc278fa5fdfb415d4fa28dbdfc733a06
Author: Dragos Tatulea <dtatulea@nvidia.com>
Subject: net/mlx5e: Recreate netdev channels on data direct device unbind

This commit handles the unbinding of a data direct device by disabling
the data direct flag and triggering channel recreation for the affected
netdevs. It intends to stop queues from using data direct resources.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6144,9 +6144,28 @@ static int mlx5e_data_direct_event(struct notifier_block *nb,

[ ... ]

> +	new_params = priv->channels.params;
> +	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);
> +	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);

[Severity: High]
Is it safe to ignore the return value of mlx5e_safe_switch_params() here?

If this function fails (for example, due to an allocation failure when setting
up the new channels), the parameter switch is aborted and the currently active
channels are left untouched. They will continue to run with the data direct
flag enabled.

Since the driver proceeds with the unbind process while the active queues
still reference the soon-to-be-destroyed data direct resources, could the
hardware access invalid resources and cause use-after-free or crashes?

[Severity: High]
Additionally, since the newly recreated channels inherit existing bindings
via rxq->mp_params, won't they continue to post DMA addresses mapped in
the unbound data direct IOMMU domain to the primary PF?

The commit message notes this will cause IOMMU faults because a devmem
revoke hook is needed to close those bindings before the switch. Does this
leave the automatic unbind path fundamentally broken when dmabuf bindings
are present?

> +	netdev_info(priv->netdev,
> +		    "data_direct device unbound, disabled data_direct\n");
> +
> +out:
> +	mutex_unlock(&priv->state_lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008132815.654147-1-tariqt@nvidia.com?part=8