|SUCCESS| net/mlx5: query hardware capability for max lro size

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Apr 20 22:43:12 CEST 2026


Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163219

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-20

Reviewing patch: net/mlx5: query hardware capability for max lro size

Commit: (from patch series.patch)
Author: Rayane Boussanni <rboussanni at gmail.com>
Subject: [PATCH] net/mlx5: query hardware capability for max lro size

This patch resolves a FIXME by dynamically checking the hardware LRO
capability flag before advertising MLX5_MAX_LRO_SIZE in device info.

> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index a29cdeeb50..fb51de031b 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -363,9 +363,9 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
>  	info->min_mtu = priv->min_mtu;
>  	info->max_mtu = priv->max_mtu;
>  	info->max_rx_pktlen = info->max_mtu + MLX5_ETH_OVERHEAD;
> -	/* FIXME: we should ask the device for these values. */
>  	info->min_rx_bufsize = 32;
> -	info->max_lro_pkt_size = MLX5_MAX_LRO_SIZE;
> +	info->max_lro_pkt_size = priv->sh->cdev->config.hca_attr.lro_cap ?
> +				 MLX5_MAX_LRO_SIZE : 0;

Does this code handle a null priv->sh or priv->sh->cdev pointer?
Looking at mlx5_dev_infos_get(), if the device is not yet configured,
could these pointers be null? The function should likely verify these
pointers exist before dereferencing through this chain:
priv->sh->cdev->config.hca_attr.lro_cap.

Should the ternary operator expression be wrapped in parentheses for
clarity, given the assignment context? While the precedence is correct,
explicit grouping improves readability:
    info->max_lro_pkt_size = (priv->sh->cdev->config.hca_attr.lro_cap ?
                              MLX5_MAX_LRO_SIZE : 0);

Is the lro_cap field itself a boolean, or does it contain a capability
mask? If it's a mask, should the code test for a specific bit rather
than treating the entire field as a boolean?


More information about the test-report mailing list