Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 24 Nov 2022 12:55:27 +0100
From:   Richard Gobert <richardbgobert@gmail.com>
To:     edumazet@google.com, davem@davemloft.net, yoshfuji@linux-ipv6.org,
        dsahern@kernel.org, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH net-next 2/2] gro: change confusing use of flush variable
Message-ID: <20221124115507.GA73719@debian>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

The variable "flush" in tcp_gro_received is confusingly used for two different
purposes. The first use is to keep track whether we are going to flush the SKB
at the end of the function (the same use as in other GRO receive functions). The
second use is just after the "found" label, where it is used to keep track
whether we are going to skip the call to skb_gro_receive that merges the SKBs.
This is entirely not related to the previous use, but uses the same variable.

To make the code more readable, this patch moves the second use to a separate
variable called "dont_merge".

Signed-off-by: Richard Gobert <richardbgobert@gmail.com>
---
 net/ipv4/tcp_offload.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index f17271e5c7c5..7ca6be2f56df 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -191,6 +191,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
 	unsigned int hlen;
 	unsigned int off;
 	int flush = 1;
+	int dont_merge;
 	int i;
 
 	off = skb_gro_offset(skb);
@@ -234,13 +235,13 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 found:
 	/* Include the IP ID check below from the inner most IP hdr */
-	flush = NAPI_GRO_CB(p)->flush;
-	flush |= (__force int)(flags & TCP_FLAG_CWR);
-	flush |= (__force int)((flags ^ tcp_flag_word(th2)) &
+	dont_merge = NAPI_GRO_CB(p)->flush;
+	dont_merge |= (__force int)(flags & TCP_FLAG_CWR);
+	dont_merge |= (__force int)((flags ^ tcp_flag_word(th2)) &
 		  ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH));
-	flush |= (__force int)(th->ack_seq ^ th2->ack_seq);
+	dont_merge |= (__force int)(th->ack_seq ^ th2->ack_seq);
 	for (i = sizeof(*th); i < thlen; i += 4)
-		flush |= *(u32 *)((u8 *)th + i) ^
+		dont_merge |= *(u32 *)((u8 *)th + i) ^
 			 *(u32 *)((u8 *)th2 + i);
 
 	/* When we receive our second frame we can made a decision on if we
@@ -250,7 +251,7 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
 	if (NAPI_GRO_CB(p)->flush_id != 1 ||
 	    NAPI_GRO_CB(p)->count != 1 ||
 	    !NAPI_GRO_CB(p)->is_atomic)
-		flush |= NAPI_GRO_CB(p)->flush_id;
+		dont_merge |= NAPI_GRO_CB(p)->flush_id;
 	else
 		NAPI_GRO_CB(p)->is_atomic = false;
 
@@ -261,16 +262,16 @@ struct sk_buff *tcp_gro_receive(struct list_head *head, struct sk_buff *skb)
 	 * is bigger than our mss.
 	 */
 	if (unlikely(skb_is_gso(skb)))
-		flush |= (mss != skb_shinfo(skb)->gso_size);
+		dont_merge |= (mss != skb_shinfo(skb)->gso_size);
 	else
-		flush |= (len - 1) >= mss;
+		dont_merge |= (len - 1) >= mss;
 
-	flush |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq);
+	dont_merge |= (ntohl(th2->seq) + skb_gro_len(p)) ^ ntohl(th->seq);
 #ifdef CONFIG_TLS_DEVICE
-	flush |= p->decrypted ^ skb->decrypted;
+	dont_merge |= p->decrypted ^ skb->decrypted;
 #endif
 
-	if (flush || skb_gro_receive(p, skb)) {
+	if (dont_merge || skb_gro_receive(p, skb)) {
 		flush = 0;
 		goto out_check_final;
 	}
-- 
2.36.1