[PATCH] netfilter: remove duplicate code

Subsystems: netfilter, networking [general], the rest

STALE1823d

3 messages, 2 authors, 2021-08-12 · open the first message on its own page

[PATCH] netfilter: remove duplicate code

From: Kangmin Park <hidden>
Date: 2021-08-07 06:21:19

nf_nat_ipv4_fn() and nf_nat_ipv6_fn() call nf_nat_inet_fn().
Those two functions are already contains routine that gets nf_conn
object and checks the untrackable situation.
So, the following code is duplicated.
ct = nf_ct_get(skb, &ctinfo);
if (!ct)
        return NF_ACCEPT;
Therefore, define a function __nf_nat_inet_fn() that has the same
contents as the nf_nat_inet_fn() except for routine gets and checks
the nf_conn object.
Then, separate the nf_nat_inet_fn() into a routine that gets a
nf_conn object and a routine that calls the __nf_nat_inet_fn().

Signed-off-by: Kangmin Park <redacted>
---
 include/net/netfilter/nf_nat.h |  5 +++++
 net/netfilter/nf_nat_core.c    | 37 ++++++++++++++++++++++------------
 net/netfilter/nf_nat_proto.c   |  4 ++--
 3 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index 987111ae5240..a66f617c5054 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -100,6 +100,11 @@ void nf_nat_ipv6_unregister_fn(struct net *net, const struct nf_hook_ops *ops);
 int nf_nat_inet_register_fn(struct net *net, const struct nf_hook_ops *ops);
 void nf_nat_inet_unregister_fn(struct net *net, const struct nf_hook_ops *ops);
 
+unsigned int
+__nf_nat_inet_fn(void *priv, struct sk_buff *skb,
+		 const struct nf_hook_state *state, struct nf_conn *ct,
+		 enum ip_conntrack_info ctinfo);
+
 unsigned int
 nf_nat_inet_fn(void *priv, struct sk_buff *skb,
 	       const struct nf_hook_state *state);
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 7de595ead06a..98ebba2c0f6d 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -682,25 +682,15 @@ unsigned int nf_nat_packet(struct nf_conn *ct,
 }
 EXPORT_SYMBOL_GPL(nf_nat_packet);
 
 unsigned int
-nf_nat_inet_fn(void *priv, struct sk_buff *skb,
-	       const struct nf_hook_state *state)
+__nf_nat_inet_fn(void *priv, struct sk_buff *skb,
+		 const struct nf_hook_state *state, struct nf_conn *ct,
+		 enum ip_conntrack_info ctinfo)
 {
-	struct nf_conn *ct;
-	enum ip_conntrack_info ctinfo;
 	struct nf_conn_nat *nat;
 	/* maniptype == SRC for postrouting. */
 	enum nf_nat_manip_type maniptype = HOOK2MANIP(state->hook);
 
-	ct = nf_ct_get(skb, &ctinfo);
-	/* Can't track?  It's not due to stress, or conntrack would
-	 * have dropped it.  Hence it's the user's responsibilty to
-	 * packet filter it out, or implement conntrack/NAT for that
-	 * protocol. 8) --RR
-	 */
-	if (!ct)
-		return NF_ACCEPT;
-
 	nat = nfct_nat(ct);
 
 	switch (ctinfo) {
@@ -755,6 +745,26 @@ nf_nat_inet_fn(void *priv, struct sk_buff *skb,
 	nf_ct_kill_acct(ct, ctinfo, skb);
 	return NF_DROP;
 }
+EXPORT_SYMBOL_GPL(__nf_nat_inet_fn);
+
+unsigned int
+nf_nat_inet_fn(void *priv, struct sk_buff *skb,
+	       const struct nf_hook_state *state)
+{
+	struct nf_conn *ct;
+	enum ip_conntrack_info ctinfo;
+
+	ct = nf_ct_get(skb, &ctinfo);
+	/* Can't track?  It's not due to stress, or conntrack would
+	 * have dropped it.  Hence it's the user's responsibilty to
+	 * packet filter it out, or implement conntrack/NAT for that
+	 * protocol. 8) --RR
+	 */
+	if (!ct)
+		return NF_ACCEPT;
+
+	return __nf_nat_inet_fn(priv, skb, state, ct, ctinfo);
+}
 EXPORT_SYMBOL_GPL(nf_nat_inet_fn);
 
 struct nf_nat_proto_clean {
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 48cc60084d28..897859730078 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -642,7 +642,7 @@ nf_nat_ipv4_fn(void *priv, struct sk_buff *skb,
 		}
 	}
 
-	return nf_nat_inet_fn(priv, skb, state);
+	return __nf_nat_inet_fn(priv, skb, state, ct, ctinfo);
 }
 
 static unsigned int
@@ -934,7 +934,7 @@ nf_nat_ipv6_fn(void *priv, struct sk_buff *skb,
 		}
 	}
 
-	return nf_nat_inet_fn(priv, skb, state);
+	return __nf_nat_inet_fn(priv, skb, state, ct, ctinfo);
 }
 
 static unsigned int
-- 
2.26.2

Re: [PATCH] netfilter: remove duplicate code

From: Kangmin Park <hidden>
Date: 2021-08-09 18:35:26

I checked the Changes Requested state in patchwork.
But I have not received any review mails.
I wonder if there is any problem.
I'm sorry if you just review a little late due to a busy schedule.

Kangmin Park

2021년 8월 7일 (토) 오후 3:21, Kangmin Park [off-list ref]님이 작성:
quoted hunk
nf_nat_ipv4_fn() and nf_nat_ipv6_fn() call nf_nat_inet_fn().
Those two functions are already contains routine that gets nf_conn
object and checks the untrackable situation.
So, the following code is duplicated.
ct = nf_ct_get(skb, &ctinfo);
if (!ct)
        return NF_ACCEPT;
Therefore, define a function __nf_nat_inet_fn() that has the same
contents as the nf_nat_inet_fn() except for routine gets and checks
the nf_conn object.
Then, separate the nf_nat_inet_fn() into a routine that gets a
nf_conn object and a routine that calls the __nf_nat_inet_fn().

Signed-off-by: Kangmin Park <redacted>
---
 include/net/netfilter/nf_nat.h |  5 +++++
 net/netfilter/nf_nat_core.c    | 37 ++++++++++++++++++++++------------
 net/netfilter/nf_nat_proto.c   |  4 ++--
 3 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/include/net/netfilter/nf_nat.h b/include/net/netfilter/nf_nat.h
index 987111ae5240..a66f617c5054 100644
--- a/include/net/netfilter/nf_nat.h
+++ b/include/net/netfilter/nf_nat.h
@@ -100,6 +100,11 @@ void nf_nat_ipv6_unregister_fn(struct net *net, const struct nf_hook_ops *ops);
 int nf_nat_inet_register_fn(struct net *net, const struct nf_hook_ops *ops);
 void nf_nat_inet_unregister_fn(struct net *net, const struct nf_hook_ops *ops);

+unsigned int
+__nf_nat_inet_fn(void *priv, struct sk_buff *skb,
+                const struct nf_hook_state *state, struct nf_conn *ct,
+                enum ip_conntrack_info ctinfo);
+
 unsigned int
 nf_nat_inet_fn(void *priv, struct sk_buff *skb,
               const struct nf_hook_state *state);
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 7de595ead06a..98ebba2c0f6d 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -682,25 +682,15 @@ unsigned int nf_nat_packet(struct nf_conn *ct,
 }
 EXPORT_SYMBOL_GPL(nf_nat_packet);

 unsigned int
-nf_nat_inet_fn(void *priv, struct sk_buff *skb,
-              const struct nf_hook_state *state)
+__nf_nat_inet_fn(void *priv, struct sk_buff *skb,
+                const struct nf_hook_state *state, struct nf_conn *ct,
+                enum ip_conntrack_info ctinfo)
 {
-       struct nf_conn *ct;
-       enum ip_conntrack_info ctinfo;
        struct nf_conn_nat *nat;
        /* maniptype == SRC for postrouting. */
        enum nf_nat_manip_type maniptype = HOOK2MANIP(state->hook);

-       ct = nf_ct_get(skb, &ctinfo);
-       /* Can't track?  It's not due to stress, or conntrack would
-        * have dropped it.  Hence it's the user's responsibilty to
-        * packet filter it out, or implement conntrack/NAT for that
-        * protocol. 8) --RR
-        */
-       if (!ct)
-               return NF_ACCEPT;
-
        nat = nfct_nat(ct);

        switch (ctinfo) {
@@ -755,6 +745,26 @@ nf_nat_inet_fn(void *priv, struct sk_buff *skb,
        nf_ct_kill_acct(ct, ctinfo, skb);
        return NF_DROP;
 }
+EXPORT_SYMBOL_GPL(__nf_nat_inet_fn);
+
+unsigned int
+nf_nat_inet_fn(void *priv, struct sk_buff *skb,
+              const struct nf_hook_state *state)
+{
+       struct nf_conn *ct;
+       enum ip_conntrack_info ctinfo;
+
+       ct = nf_ct_get(skb, &ctinfo);
+       /* Can't track?  It's not due to stress, or conntrack would
+        * have dropped it.  Hence it's the user's responsibilty to
+        * packet filter it out, or implement conntrack/NAT for that
+        * protocol. 8) --RR
+        */
+       if (!ct)
+               return NF_ACCEPT;
+
+       return __nf_nat_inet_fn(priv, skb, state, ct, ctinfo);
+}
 EXPORT_SYMBOL_GPL(nf_nat_inet_fn);

 struct nf_nat_proto_clean {
diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
index 48cc60084d28..897859730078 100644
--- a/net/netfilter/nf_nat_proto.c
+++ b/net/netfilter/nf_nat_proto.c
@@ -642,7 +642,7 @@ nf_nat_ipv4_fn(void *priv, struct sk_buff *skb,
                }
        }

-       return nf_nat_inet_fn(priv, skb, state);
+       return __nf_nat_inet_fn(priv, skb, state, ct, ctinfo);
 }

 static unsigned int
@@ -934,7 +934,7 @@ nf_nat_ipv6_fn(void *priv, struct sk_buff *skb,
                }
        }

-       return nf_nat_inet_fn(priv, skb, state);
+       return __nf_nat_inet_fn(priv, skb, state, ct, ctinfo);
 }

 static unsigned int
--
2.26.2

Re: [PATCH] netfilter: remove duplicate code

From: Florian Westphal <fw@strlen.de>
Date: 2021-08-12 14:31:31

Kangmin Park [off-list ref] wrote:
I checked the Changes Requested state in patchwork.
But I have not received any review mails.
I did not see any either.
I wonder if there is any problem.
I don't think its worth doing, the 'extra' check avoids the
need for two additional function arguments (and those are not free
either).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help