Currently, packets with fixed IDs will be merged only if their
don't-fragment bit is set. This restriction is unnecessary since packets
without the don't-fragment bit will be forwarded as-is even if they were
merged together.

If packets are merged together and then fragmented, they will first be
re-split into segments before being further fragmented, so the behavior
is identical whether or not the packets were first merged together.

Clean up the code by removing the unnecessary don't-fragment checks.

Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
---
 include/net/gro.h                 | 5 ++---
 net/ipv4/af_inet.c                | 3 ---
 tools/testing/selftests/net/gro.c | 9 ++++-----
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/include/net/gro.h b/include/net/gro.h
index 6aa563eec3d0..f14b7e88dbef 100644
--- a/include/net/gro.h
+++ b/include/net/gro.h
@@ -448,17 +448,16 @@ static inline int inet_gro_flush(const struct iphdr *iph, const struct iphdr *ip
 	const u32 id2 = ntohl(*(__be32 *)&iph2->id);
 	const u16 ipid_offset = (id >> 16) - (id2 >> 16);
 	const u16 count = NAPI_GRO_CB(p)->count;
-	const u32 df = id & IP_DF;
 
 	/* All fields must match except length and checksum. */
-	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | (df ^ (id2 & IP_DF)))
+	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | ((id ^ id2) & IP_DF))
 		return true;
 
 	/* When we receive our second frame we can make a decision on if we
 	 * continue this flow as an atomic flow with a fixed ID or if we use
 	 * an incrementing ID.
 	 */
-	if (count == 1 && df && !ipid_offset)
+	if (count == 1 && !ipid_offset)
 		NAPI_GRO_CB(p)->ip_fixedid |= 1 << inner;
 
 	return ipid_offset ^ (count * !(NAPI_GRO_CB(p)->ip_fixedid & (1 << inner)));
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index fc7a6955fa0a..c0542d9187e2 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1393,10 +1393,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 
 	segs = ERR_PTR(-EPROTONOSUPPORT);
 
-	/* fixed ID is invalid if DF bit is not set */
 	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));
-	if (fixedid && !(ip_hdr(skb)->frag_off & htons(IP_DF)))
-		goto out;
 
 	if (!skb->encapsulation || encap)
 		udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
index d5824eadea10..3d4a82a2607c 100644
--- a/tools/testing/selftests/net/gro.c
+++ b/tools/testing/selftests/net/gro.c
@@ -670,7 +670,7 @@ static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
 		iph2->id = htons(9);
 		break;
 
-	case 3: /* DF=0, Fixed - should not coalesce */
+	case 3: /* DF=0, Fixed - should coalesce */
 		iph1->frag_off &= ~htons(IP_DF);
 		iph1->id = htons(8);
 
@@ -1188,10 +1188,9 @@ static void gro_receiver(void)
 			correct_payload[0] = PAYLOAD_LEN * 2;
 			check_recv_pkts(rxfd, correct_payload, 1);
 
-			printf("DF=0, Fixed - should not coalesce: ");
-			correct_payload[0] = PAYLOAD_LEN;
-			correct_payload[1] = PAYLOAD_LEN;
-			check_recv_pkts(rxfd, correct_payload, 2);
+			printf("DF=0, Fixed - should coalesce: ");
+			correct_payload[0] = PAYLOAD_LEN * 2;
+			check_recv_pkts(rxfd, correct_payload, 1);
 
 			printf("DF=1, 2 Incrementing and one fixed - should coalesce only first 2 packets: ");
 			correct_payload[0] = PAYLOAD_LEN * 2;
-- 
2.36.1

Richard Gobert wrote:
> Currently, packets with fixed IDs will be merged only if their
> don't-fragment bit is set. This restriction is unnecessary since packets
> without the don't-fragment bit will be forwarded as-is even if they were
> merged together.

Please expand why this is true.

Because either NETIF_F_TSO_MANGLEID is set or segmentation
falls back onto software GSO which handles the two FIXEDID
variants correctly now, I guess?

> If packets are merged together and then fragmented, they will first be
> re-split into segments before being further fragmented, so the behavior
> is identical whether or not the packets were first merged together.

I don't follow this scenario. Fragmentation of a GSO packet after GRO
and before GSO?

> Clean up the code by removing the unnecessary don't-fragment checks.
> 
> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
> ---
>  include/net/gro.h                 | 5 ++---
>  net/ipv4/af_inet.c                | 3 ---
>  tools/testing/selftests/net/gro.c | 9 ++++-----
>  3 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/gro.h b/include/net/gro.h
> index 6aa563eec3d0..f14b7e88dbef 100644
> --- a/include/net/gro.h
> +++ b/include/net/gro.h
> @@ -448,17 +448,16 @@ static inline int inet_gro_flush(const struct iphdr *iph, const struct iphdr *ip
>  	const u32 id2 = ntohl(*(__be32 *)&iph2->id);
>  	const u16 ipid_offset = (id >> 16) - (id2 >> 16);
>  	const u16 count = NAPI_GRO_CB(p)->count;
> -	const u32 df = id & IP_DF;
>  
>  	/* All fields must match except length and checksum. */
> -	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | (df ^ (id2 & IP_DF)))
> +	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | ((id ^ id2) & IP_DF))
>  		return true;
>  
>  	/* When we receive our second frame we can make a decision on if we
>  	 * continue this flow as an atomic flow with a fixed ID or if we use
>  	 * an incrementing ID.
>  	 */
> -	if (count == 1 && df && !ipid_offset)
> +	if (count == 1 && !ipid_offset)
>  		NAPI_GRO_CB(p)->ip_fixedid |= 1 << inner;
>  
>  	return ipid_offset ^ (count * !(NAPI_GRO_CB(p)->ip_fixedid & (1 << inner)));
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index fc7a6955fa0a..c0542d9187e2 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1393,10 +1393,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>  
>  	segs = ERR_PTR(-EPROTONOSUPPORT);
>  
> -	/* fixed ID is invalid if DF bit is not set */
>  	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));
> -	if (fixedid && !(ip_hdr(skb)->frag_off & htons(IP_DF)))
> -		goto out;
>  
>  	if (!skb->encapsulation || encap)
>  		udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
> index d5824eadea10..3d4a82a2607c 100644
> --- a/tools/testing/selftests/net/gro.c
> +++ b/tools/testing/selftests/net/gro.c
> @@ -670,7 +670,7 @@ static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
>  		iph2->id = htons(9);
>  		break;
>  
> -	case 3: /* DF=0, Fixed - should not coalesce */
> +	case 3: /* DF=0, Fixed - should coalesce */
>  		iph1->frag_off &= ~htons(IP_DF);
>  		iph1->id = htons(8);
>  
> @@ -1188,10 +1188,9 @@ static void gro_receiver(void)
>  			correct_payload[0] = PAYLOAD_LEN * 2;
>  			check_recv_pkts(rxfd, correct_payload, 1);
>  
> -			printf("DF=0, Fixed - should not coalesce: ");
> -			correct_payload[0] = PAYLOAD_LEN;
> -			correct_payload[1] = PAYLOAD_LEN;
> -			check_recv_pkts(rxfd, correct_payload, 2);
> +			printf("DF=0, Fixed - should coalesce: ");
> +			correct_payload[0] = PAYLOAD_LEN * 2;
> +			check_recv_pkts(rxfd, correct_payload, 1);
>  
>  			printf("DF=1, 2 Incrementing and one fixed - should coalesce only first 2 packets: ");
>  			correct_payload[0] = PAYLOAD_LEN * 2;
> -- 
> 2.36.1
>

Willem de Bruijn wrote:
> Richard Gobert wrote:
>> Currently, packets with fixed IDs will be merged only if their
>> don't-fragment bit is set. This restriction is unnecessary since packets
>> without the don't-fragment bit will be forwarded as-is even if they were
>> merged together.
> 
> Please expand why this is true.
> 
> Because either NETIF_F_TSO_MANGLEID is set or segmentation
> falls back onto software GSO which handles the two FIXEDID
> variants correctly now, I guess?
> 

This is true because the merged packets will be segmented back to
their original forms before being forwarded. As you already said, the IDs
will either stay identical or potentially become incrementing if MANGLEID
is set, either of which is fine.

>> If packets are merged together and then fragmented, they will first be
>> re-split into segments before being further fragmented, so the behavior
>> is identical whether or not the packets were first merged together.
> 
> I don't follow this scenario. Fragmentation of a GSO packet after GRO
> and before GSO?
> 

Yes. One could worry that merging packets with the same ID but without DF
would cause issues if they are then fragmented by the host. What I'm saying
is that if such packets are merged and then fragmented, they will first be
segmented back to their original forms by GSO before being further fragmented
(see ip_finish_output_gso). The fragmentation occurs as if the packets were
never merged to begin with. IOW, fragmentation occurs the same way regardless
of whether the packets were merged (GRO + GSO is transparent). I thought I'd
mention this to clarify why this patch doesn't cause any issues.

>> Clean up the code by removing the unnecessary don't-fragment checks.
>>
>> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
>> ---
>>  include/net/gro.h                 | 5 ++---
>>  net/ipv4/af_inet.c                | 3 ---
>>  tools/testing/selftests/net/gro.c | 9 ++++-----
>>  3 files changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/net/gro.h b/include/net/gro.h
>> index 6aa563eec3d0..f14b7e88dbef 100644
>> --- a/include/net/gro.h
>> +++ b/include/net/gro.h
>> @@ -448,17 +448,16 @@ static inline int inet_gro_flush(const struct iphdr *iph, const struct iphdr *ip
>>  	const u32 id2 = ntohl(*(__be32 *)&iph2->id);
>>  	const u16 ipid_offset = (id >> 16) - (id2 >> 16);
>>  	const u16 count = NAPI_GRO_CB(p)->count;
>> -	const u32 df = id & IP_DF;
>>  
>>  	/* All fields must match except length and checksum. */
>> -	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | (df ^ (id2 & IP_DF)))
>> +	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | ((id ^ id2) & IP_DF))
>>  		return true;
>>  
>>  	/* When we receive our second frame we can make a decision on if we
>>  	 * continue this flow as an atomic flow with a fixed ID or if we use
>>  	 * an incrementing ID.
>>  	 */
>> -	if (count == 1 && df && !ipid_offset)
>> +	if (count == 1 && !ipid_offset)
>>  		NAPI_GRO_CB(p)->ip_fixedid |= 1 << inner;
>>  
>>  	return ipid_offset ^ (count * !(NAPI_GRO_CB(p)->ip_fixedid & (1 << inner)));
>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>> index fc7a6955fa0a..c0542d9187e2 100644
>> --- a/net/ipv4/af_inet.c
>> +++ b/net/ipv4/af_inet.c
>> @@ -1393,10 +1393,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>>  
>>  	segs = ERR_PTR(-EPROTONOSUPPORT);
>>  
>> -	/* fixed ID is invalid if DF bit is not set */
>>  	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));
>> -	if (fixedid && !(ip_hdr(skb)->frag_off & htons(IP_DF)))
>> -		goto out;
>>  
>>  	if (!skb->encapsulation || encap)
>>  		udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
>> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
>> index d5824eadea10..3d4a82a2607c 100644
>> --- a/tools/testing/selftests/net/gro.c
>> +++ b/tools/testing/selftests/net/gro.c
>> @@ -670,7 +670,7 @@ static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
>>  		iph2->id = htons(9);
>>  		break;
>>  
>> -	case 3: /* DF=0, Fixed - should not coalesce */
>> +	case 3: /* DF=0, Fixed - should coalesce */
>>  		iph1->frag_off &= ~htons(IP_DF);
>>  		iph1->id = htons(8);
>>  
>> @@ -1188,10 +1188,9 @@ static void gro_receiver(void)
>>  			correct_payload[0] = PAYLOAD_LEN * 2;
>>  			check_recv_pkts(rxfd, correct_payload, 1);
>>  
>> -			printf("DF=0, Fixed - should not coalesce: ");
>> -			correct_payload[0] = PAYLOAD_LEN;
>> -			correct_payload[1] = PAYLOAD_LEN;
>> -			check_recv_pkts(rxfd, correct_payload, 2);
>> +			printf("DF=0, Fixed - should coalesce: ");
>> +			correct_payload[0] = PAYLOAD_LEN * 2;
>> +			check_recv_pkts(rxfd, correct_payload, 1);
>>  
>>  			printf("DF=1, 2 Incrementing and one fixed - should coalesce only first 2 packets: ");
>>  			correct_payload[0] = PAYLOAD_LEN * 2;
>> -- 
>> 2.36.1
>>
> 
>

Richard Gobert wrote:
> Willem de Bruijn wrote:
> > Richard Gobert wrote:
> >> Currently, packets with fixed IDs will be merged only if their
> >> don't-fragment bit is set. This restriction is unnecessary since packets
> >> without the don't-fragment bit will be forwarded as-is even if they were
> >> merged together.
> > 
> > Please expand why this is true.
> > 
> > Because either NETIF_F_TSO_MANGLEID is set or segmentation
> > falls back onto software GSO which handles the two FIXEDID
> > variants correctly now, I guess?
> > 
> 
> This is true because the merged packets will be segmented back to
> their original forms before being forwarded. As you already said, the IDs
> will either stay identical or potentially become incrementing if MANGLEID
> is set, either of which is fine.
> 
> >> If packets are merged together and then fragmented, they will first be
> >> re-split into segments before being further fragmented, so the behavior
> >> is identical whether or not the packets were first merged together.
> > 
> > I don't follow this scenario. Fragmentation of a GSO packet after GRO
> > and before GSO?
> > 
> 
> Yes. One could worry that merging packets with the same ID but without DF
> would cause issues if they are then fragmented by the host. What I'm saying
> is that if such packets are merged and then fragmented, they will first be
> segmented back to their original forms by GSO before being further fragmented
> (see ip_finish_output_gso). The fragmentation occurs as if the packets were
> never merged to begin with.

This explicit pointer that fragmentation for such GSO packets happens
in ip_finish_output_gso, which first calls skb_gso_segment, is
informative. It again turns an assertion into an explanation.

I think you jumped the gun a bit on sending a v6 right with these
answers. I'd like these clarifications recorded.

> IOW, fragmentation occurs the same way regardless
> of whether the packets were merged (GRO + GSO is transparent). I thought I'd
> mention this to clarify why this patch doesn't cause any issues.
> 
> >> Clean up the code by removing the unnecessary don't-fragment checks.
> >>
> >> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
> >> ---
> >>  include/net/gro.h                 | 5 ++---
> >>  net/ipv4/af_inet.c                | 3 ---
> >>  tools/testing/selftests/net/gro.c | 9 ++++-----
> >>  3 files changed, 6 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/include/net/gro.h b/include/net/gro.h
> >> index 6aa563eec3d0..f14b7e88dbef 100644
> >> --- a/include/net/gro.h
> >> +++ b/include/net/gro.h
> >> @@ -448,17 +448,16 @@ static inline int inet_gro_flush(const struct iphdr *iph, const struct iphdr *ip
> >>  	const u32 id2 = ntohl(*(__be32 *)&iph2->id);
> >>  	const u16 ipid_offset = (id >> 16) - (id2 >> 16);
> >>  	const u16 count = NAPI_GRO_CB(p)->count;
> >> -	const u32 df = id & IP_DF;
> >>  
> >>  	/* All fields must match except length and checksum. */
> >> -	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | (df ^ (id2 & IP_DF)))
> >> +	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | ((id ^ id2) & IP_DF))
> >>  		return true;
> >>  
> >>  	/* When we receive our second frame we can make a decision on if we
> >>  	 * continue this flow as an atomic flow with a fixed ID or if we use
> >>  	 * an incrementing ID.
> >>  	 */
> >> -	if (count == 1 && df && !ipid_offset)
> >> +	if (count == 1 && !ipid_offset)
> >>  		NAPI_GRO_CB(p)->ip_fixedid |= 1 << inner;
> >>  
> >>  	return ipid_offset ^ (count * !(NAPI_GRO_CB(p)->ip_fixedid & (1 << inner)));
> >> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> >> index fc7a6955fa0a..c0542d9187e2 100644
> >> --- a/net/ipv4/af_inet.c
> >> +++ b/net/ipv4/af_inet.c
> >> @@ -1393,10 +1393,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
> >>  
> >>  	segs = ERR_PTR(-EPROTONOSUPPORT);
> >>  
> >> -	/* fixed ID is invalid if DF bit is not set */
> >>  	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));
> >> -	if (fixedid && !(ip_hdr(skb)->frag_off & htons(IP_DF)))
> >> -		goto out;
> >>  
> >>  	if (!skb->encapsulation || encap)
> >>  		udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
> >> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
> >> index d5824eadea10..3d4a82a2607c 100644
> >> --- a/tools/testing/selftests/net/gro.c
> >> +++ b/tools/testing/selftests/net/gro.c
> >> @@ -670,7 +670,7 @@ static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
> >>  		iph2->id = htons(9);
> >>  		break;
> >>  
> >> -	case 3: /* DF=0, Fixed - should not coalesce */
> >> +	case 3: /* DF=0, Fixed - should coalesce */
> >>  		iph1->frag_off &= ~htons(IP_DF);
> >>  		iph1->id = htons(8);
> >>  
> >> @@ -1188,10 +1188,9 @@ static void gro_receiver(void)
> >>  			correct_payload[0] = PAYLOAD_LEN * 2;
> >>  			check_recv_pkts(rxfd, correct_payload, 1);
> >>  
> >> -			printf("DF=0, Fixed - should not coalesce: ");
> >> -			correct_payload[0] = PAYLOAD_LEN;
> >> -			correct_payload[1] = PAYLOAD_LEN;
> >> -			check_recv_pkts(rxfd, correct_payload, 2);
> >> +			printf("DF=0, Fixed - should coalesce: ");
> >> +			correct_payload[0] = PAYLOAD_LEN * 2;
> >> +			check_recv_pkts(rxfd, correct_payload, 1);
> >>  
> >>  			printf("DF=1, 2 Incrementing and one fixed - should coalesce only first 2 packets: ");
> >>  			correct_payload[0] = PAYLOAD_LEN * 2;
> >> -- 
> >> 2.36.1
> >>
> > 
> > 
>

Willem de Bruijn wrote:
> Richard Gobert wrote:
>> Willem de Bruijn wrote:
>>> Richard Gobert wrote:
>>>> Currently, packets with fixed IDs will be merged only if their
>>>> don't-fragment bit is set. This restriction is unnecessary since packets
>>>> without the don't-fragment bit will be forwarded as-is even if they were
>>>> merged together.
>>>
>>> Please expand why this is true.
>>>
>>> Because either NETIF_F_TSO_MANGLEID is set or segmentation
>>> falls back onto software GSO which handles the two FIXEDID
>>> variants correctly now, I guess?
>>>
>>
>> This is true because the merged packets will be segmented back to
>> their original forms before being forwarded. As you already said, the IDs
>> will either stay identical or potentially become incrementing if MANGLEID
>> is set, either of which is fine.
>>
>>>> If packets are merged together and then fragmented, they will first be
>>>> re-split into segments before being further fragmented, so the behavior
>>>> is identical whether or not the packets were first merged together.
>>>
>>> I don't follow this scenario. Fragmentation of a GSO packet after GRO
>>> and before GSO?
>>>
>>
>> Yes. One could worry that merging packets with the same ID but without DF
>> would cause issues if they are then fragmented by the host. What I'm saying
>> is that if such packets are merged and then fragmented, they will first be
>> segmented back to their original forms by GSO before being further fragmented
>> (see ip_finish_output_gso). The fragmentation occurs as if the packets were
>> never merged to begin with.
> 
> This explicit pointer that fragmentation for such GSO packets happens
> in ip_finish_output_gso, which first calls skb_gso_segment, is
> informative. It again turns an assertion into an explanation.
> 
> I think you jumped the gun a bit on sending a v6 right with these
> answers. I'd like these clarifications recorded.
> 

Sorry about that. I'll explicitly mention ip_finish_output_gso in the commit
message.

>> IOW, fragmentation occurs the same way regardless
>> of whether the packets were merged (GRO + GSO is transparent). I thought I'd
>> mention this to clarify why this patch doesn't cause any issues.
>>
>>>> Clean up the code by removing the unnecessary don't-fragment checks.
>>>>
>>>> Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
>>>> ---
>>>>  include/net/gro.h                 | 5 ++---
>>>>  net/ipv4/af_inet.c                | 3 ---
>>>>  tools/testing/selftests/net/gro.c | 9 ++++-----
>>>>  3 files changed, 6 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/include/net/gro.h b/include/net/gro.h
>>>> index 6aa563eec3d0..f14b7e88dbef 100644
>>>> --- a/include/net/gro.h
>>>> +++ b/include/net/gro.h
>>>> @@ -448,17 +448,16 @@ static inline int inet_gro_flush(const struct iphdr *iph, const struct iphdr *ip
>>>>  	const u32 id2 = ntohl(*(__be32 *)&iph2->id);
>>>>  	const u16 ipid_offset = (id >> 16) - (id2 >> 16);
>>>>  	const u16 count = NAPI_GRO_CB(p)->count;
>>>> -	const u32 df = id & IP_DF;
>>>>  
>>>>  	/* All fields must match except length and checksum. */
>>>> -	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | (df ^ (id2 & IP_DF)))
>>>> +	if ((iph->ttl ^ iph2->ttl) | (iph->tos ^ iph2->tos) | ((id ^ id2) & IP_DF))
>>>>  		return true;
>>>>  
>>>>  	/* When we receive our second frame we can make a decision on if we
>>>>  	 * continue this flow as an atomic flow with a fixed ID or if we use
>>>>  	 * an incrementing ID.
>>>>  	 */
>>>> -	if (count == 1 && df && !ipid_offset)
>>>> +	if (count == 1 && !ipid_offset)
>>>>  		NAPI_GRO_CB(p)->ip_fixedid |= 1 << inner;
>>>>  
>>>>  	return ipid_offset ^ (count * !(NAPI_GRO_CB(p)->ip_fixedid & (1 << inner)));
>>>> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
>>>> index fc7a6955fa0a..c0542d9187e2 100644
>>>> --- a/net/ipv4/af_inet.c
>>>> +++ b/net/ipv4/af_inet.c
>>>> @@ -1393,10 +1393,7 @@ struct sk_buff *inet_gso_segment(struct sk_buff *skb,
>>>>  
>>>>  	segs = ERR_PTR(-EPROTONOSUPPORT);
>>>>  
>>>> -	/* fixed ID is invalid if DF bit is not set */
>>>>  	fixedid = !!(skb_shinfo(skb)->gso_type & (SKB_GSO_TCP_FIXEDID << encap));
>>>> -	if (fixedid && !(ip_hdr(skb)->frag_off & htons(IP_DF)))
>>>> -		goto out;
>>>>  
>>>>  	if (!skb->encapsulation || encap)
>>>>  		udpfrag = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
>>>> diff --git a/tools/testing/selftests/net/gro.c b/tools/testing/selftests/net/gro.c
>>>> index d5824eadea10..3d4a82a2607c 100644
>>>> --- a/tools/testing/selftests/net/gro.c
>>>> +++ b/tools/testing/selftests/net/gro.c
>>>> @@ -670,7 +670,7 @@ static void send_flush_id_case(int fd, struct sockaddr_ll *daddr, int tcase)
>>>>  		iph2->id = htons(9);
>>>>  		break;
>>>>  
>>>> -	case 3: /* DF=0, Fixed - should not coalesce */
>>>> +	case 3: /* DF=0, Fixed - should coalesce */
>>>>  		iph1->frag_off &= ~htons(IP_DF);
>>>>  		iph1->id = htons(8);
>>>>  
>>>> @@ -1188,10 +1188,9 @@ static void gro_receiver(void)
>>>>  			correct_payload[0] = PAYLOAD_LEN * 2;
>>>>  			check_recv_pkts(rxfd, correct_payload, 1);
>>>>  
>>>> -			printf("DF=0, Fixed - should not coalesce: ");
>>>> -			correct_payload[0] = PAYLOAD_LEN;
>>>> -			correct_payload[1] = PAYLOAD_LEN;
>>>> -			check_recv_pkts(rxfd, correct_payload, 2);
>>>> +			printf("DF=0, Fixed - should coalesce: ");
>>>> +			correct_payload[0] = PAYLOAD_LEN * 2;
>>>> +			check_recv_pkts(rxfd, correct_payload, 1);
>>>>  
>>>>  			printf("DF=1, 2 Incrementing and one fixed - should coalesce only first 2 packets: ");
>>>>  			correct_payload[0] = PAYLOAD_LEN * 2;
>>>> -- 
>>>> 2.36.1
>>>>
>>>
>>>
>>
> 
>