Software GRO fails to coalesce a small IPv4 segment that was padded up to the 60-byte minimum Ethernet frame. The selftest tools/testing/selftests/drivers/net/gro.py subtest sw_ipv4_data_lrg_1byte sends {100, 1} expecting to receive {101}. In current code, it receives {100, 1} (no coalescing) instead. Cause: inet_gro_receive() computes its flush term from tot_len ^ skb_gro_len() before skb_gro_pull(), while skb_gro_len() still includes trailing Ethernet padding. A small IPv4 segment padded up to the 60-byte minimum frame has tot_len != skb_gro_len(), so flush is set and the runt never coalesces. v1 detected the padding with an added iph->tot_len read and skb_gro_len() comparison on every IPv4 GRO packet. Instead, split inet_gro_receive() so that everything after the header validation takes the flush term as a parameter, and pass a literal 0 on the common path. That folds away both flush updates and lets the transport dispatch become a tail call, leaving the common path shorter than before this patch rather than merely unchanged. Assisted-by: Claude:claude-opus-5 Assisted-by: Codex:gpt-5.6 Assisted-by: Meta:internal-AI-tooling Signed-off-by: Glenn Judd --- v1: https://lore.kernel.org/netdev/20260731185431.2777685-1-gmj@meta.com/ v2: - reworked so the fix costs nothing on the common path: the flush term is computed before the branch and passed to a split-out inet_gro_receive_finish() as a literal 0, rather than adding an iph->tot_len read and an skb_gro_len() comparison to every packet - the common path is now shorter than before the patch - retitled: the change is not TCP specific, it covers all IPv4 GRO - do not trim inner encapsulated headers, or under NETIF_F_RXFCS - spell the padding bound ETH_ZLEN - ETH_HLEN + ETH_FCS_LEN (unchanged at 50); tag depth cancels, the ETH_FCS_LEN is deliberate slack - use mem_is_zero() to verify the pad rather than an open-coded scan Overview After a bit more work, I think that it's also worth considering another approach. By decomposing the logic in inet_gro_receive() into helpers, we can allow the compiler to remove code -- making the common path shorter/faster than the current code. The key motivation for this improvement is that coalescing padded runts can then be handled "for free" (no overhead on the common path). (As with the original RFC, the behavior that this addresses occurs under software gro for IPv4 with no timestamps.) The table below shows the non-blank, non-comment code, instruction count, and memory access differences relative to base unmodified code. (a is the original RFC. b is the new approach.) Instruction and memory access counts are for the common path only (function entry to the transport dispatch), measured with clang 22.1.3 x86_64. +--------+----------+----------+----------+ | | LoC | insns | mem | +--------+----------+----------+----------+ | base | - | - | - | | a | +17 | +14 | +3 | | b | +33 | -9 | -2 | +--------+----------+----------+----------+ Note that the majority of the code change in b is decomposition of existing code. The new logic comprises 10 lines. Where the padding cannot be safely stripped (a tunnel, a retained FCS, a nonlinear skb, or a pad that is not zero), b does not trim, and the segment is simply not coalesced, exactly as base behaves today. Testing I have run a number of performance tests to analyze the bulk throughput performance of each approach, as well as the performance impact of coalescing the runt packet vs. not coalescing. In all cases I pinned both irq handling and the test application to 1 CPU. I turned off frequency scaling and restricted power states. Throughput below is CPU-bound rather than link-bound. Bulk Throughput I first measured bulk throughput for 8 independent TCP flows between a single sender and receiver. As shown below, the performance in this test matches the code analysis above. b's reduced common path results in better throughput. +---------+-----------+-----------+-----------+ | | Gbps | delta % | 95% CI | +---------+-----------+-----------+-----------+ | base | 12.222 | - | - | | a | 12.173 | -0.404 | +-0.162 | | b | 12.289 | +0.543 | +-0.147 | +---------+-----------+-----------+-----------+ Padding Fix Performance Impact I then measured the performance impact of coalescing runts vs. not coalescing. I did this by sending 8 concurrent flows where each flow sent a pair of packets sized [1460], [l (length)]. The second packet shows up as a "runt", and triggers the packet coalescing behavior for lengths >= 6 for all cases, and triggers the padding removal/coalescing for lengths < 6 for approaches a and b. The results below show the average time to process a single packet pair for the given scenarios. As stated above, packets with l=6 coalesce for all implementations (no padding is present). l=5 requires new logic to coalesce the padded runt. The last two columns show the average number of packets coalesced for each test (a sanity check to indicate when coalescing is happening [~2] vs. not happening [~1]). Computing l5-l6, we can measure the penalty (if any) of not coalescing packets for the base case, or the cost of handling the padded packet for cases a and b. This test shows a ~0.76 us penalty per failed coalesce for the base case, and ~0 us cost for coalescing with approach b. +-------+----------+----------+----------+-----------+---------+---------+ | | l=5 us | l=6 us | l5-l6 us | 95% CI | gro l5 | gro l6 | +-------+----------+----------+----------+-----------+---------+---------+ | base | 14.311 | 13.547 | +0.764 | +-0.130 | 0.993 | 1.974 | | a | 13.470 | 13.359 | +0.111 | +-0.056 | 1.965 | 1.974 | | b | 13.361 | 13.351 | +0.010 | +-0.040 | 1.964 | 1.974 | +-------+----------+----------+----------+-----------+---------+---------+ Summary In short, improving the sw gro common path (9 fewer instructions, 0.54% more throughput) allows us to fix the failure to coalesce padded runts with zero additional overhead. The penalty for not coalescing a padded runt is ~0.76 us. Approach b recovers all of that. net/ipv4/af_inet.c | 136 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 103 insertions(+), 33 deletions(-) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 32d006c1a8ee..6ac2089385dc 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1465,40 +1465,26 @@ static struct sk_buff *ipip_gso_segment(struct sk_buff *skb, return inet_gso_segment(skb, features); } -struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb) +/* Non-zero means tot_len != gro_len OR IP_CE is set: ip_is_fragment() tests + * only IP_MF and IP_OFFSET, and IP_DF is masked here, but IP_CE is not. + * Recompute after trimming; never assume a trimmed packet has a zero term. + */ +static int inet_gro_flush_term(const struct iphdr *iph, + unsigned int gro_len) { - const struct net_offload *ops; - struct sk_buff *pp = NULL; - const struct iphdr *iph; - struct sk_buff *p; - unsigned int hlen; - unsigned int off; - int flush = 1; - int proto; - - off = skb_gro_offset(skb); - hlen = off + sizeof(*iph); - iph = skb_gro_header(skb, hlen, off); - if (unlikely(!iph)) - goto out; - - proto = iph->protocol; - - ops = rcu_dereference(inet_offloads[proto]); - if (!ops || !ops->callbacks.gro_receive) - goto out; - - if (*(u8 *)iph != 0x45) - goto out; - - if (ip_is_fragment(iph)) - goto out; - - if (unlikely(ip_fast_csum((u8 *)iph, 5))) - goto out; + return (u16)((ntohl(*(__be32 *)iph) ^ gro_len) | + (ntohl(*(__be32 *)&iph->id) & ~IP_DF)); +} - NAPI_GRO_CB(skb)->proto = proto; - flush = (u16)((ntohl(*(__be32 *)iph) ^ skb_gro_len(skb)) | (ntohl(*(__be32 *)&iph->id) & ~IP_DF)); +/* The common caller passes a literal 0 for flush, which (with + * __always_inline) folds away both updates where it is used below. + */ +static __always_inline struct sk_buff * +inet_gro_receive_finish(struct list_head *head, struct sk_buff *skb, + const struct net_offload *ops, const struct iphdr *iph, + unsigned int off, int flush) +{ + struct sk_buff *pp, *p; list_for_each_entry(p, head, list) { struct iphdr *iph2; @@ -1532,11 +1518,95 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb) pp = indirect_call_gro_receive(tcp4_gro_receive, udp4_gro_receive, ops->callbacks.gro_receive, head, skb); -out: skb_gro_flush_final(skb, pp, flush); return pp; } + +/* Superset gate: a VLAN tag lengthens both the padded frame and the L2 header + * so tag depth cancels; gro_len > tot_len and the all-zero scan decide. + */ +static noinline struct sk_buff * +inet_gro_receive_slow(struct list_head *head, struct sk_buff *skb, + const struct net_offload *ops, const struct iphdr *iph, + unsigned int off) +{ + unsigned int tot_len = ntohs(iph->tot_len); + unsigned int gro_len = skb->len - off; + + if (NAPI_GRO_CB(skb)->encap_mark || + (skb->dev->features & NETIF_F_RXFCS) || + gro_len > ETH_ZLEN - ETH_HLEN + ETH_FCS_LEN || gro_len <= tot_len || + tot_len < sizeof(*iph)) + goto no_trim; + + /* A linear skb is contiguous through skb->len, so the scan below ends + * at skb->data + skb->len. Keep this test ahead of it. + */ + if (skb_is_nonlinear(skb)) + goto no_trim; + + if (!mem_is_zero(skb->data + off + tot_len, gro_len - tot_len)) + goto no_trim; + + /* Trailing zeros leave a one's-complement sum unchanged, so the + * NAPI_GRO_CB(skb)->csum cached before this call stays valid; + * __skb_trim() cannot reallocate, so iph stays valid. + */ + __skb_trim(skb, off + tot_len); + NAPI_GRO_CB(skb)->frag0_len = skb->len; + gro_len = tot_len; + +no_trim: + return inet_gro_receive_finish(head, skb, ops, iph, off, + inet_gro_flush_term(iph, gro_len)); +} + +struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb) +{ + const struct net_offload *ops; + const struct iphdr *iph; + unsigned int gro_len; + unsigned int off; + int proto; + + off = skb_gro_offset(skb); + iph = skb_gro_header(skb, off + sizeof(*iph), off); + if (unlikely(!iph)) + goto out; + + proto = iph->protocol; + + ops = rcu_dereference(inet_offloads[proto]); + if (!ops || !ops->callbacks.gro_receive) + goto out; + + if (*(u8 *)iph != 0x45) + goto out; + + if (ip_is_fragment(iph)) + goto out; + + if (unlikely(ip_fast_csum((u8 *)iph, 5))) + goto out; + + NAPI_GRO_CB(skb)->proto = proto; + + /* skb_gro_len(skb) without re-reading data_offset; the skb_gro_pull() + * in finish() must stay below this. + */ + gro_len = skb->len - off; + + if (unlikely(inet_gro_flush_term(iph, gro_len))) + return inet_gro_receive_slow(head, skb, ops, iph, off); + + return inet_gro_receive_finish(head, skb, ops, iph, off, 0); + +out: + skb_gro_flush_final(skb, NULL, 1); + + return NULL; +} EXPORT_INDIRECT_CALLABLE(inet_gro_receive); static struct sk_buff *ipip_gro_receive(struct list_head *head, base-commit: 2fbade66245059c78daeaccfce13ecf499fffb51 -- 2.53.0-Meta