Re: [PATCH net-next 6/6] selftests: drv-net: gro: add a test for bad IPv4 csum

[Willem de Bruijn]

Jakub Kicinski wrote:
> On Wed, 01 Apr 2026 20:28:46 -0400 Willem de Bruijn wrote:
> > >   /* ip sub-tests - IPv4 only */
> > > + } else if (strcmp(testname, "ip_csum") == 0) {
> > > + correct_payload[0] = PAYLOAD_LEN;
> > > + correct_payload[1] = PAYLOAD_LEN;
> > > + printf("bad ip checksum doesn't coalesce: ");
> > > + check_recv_pkts(rxfd, correct_payload, 2);  
> > 
> > This verifies that a packet with bad csum does not coalesce to a valid
> > packet. Perhaps too paranoid but, do you also want to test the reverse
> > case?
> 
> Will do, easy enough. Tho TBH I can't think of a case where this would
> matter. Bad csum pkt must bypass all GRO processing completely right?
> Because we don't want a corrupted packet to flush a valid session?
> Or you think some implementation may actually feed these packets into
> GRO to avoid waiting for a session timeout?

Interesting, I thought the opposite: this would flush an RSC context.
That's also how the OCP text is written: "An SO context closes if a
packet matches the flow, but not the other conditions." 

There's a slight subtlety when sending three packets p1, p2 and p3, of
which p2 has a corrupted checksum. If three consecutive payloads, then
if p2 bypasses GRO, the other two are not consecutive so will not
coalesce either. This is the likely case for a real bit flip in
transit. Only if p2 and p3 have the same seqno would p1 and p3
coalesce if p2 bypasses GRO. That would be a weird, possibly malicious
packet, which is not much different from other TCP injection attacks.
Which have more serious consequences than coalescing efficiency. 

Re: [PATCH net-next 6/6] selftests: drv-net: gro: add a test for bad IPv4 csum

[Jakub Kicinski]

On Wed, 01 Apr 2026 20:28:46 -0400 Willem de Bruijn wrote:
> > /* ip sub-tests - IPv4 only */
> > +   } else if (strcmp(testname, "ip_csum") == 0) {
> > +   correct_payload[0] = PAYLOAD_LEN;
> > +   correct_payload[1] = PAYLOAD_LEN;
> > +   printf("bad ip checksum doesn't coalesce: ");
> > +   check_recv_pkts(rxfd, correct_payload, 2);  
> 
> This verifies that a packet with bad csum does not coalesce to a valid
> packet. Perhaps too paranoid but, do you also want to test the reverse
> case?

Will do, easy enough. Tho TBH I can't think of a case where this would
matter. Bad csum pkt must bypass all GRO processing completely right?
Because we don't want a corrupted packet to flush a valid session?
Or you think some implementation may actually feed these packets into
GRO to avoid waiting for a session timeout?

Re: [PATCH net-next 6/6] selftests: drv-net: gro: add a test for bad IPv4 csum

[Willem de Bruijn]

Jakub Kicinski wrote:
> We have a test for coalescing with bad TCP checksum, let's also
> test bad IPv4 header checksum.
> 
> Signed-off-by: Jakub Kicinski 
> ---
>  tools/testing/selftests/net/lib/gro.c  | 25 ++
>  tools/testing/selftests/drivers/net/gro.py |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/lib/gro.c 
> b/tools/testing/selftests/net/lib/gro.c
> index 762e88932ed2..a458cfbd11eb 100644
> --- a/tools/testing/selftests/net/lib/gro.c
> +++ b/tools/testing/selftests/net/lib/gro.c
> @@ -36,6 +36,7 @@
>   *  Packets with different (ECN, TTL, TOS) header, IP options or
>   *  IP fragments shouldn't coalesce.
>   *   - ip_ecn, ip_tos:shared between IPv4/IPv6
> + *   - ip_csum:   IPv4 only, bad IP header checksum
>   *   - ip_ttl, ip_opt, ip_frag4:  IPv4 only
>   *   - ip_id_df*: IPv4 IP ID field coalescing tests
>   *   - ip_frag6, ip_v6ext_*:  IPv6 only
> @@ -675,6 +676,21 @@ static void send_changed_checksum(int fd, struct 
> sockaddr_ll *daddr)
>   write_packet(fd, buf, pkt_size, daddr);
>  }
>  
> +/* Packets with incorrect IPv4 header checksum don't coalesce. */
> +static void send_changed_ip_checksum(int fd, struct sockaddr_ll *daddr)
> +{
> + static char buf[MAX_HDR_LEN + PAYLOAD_LEN];
> + struct iphdr *iph = (struct iphdr *)(buf + ETH_HLEN);
> + int pkt_size = total_hdr_len + PAYLOAD_LEN;
> +
> + create_packet(buf, 0, 0, PAYLOAD_LEN, 0);
> + write_packet(fd, buf, pkt_size, daddr);
> +
> + create_packet(buf, PAYLOAD_LEN, 0, PAYLOAD_LEN, 0);
> + iph->check = iph->check - 1;
> + write_packet(fd, buf, pkt_size, daddr);
> +}
> +
>   /* Packets with non-consecutive sequence number don't coalesce.*/
>  static void send_changed_seq(int fd, struct sockaddr_ll *daddr)
>  {
> @@ -1392,6 +1408,10 @@ static void gro_sender(void)
>   write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
>  
>   /* ip sub-tests - IPv4 only */
> + } else if (strcmp(testname, "ip_csum") == 0) {
> + send_changed_ip_checksum(txfd, &daddr);
> + usleep(fin_delay_us);
> + write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
>   } else if (strcmp(testname, "ip_ttl") == 0) {
>   send_changed_ttl(txfd, &daddr);
>   write_packet(txfd, fin_pkt, total_hdr_len, &daddr);
> @@ -1588,6 +1608,11 @@ static void gro_receiver(void)
>   check_recv_pkts(rxfd, correct_payload, 2);
>  
>   /* ip sub-tests - IPv4 only */
> + } else if (strcmp(testname, "ip_csum") == 0) {
> + correct_payload[0] = PAYLOAD_LEN;
> + correct_payload[1] = PAYLOAD_LEN;
> + printf("bad ip checksum doesn't coalesce: ");
> + check_recv_pkts(rxfd, correct_payload, 2);

This verifies that a packet with bad csum does not coalesce to a valid
packet. Perhaps too paranoid but, do you also want to test the reverse
case?

Can be a single test by adding another write_packet and observing all
three separate packets.