Re: [PATCH net-next V2 13/13] net/mlx5e: Enable the data direct netdev feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net/mlx5e: stale MLX5E_PFLAG_DATA_DIRECT state after resume
  if dd_dev unbound
--

commit 2963efd7a308324cf29d1ee3e4d43832eae0f949
Author: Dragos Tatulea <dtatulea@nvidia.com>

net/mlx5e: Enable the data direct netdev feature

This commit implements the ethtool set_pflag_data_direct() callback to
validate device state and toggle MLX5E_PFLAG_DATA_DIRECT via
mlx5e_safe_switch_params().

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index 26f76449af351..2204b370f4395 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c

[ ... ]

> @@ -2407,7 +2410,65 @@ static int set_pflag_tx_port_ts(struct net_device *netdev, bool enable)
>  
>  static int set_pflag_data_direct(struct net_device *netdev, bool enable)
>  {
> -	return -EOPNOTSUPP;
> +	struct mlx5e_priv *priv = netdev_priv(netdev);
> +	struct mlx5_core_dev *mdev = priv->mdev;
> +	struct mlx5e_params new_params;
> +	unsigned int i;
> +
> +	if (enable) {
> +		if (!mlx5_data_direct_supported(mdev) ||
> +		    !MLX5_CAP_GEN(mdev, crossing_vhca_mkey)) {
> +			netdev_err(netdev, "data_direct is not supported by the device\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (!MLX5_CAP_GEN(mdev, shampo)) {
> +			netdev_err(netdev, "data_direct requires SHAMPO (Header-Data Split) support\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (mlx5_get_sd(mdev)) {
> +			netdev_err(netdev, "data_direct is not supported with socket direct\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (mlx5e_is_uplink_rep(priv)) {
> +			netdev_err(netdev, "data_direct is not supported for uplink representors\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (!mlx5_data_direct_get_dev(mdev->data_direct)) {
> +			netdev_err(netdev, "data_direct device not bound\n");
> +			return -ENODEV;
> +		}
> +
> +		if (mlx5e_has_leased_rxqs(netdev)) {
> +			netdev_err(netdev, "data_direct is not supported with leased RX queues\n");
> +			return -EBUSY;
> +		}
> +	}
> +
> +	/* Changing the flag reopens the channels with a different DMA device,
> +	 * which would break the memory providers installed on the RX queues
> +	 * (devmem or io_uring zcrx) and the dmabufs bound for TX.
> +	 */
> +	for (i = 0; i < netdev->real_num_rx_queues; i++) {
> +		if (netif_rxq_has_unreadable_mp(netdev, i)) {
> +			netdev_err(netdev, "Can't change data_direct while rxq %u has a memory provider\n",
> +				   i);
> +			return -EBUSY;
> +		}
> +	}
> +
> +	if (netdev_has_dmabuf_binding(netdev, NULL)) {
> +		netdev_err(netdev, "Can't change data_direct while dmabufs are bound\n");
> +		return -EBUSY;
> +	}
> +
> +	new_params = priv->channels.params;
> +	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, enable);
> +
> +	return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
>  }

[Severity: Medium]
Can MLX5E_PFLAG_DATA_DIRECT remain set after system resume if the data
direct device was unbound while suspended?

During suspend, mlx5e_nic_disable() calls mlx5e_data_direct_unbind_dev()
and unregisters priv->data_direct_nb without clearing
MLX5E_PFLAG_DATA_DIRECT in priv->channels.params.  If the data direct
PCI device is unbound before mlx5e_nic_enable() re-registers the
notifier on resume, mdev->data_direct->dev is NULL, yet the channels
reopen with MLX5E_PFLAG_DATA_DIRECT still enabled.  Subsequent calls to
mlx5e_queue_get_dma_dev() then return NULL instead of the NIC DMA
device, breaking zero-copy memory provider operations until the flag is
manually disabled.

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