Re: [PATCH net] net/mlx5e: SHAMPO, Fix IP length overflow on large HW GRO sessions
From: Dragos Tatulea <dtatulea@nvidia.com>
Date: 2026-07-30 17:57:37
Also in:
linux-rdma, lkml
Hi Tianyu, Thanks for your patch. It is a good find. On 29.07.26 22:47, Tianyu Zuo wrote:
mlx5e_hw_gro_skb_has_enough_space() bounds a HW GRO session by the payload held in the skb fragments only. The L3/L4 headers that header-data split placed in the linear area are not accounted for, and the limit is inclusive of GRO_LEGACY_MAX_SIZE. On a 4K page system a session can therefore grow to 16 full page fragments (65536 bytes) plus the 40 bytes of IPv4/TCP headers in the linear part, giving skb->len = 65576. mlx5e_shampo_update_hdr() writes the IP length itself: __be16 newlen = htons(skb->len - nhoff); csum_replace2(&ipv4->check, ipv4->tot_len, newlen); ipv4->tot_len = newlen;
Commit message is clear until here.
With nhoff == 0 this stores tot_len = 40 and updates the header checksum to match, so the corruption is self-consistent. The GRO stack does not repair it: the header is written before napi_gro_receive(), and inet_gro_complete() only runs for skbs that the GRO engine holds on its gro_list. HW GRO sessions are typically flushed on TCP_FLAG_PSH, which makes tcp_gro_receive() set NAPI_GRO_CB(skb)->flush, so dev_gro_receive() hands the skb over via GRO_NORMAL and the gro_complete() callbacks are never invoked. The length check in inet_gro_receive() cannot catch it either, since tot_len and skb_gro_len() are compared modulo 64K. ip_rcv_core() then trims the 64KB skb down to the wrapped tot_len, silently dropping the payload. The IPv6 path wraps identically in ipv6hdr->payload_len. Triggering this requires the payload of the aggregated session to reach GRO_LEGACY_MAX_SIZE with every fragment fully populated, since page_size * nr_frags otherwise overestimates the data actually present and the session is flushed earlier. In practice this needs an MSS that is a multiple of the page size (for example 8192 on a 4K page host with jumbo frames) together with a page aligned start of the session.
You can drop this bit.
Account for skb_headlen() and make both checks strictly less than GRO_LEGACY_MAX_SIZE so that skb->len can never exceed 65535.
This is useful.
The check is strictly more conservative than before, so the implicit bound on the fragment count is preserved: page_size * nr_frags + data_bcnt <= 65535 gives nr_frags + data_bcnt / page_size <= 65536 / page_size - 1, and a single CQE adds at most data_bcnt / page_size + 1 fragments, for a total of at most 65536 / page_size (16 on 4K pages), well below MAX_SKB_FRAGS.
You can drop this last paragraph. Or summarize it in the previous one.
quoted hunk ↗ jump to hunk
Fixes: 92552d3abd32 ("net/mlx5e: HW_GRO cqe handler implementation") Signed-off-by: Tianyu Zuo <redacted> --- drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c index 6fbc0441c4b8..2e9676305439 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c@@ -2222,9 +2222,10 @@ static bool mlx5e_hw_gro_skb_has_enough_space(struct sk_buff *skb, int nr_frags = skb_shinfo(skb)->nr_frags; if (page_size >= GRO_LEGACY_MAX_SIZE) - return skb->len + data_bcnt <= GRO_LEGACY_MAX_SIZE; + return skb->len + data_bcnt < GRO_LEGACY_MAX_SIZE; else - return page_size * nr_frags + data_bcnt <= GRO_LEGACY_MAX_SIZE; + return skb_headlen(skb) + page_size * nr_frags + data_bcnt < + GRO_LEGACY_MAX_SIZE; } static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)base-commit: 51b093a7ba27476e1f639455f005e8d2e75390e4
The code looks good to me. Wait for Tariq's comments before re-spinning. Thanks, Dragos