|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