Skip to contentSkip to conversation

: Fix HW checksum handling in TCPMSS target

PATCH In review Patrick McHardy 2 participants 3 messages Aug 20, 2005 – Aug 21, 2005

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Patrick McHardy

author
#0 lore raw

PATCH 3/3 : Fix HW checksum handling in TCPMSS target

[NETFILTER]: Fix HW checksum handling in TCPMSS target

Most importantly, remove bogus BUG() in receive path.

Signed-off-by Patrick McHardy <[email protected]>

---
commit d02587b6086d97c5bb1236c817702f87114c03cc
tree d962e88ca0c71a2f082a6926afffbfd18217d856
parent b477e1c6defc1b0e1181350908714e11de5768dd
author Patrick McHardy <[email protected]> Sat, 20 Aug 2005 03:35:30 +0200 committer Patrick McHardy <[email protected]> Sat, 20 Aug 2005 03:35:30 +0200

 net/ipv4/netfilter/ipt_TCPMSS.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)
net/ipv4/netfilter/ipt_TCPMSS.c +19 -13
@@ -61,6 +61,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	if (!skb_ip_make_writable(pskb, (*pskb)->len))
 		return NF_DROP;
 
+	if ((*pskb)->ip_summed == CHECKSUM_HW &&
+	    skb_checksum_help(*pskb, out == NULL))
+		return NF_DROP;
+
 	iph = (*pskb)->nh.iph;
 	tcplen = (*pskb)->len - iph->ihl*4;
 

@@ -114,9 +118,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 			opt[i+2] = (newmss & 0xff00) >> 8;
 			opt[i+3] = (newmss & 0x00ff);
 
-			tcph->check = cheat_check(htons(oldmss)^0xFFFF,
-						  htons(newmss),
-						  tcph->check);
+			if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+				tcph->check = cheat_check(htons(oldmss)^0xFFFF,
+							  htons(newmss),
+							  tcph->check);
 
 			DEBUGP(KERN_INFO "ipt_tcpmss_target: %u.%u.%u.%u:%hu"
 			       "->%u.%u.%u.%u:%hu changed TCP MSS option"

@@ -126,7 +131,7 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 			       NIPQUAD((*pskb)->nh.iph->daddr),
 			       ntohs(tcph->dest),
 			       oldmss, newmss);
-			goto retmodified;
+			goto out;
 		}
 	}
 

@@ -156,8 +161,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
  	opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
 	memmove(opt + TCPOLEN_MSS, opt, tcplen - sizeof(struct tcphdr));
 
-	tcph->check = cheat_check(htons(tcplen) ^ 0xFFFF,
-				  htons(tcplen + TCPOLEN_MSS), tcph->check);
+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+		tcph->check = cheat_check(htons(tcplen) ^ 0xFFFF,
+					  htons(tcplen + TCPOLEN_MSS),
+					  tcph->check);
 	tcplen += TCPOLEN_MSS;
 
 	opt[0] = TCPOPT_MSS;

@@ -165,12 +172,14 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	opt[2] = (newmss & 0xff00) >> 8;
 	opt[3] = (newmss & 0x00ff);
 
-	tcph->check = cheat_check(~0, *((u_int32_t *)opt), tcph->check);
+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+		tcph->check = cheat_check(~0, *((u_int32_t *)opt), tcph->check);
 
 	oldval = ((u_int16_t *)tcph)[6];
 	tcph->doff += TCPOLEN_MSS/4;
-	tcph->check = cheat_check(oldval ^ 0xFFFF,
-				  ((u_int16_t *)tcph)[6], tcph->check);
+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+		tcph->check = cheat_check(oldval ^ 0xFFFF,
+					  ((u_int16_t *)tcph)[6], tcph->check);
 
 	newtotlen = htons(ntohs(iph->tot_len) + TCPOLEN_MSS);
 	iph->check = cheat_check(iph->tot_len ^ 0xFFFF,

@@ -185,10 +194,7 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	       ntohs(tcph->dest),
 	       newmss);
 
- retmodified:
-	/* We never hw checksum SYN packets.  */
-	BUG_ON((*pskb)->ip_summed == CHECKSUM_HW);
-
+out:
 	(*pskb)->nfcache |= NFC_UNKNOWN | NFC_ALTERED;
 	return IPT_CONTINUE;
 }

[NETFILTER]: Fix HW checksum handling in TCPMSS target

Most importantly, remove bogus BUG() in receive path.

Signed-off-by Patrick McHardy <[email protected]>

---
commit be3af8346490b612b4f031e5421efcc3660ae6a8
tree e685b08f68938861c6e16744703e5e53e24e421d
parent f1929b1a15a9a7d9761b344d3522698bd656154e
author Patrick McHardy <[email protected]> Sat, 20 Aug 2005 03:29:14 +0200 committer Patrick McHardy <[email protected]> Sat, 20 Aug 2005 03:29:14 +0200

 net/ipv4/netfilter/ipt_TCPMSS.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)
net/ipv4/netfilter/ipt_TCPMSS.c +19 -13
@@ -61,6 +61,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	if (!skb_make_writable(pskb, (*pskb)->len))
 		return NF_DROP;
 
+	if ((*pskb)->ip_summed == CHECKSUM_HW &&
+	    skb_checksum_help(*pskb, out == NULL))
+		return NF_DROP;
+
 	iph = (*pskb)->nh.iph;
 	tcplen = (*pskb)->len - iph->ihl*4;
 

@@ -114,9 +118,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 			opt[i+2] = (newmss & 0xff00) >> 8;
 			opt[i+3] = (newmss & 0x00ff);
 
-			tcph->check = cheat_check(htons(oldmss)^0xFFFF,
-						  htons(newmss),
-						  tcph->check);
+			if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+				tcph->check = cheat_check(htons(oldmss)^0xFFFF,
+							  htons(newmss),
+							  tcph->check);
 
 			DEBUGP(KERN_INFO "ipt_tcpmss_target: %u.%u.%u.%u:%hu"
 			       "->%u.%u.%u.%u:%hu changed TCP MSS option"

@@ -126,7 +131,7 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 			       NIPQUAD((*pskb)->nh.iph->daddr),
 			       ntohs(tcph->dest),
 			       oldmss, newmss);
-			goto retmodified;
+			goto out;
 		}
 	}
 

@@ -156,8 +161,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
  	opt = (u_int8_t *)tcph + sizeof(struct tcphdr);
 	memmove(opt + TCPOLEN_MSS, opt, tcplen - sizeof(struct tcphdr));
 
-	tcph->check = cheat_check(htons(tcplen) ^ 0xFFFF,
-				  htons(tcplen + TCPOLEN_MSS), tcph->check);
+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+		tcph->check = cheat_check(htons(tcplen) ^ 0xFFFF,
+					  htons(tcplen + TCPOLEN_MSS),
+					  tcph->check);
 	tcplen += TCPOLEN_MSS;
 
 	opt[0] = TCPOPT_MSS;

@@ -165,12 +172,14 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	opt[2] = (newmss & 0xff00) >> 8;
 	opt[3] = (newmss & 0x00ff);
 
-	tcph->check = cheat_check(~0, *((u_int32_t *)opt), tcph->check);
+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+		tcph->check = cheat_check(~0, *((u_int32_t *)opt), tcph->check);
 
 	oldval = ((u_int16_t *)tcph)[6];
 	tcph->doff += TCPOLEN_MSS/4;
-	tcph->check = cheat_check(oldval ^ 0xFFFF,
-				  ((u_int16_t *)tcph)[6], tcph->check);
+	if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+		tcph->check = cheat_check(oldval ^ 0xFFFF,
+					  ((u_int16_t *)tcph)[6], tcph->check);
 
 	newtotlen = htons(ntohs(iph->tot_len) + TCPOLEN_MSS);
 	iph->check = cheat_check(iph->tot_len ^ 0xFFFF,

@@ -185,10 +194,7 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	       ntohs(tcph->dest),
 	       newmss);
 
- retmodified:
-	/* We never hw checksum SYN packets.  */
-	BUG_ON((*pskb)->ip_summed == CHECKSUM_HW);
-
+out:
 	return IPT_CONTINUE;
 }
author
#1 lore raw

Review ipt_tcpmss_target(struct sk_buff **pskb, · line 124

Quoted 11 lines · Patrick McHardy
(quoted diff) +4 -3
@@ -114,9 +118,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 			opt[i+2] = (newmss & 0xff00) >> 8;
 			opt[i+3] = (newmss & 0x00ff);
 
-			tcph->check = cheat_check(htons(oldmss)^0xFFFF,
-						  htons(newmss),
-						  tcph->check);
+			if ((*pskb)->ip_summed != CHECKSUM_UNNECESSARY)
+				tcph->check = cheat_check(htons(oldmss)^0xFFFF,
+							  htons(newmss),
+							  tcph->check);

On second thought, this is not a good idea, we need to update the checksum in any case for forwarded packets. These patches should be better.

[NETFILTER]: Fix HW checksum handling in TCPMSS target

Most importantly, remove bogus BUG() in receive path.

Signed-off-by Patrick McHardy <[email protected]>

---
commit 2f22a5a1a34286cde024bef21c75299a9ec8b564
tree 2ff9f0f32fd4cef9df2c121bf6772ab254f8ec41
parent b477e1c6defc1b0e1181350908714e11de5768dd
author Patrick McHardy <[email protected]> Sat, 20 Aug 2005 04:11:03 +0200 committer Patrick McHardy <[email protected]> Sat, 20 Aug 2005 04:11:03 +0200

 net/ipv4/netfilter/ipt_TCPMSS.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
net/ipv4/netfilter/ipt_TCPMSS.c +4 -3
@@ -61,6 +61,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	if (!skb_ip_make_writable(pskb, (*pskb)->len))
 		return NF_DROP;
 
+	if ((*pskb)->ip_summed == CHECKSUM_HW &&
+	    skb_checksum_help(*pskb, out == NULL))
+		return NF_DROP;
+
 	iph = (*pskb)->nh.iph;
 	tcplen = (*pskb)->len - iph->ihl*4;
 

@@ -186,9 +190,6 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	       newmss);
 
  retmodified:
-	/* We never hw checksum SYN packets.  */
-	BUG_ON((*pskb)->ip_summed == CHECKSUM_HW);
-
 	(*pskb)->nfcache |= NFC_UNKNOWN | NFC_ALTERED;
 	return IPT_CONTINUE;
 }

[NETFILTER]: Fix HW checksum handling in TCPMSS target

Most importantly, remove bogus BUG() in receive path.

Signed-off-by Patrick McHardy <[email protected]>

---
commit 7591ae593acedfcb570f713902b9dbd6b4ae3581
tree 92a7500452c3edd781680c93b93056d04ce66d87
parent f1929b1a15a9a7d9761b344d3522698bd656154e
author Patrick McHardy <[email protected]> Sat, 20 Aug 2005 04:10:18 +0200 committer Patrick McHardy <[email protected]> Sat, 20 Aug 2005 04:10:18 +0200

 net/ipv4/netfilter/ipt_TCPMSS.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
net/ipv4/netfilter/ipt_TCPMSS.c +4 -3
@@ -61,6 +61,10 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	if (!skb_make_writable(pskb, (*pskb)->len))
 		return NF_DROP;
 
+	if ((*pskb)->ip_summed == CHECKSUM_HW &&
+	    skb_checksum_help(*pskb, out == NULL))
+		return NF_DROP;
+
 	iph = (*pskb)->nh.iph;
 	tcplen = (*pskb)->len - iph->ihl*4;
 

@@ -186,9 +190,6 @@ ipt_tcpmss_target(struct sk_buff **pskb,
 	       newmss);
 
  retmodified:
-	/* We never hw checksum SYN packets.  */
-	BUG_ON((*pskb)->ip_summed == CHECKSUM_HW);
-
 	return IPT_CONTINUE;
 }
maintainer
#2 lore raw

From: Patrick McHardy <[email protected]>
Date: Sat, 20 Aug 2005 04:12:46 +0200

Quoted 3 lines

On second thought, this is not a good idea, we need to update the checksum in any case for forwarded packets. These patches should be better.

All 3 patches applied to both trees, thanks Patrick.