* thunderbolt_net: GRO header-length ordering defect on Proxmox 7.0.14
@ 2026-11-08 19:48 brianrobertjames
2026-11-09 12:17 ` [PATCH net] thunderbolt: Use needed_headroom for the transport header brianrobertjames
0 siblings, 1 reply; 3+ messages in thread
From: brianrobertjames @ 2026-11-08 19:48 UTC (permalink / raw)
To: westeri, YehezkelShB; +Cc: netdev, linux-usb, linux-kernel
Hello Mika and Yehezkel,
I would appreciate your assessment of a thunderbolt_net GRO ordering defect
observed on a direct USB4 link between two AMD Proxmox hosts. We have
receiver-only original/corrected/original comparisons and packet-order
traces supporting the Ethernet-header/headroom correction already published
by Shun Li.
Credit for that prior published correction belongs to Shun Li:
https://github.com/riverscn/thunderbolt-net-dkms/commit/b18a196bb5ca54246443
e1a29924d6007aa16036
We independently investigated the same defect and applied the minimal change
to pinned official Proxmox source. We did not install the project's broader
DKMS/RX-normalization modifications.
Tested environment:
- Two AMD x86-64 hosts with one direct USB4 cable; native thunderbolt0
traffic, no bridge, VM or Ceph in the path.
- Receiver kernel 7.0.14-28-pve, #2 SMP PREEMPT_DYNAMIC PMX 7.0.14-28
(2026-11-02T11:54Z). Also reproduced historically on 7.0.2-6-pve.
- MTU 9000; TSO/GSO/GRO enabled; gso_max_size=65536; unpaced single-stream
IPv4 TCP iperf3.
- Stock thunderbolt controller driver unchanged. Receiver comparisons
changed only the network module's header/headroom metadata. Cable, ports,
offloads and CPU/IRQ placement stayed constant within each comparison.
Secure Boot remained enabled with signed test modules.
Receiver-only comparison, original -> corrected -> original, 20 seconds per
arm:
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 historical receiver-isolation
test was still on 7.0.2-6-pve with TX E2E enabled; receiver RX flags stayed
FRAME|E2E (6). Both hosts now remain on 7.0.14-28-pve. This is not a
two-latest-kernel GRO-only comparison.
Minimal reproduction outline: establish direct USB4 IP connectivity with the
above settings, run an iperf3 server bound to the receiver's USB4 address,
and run one 20-second TCP stream from the peer:
receiver: iperf3 -s -B <receiver-USB4-IP>
sender: iperf3 -c <receiver-USB4-IP> -B <sender-USB4-IP> -P 1 -t 20 -J
Repeat after changing only the receiver's header/headroom line, then restore
the original receiver module and repeat. Module reload interrupts the data
link; our management connection was separate.
Mechanism and minimal correction in tbnet_probe():
- dev->hard_header_len += sizeof(struct thunderbolt_ip_frame_header);
+ dev->needed_headroom = sizeof(struct thunderbolt_ip_frame_header);
The 12-byte transport header is not part of the Ethernet MAC header
presented to eth_type_trans()/GRO. The original hard_header_len is 26 rather
than ETH_HLEN (14), so gro_list_prepare() can compare 12 IP-header bytes as
link-layer bytes and fail to recognize packets belonging to the same flow.
Runtime tracing confirmed original hard_header_len/needed_headroom=26/0
versus corrected 14/12.
Historical paired receive hooks on 7.0.2 covered the same 18,871,120,863 TCP
payload bytes and 303,590 packets 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 and zero forward gaps. Retransmitted/overlapping sequence ranges
were treated separately. This localizes the observed ordering defect to the
receive software path; it is not proof of physical USB4 packet loss.
Full reproduction details, source provenance and the one-line patch:
https://bugzilla.proxmox.com/show_bug.cgi?id=8306
Independent hardware validation shared with the prior fix's author:
https://github.com/riverscn/thunderbolt-net-dkms/issues/27
Separate context: with both hosts on 7.0.14-28-pve and GRO already
corrected, we also observed TX-credit-dependent fragment-validation errors.
A local negotiated TX E2E opt-in gave 18,604 -> 1 -> 18,601 TCP
retransmissions in off/on/off 20-second arms. This is a separate diagnostic
change, excluded from the GRO patch. Its lower-level cause is unresolved; we
are not proposing globally re-enabling TX E2E given the known controller
compatibility concerns. The related discussion and limitations are here:
https://github.com/hellas-ai/thunderbolt-ibverbs/pull/78#issuecomment-672616
9650
We have not reproduced these observations on the latest vanilla mainline
kernel, and upstream acceptance of Shun Li's GRO correction has not been
verified. Could you assess whether the GRO correction is appropriate for
upstream/backporting, or point us to an existing fix or discussion? Raw
benchmark and trace evidence is retained locally.
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).
Measurements came from actual hosts and are not simulated results.
Thank you,
Brian James
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH net] thunderbolt: Use needed_headroom for the transport header
2026-11-08 19:48 thunderbolt_net: GRO header-length ordering defect on Proxmox 7.0.14 brianrobertjames
@ 2026-11-09 12:17 ` brianrobertjames
2026-11-09 14:15 ` Mika Westerberg
0 siblings, 1 reply; 3+ messages in thread
From: brianrobertjames @ 2026-11-09 12:17 UTC (permalink / raw)
To: netdev; +Cc: westeri, YehezkelShB, riverscn
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);
netif_napi_add(dev, &net->napi, tbnet_poll);
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] thunderbolt: Use needed_headroom for the transport header
2026-11-09 12:17 ` [PATCH net] thunderbolt: Use needed_headroom for the transport header brianrobertjames
@ 2026-11-09 14:15 ` Mika Westerberg
0 siblings, 0 replies; 3+ messages in thread
From: Mika Westerberg @ 2026-11-09 14:15 UTC (permalink / raw)
To: brianrobertjames; +Cc: netdev, westeri, YehezkelShB, riverscn
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-09 14:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-08 19:48 thunderbolt_net: GRO header-length ordering defect on Proxmox 7.0.14 brianrobertjames
2026-11-09 12:17 ` [PATCH net] thunderbolt: Use needed_headroom for the transport header brianrobertjames
2026-11-09 14:15 ` Mika Westerberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox