diff mbox

[nf-next] netfilter: xt_TCPMSS: Refactor the codes to decrease one condition check and more readable

Message ID 1473216024-7824-1-git-send-email-fgao@ikuai8.com
State Accepted
Delegated to: Pablo Neira
Headers show

Commit Message

高峰 Sept. 7, 2016, 2:40 a.m. UTC 
From: Gao Feng <fgao@ikuai8.com>

The origin codes perform two condition checks with dst_mtu(skb_dst(skb))
and in_mtu. And the last statement is "min(dst_mtu(skb_dst(skb)),
in_mtu) - minlen". It may let reader think about how about the result.
Would it be negative.

Now assign the result of min(dst_mtu(skb_dst(skb)), in_mtu) to a new
variable, then only perform one condition check, and it is more readable.

Signed-off-by: Gao Feng <fgao@ikuai8.com>
---
 net/netfilter/xt_TCPMSS.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Comments

Pablo Neira Ayuso Sept. 24, 2016, 7:14 p.m. UTC | #1 
On Wed, Sep 07, 2016 at 10:40:24AM +0800, fgao@ikuai8.com wrote:
> From: Gao Feng <fgao@ikuai8.com>
> 
> The origin codes perform two condition checks with dst_mtu(skb_dst(skb))
> and in_mtu. And the last statement is "min(dst_mtu(skb_dst(skb)),
> in_mtu) - minlen". It may let reader think about how about the result.
> Would it be negative.
> 
> Now assign the result of min(dst_mtu(skb_dst(skb)), in_mtu) to a new
> variable, then only perform one condition check, and it is more readable.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c
index e118397..872db2d 100644
--- a/net/netfilter/xt_TCPMSS.c
+++ b/net/netfilter/xt_TCPMSS.c
@@ -110,18 +110,14 @@  tcpmss_mangle_packet(struct sk_buff *skb,
 	if (info->mss == XT_TCPMSS_CLAMP_PMTU) {
 		struct net *net = par->net;
 		unsigned int in_mtu = tcpmss_reverse_mtu(net, skb, family);
+		unsigned int min_mtu = min(dst_mtu(skb_dst(skb)), in_mtu);
 
-		if (dst_mtu(skb_dst(skb)) <= minlen) {
+		if (min_mtu <= minlen) {
 			net_err_ratelimited("unknown or invalid path-MTU (%u)\n",
-					    dst_mtu(skb_dst(skb)));
+					    min_mtu);
 			return -1;
 		}
-		if (in_mtu <= minlen) {
-			net_err_ratelimited("unknown or invalid path-MTU (%u)\n",
-					    in_mtu);
-			return -1;
-		}
-		newmss = min(dst_mtu(skb_dst(skb)), in_mtu) - minlen;
+		newmss = min_mtu - minlen;
 	} else
 		newmss = info->mss;