Re: [PATCH net] thunderbolt: Use needed_headroom for the transport header

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Mon, Nov 09, 2026 at 07:17:04AM -0500, brianrobertjames@gmail.com wrote:
> alloc_etherdev() initializes hard_header_len to ETH_HLEN. tbnet_probe()
> then adds the 12-byte Thunderbolt transport header, although that header
> is not part of the Ethernet MAC header handed to eth_type_trans()/GRO.
> 
> This makes gro_list_prepare() include 12 bytes of the IP header in its
> nonstandard link-layer comparison. Changing IP lengths, IDs or checksums
> can prevent recognition of packets belonging to the same TCP flow,
> allowing later packets to bypass earlier data held by GRO. On the tested
> direct USB4 link this causes TCP reordering and excess retransmissions.
> 
> Reserve the transport header in needed_headroom instead. Keep the
> Ethernet hard_header_len at 14, with 12 bytes of needed_headroom. This
> preserves the transport space requirement without changing wire format,
> offload features, or RX/TX credit allocation.
> 
> Shun Li published the identical correction on October 9, 2026, in
> riverscn/thunderbolt-net-dkms (commit b18a196bb5ca). Credit for that prior
> published correction belongs to Shun Li. This submission adds our
> independent AMD/Proxmox hardware validation and receive-order evidence.
> 
> Link: https://github.com/riverscn/thunderbolt-net-dkms/commit/b18a196bb5ca
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=8306
> Signed-off-by: Brian James <brianrobertjames@gmail.com>
> ---
> Follow-up to my November 8 report in this thread. Fabian Grunbichler at
> Proxmox requested that the actual patch be submitted upstream. The
> one-line diff is included inline below for review.
> 
> Test environment:
> - Two AMD x86-64 hosts, direct native USB4/thunderbolt0 traffic.
> - Receiver: Proxmox 7.0.14-28-pve; historical tests also used 7.0.2-6-pve.
> - MTU 9000, TSO/GSO/GRO enabled, gso_max_size 65536.
> - Unpaced single-stream IPv4 TCP iperf3, 20 seconds per arm.
> - Stock thunderbolt controller unchanged; only the receiver's network
>   driver's header/headroom line changed. Cable, ports, MTU, offloads and
>   CPU/IRQ placement were held constant within each comparison.
> 
> Receiver-only original -> corrected -> original comparison:
>   Original:           30.599221 Gb/s, 620 TCP retransmissions
>   GRO corrected:      29.779541 Gb/s,   0 TCP retransmissions
>   Original restored:  29.994158 Gb/s, 540 TCP retransmissions
> 
> Endpoint interface error/drop increments and receiver FQ drops were zero
> in all three arms. The unchanged sender in this isolation test ran
> 7.0.2-6-pve with TX E2E enabled; receiver RX flags stayed FRAME|E2E (6).
> This comparison isolates the receiver fix with that sender configuration.
> It does not establish that GRO alone resolves every retransmission mode
> when both peers run 7.0.14-28-pve.
> 
> Historical 7.0.2 receiver-only comparisons on each host, 20 seconds/arm:
>   Receiver A retransmissions: 536 -> 0 -> 132
>   Receiver B retransmissions: 564 -> 0 -> 769
> 
> Historical paired receive hooks covered identical TCP payload bytes
> (18,871,120,863) and packet counts (303,590) before GRO and at TCP ingress,
> on the same CPU. Forward sequence gaps were 0 before GRO and 1,219 at TCP
> ingress, with no packet-length parsing mismatches. A corrected capture
> had equal byte coverage at both hooks and zero forward gaps. Overlapping
> and retransmitted sequence ranges were handled separately. This supports
> an ordering defect in the receive software path for this failure mode.
> Runtime tracing confirmed header/headroom 26/0 before and 14/12 after.
> 
> The measurements above came from actual Proxmox hosts. The current
> mainline source was inspected but has not been runtime-tested on this
> hardware. The diff context was checked against mainline commit
> 7475ea8b58616b33c04afb976cf0fbaad9dd63fb. The separate TX-credit diagnostic
> is excluded from this patch. No Ceph qualification is claimed.
> 
> AI assistance disclosure: source review, diagnostic automation, analysis
> and preparation of this email were AI-assisted using OpenAI Codex
> (GPT-6 model family; a more specific serving-model identifier was not
> provided). The measurements and traces are not simulated results.
> 
>  drivers/net/thunderbolt/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> --- a/drivers/net/thunderbolt/main.c
> +++ b/drivers/net/thunderbolt/main.c
> @@ -1410,7 +1410,7 @@ static int tbnet_probe(struct tb_service *svc)
>  	dev->hw_features = NETIF_F_SG | NETIF_F_ALL_TSO | NETIF_F_GRO |
>  			   NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
>  	dev->features = dev->hw_features | NETIF_F_HIGHDMA;
> -	dev->hard_header_len += sizeof(struct thunderbolt_ip_frame_header);
> +	dev->needed_headroom = sizeof(struct thunderbolt_ip_frame_header);

I don't think dev->needed_headroom is needed either. The TX side copies the
data directly to the frame pages.