All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns
@ 2021-07-22  8:48 Florian Westphal
  2021-07-22  8:48 ` [PATCH nf-next 1/3] netfilter: ipt_CLUSTERIP: only add arp mangle hook when required Florian Westphal
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Florian Westphal @ 2021-07-22  8:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

This series stops ipt_CLUSTERIP from registering arp mangling hook
unconditionally.

Hook gets installed/removed from checkentry/destroy callbacks.

Before this, modprobe ipt_CLUSTERIP would add a hook in each netns.
While at it, also get rid of x_tables.h/xt storage space in struct net,
there is no need for this.

Florian Westphal (3):
  netfilter: ipt_CLUSTERIP: only add arp mangle hook when required
  netfilter: ipt_CLUSTERIP: use clusterip_net to store pernet warning
  netfilter: remove xt pernet data

 include/net/net_namespace.h        |  2 --
 include/net/netns/x_tables.h       | 12 -------
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 56 ++++++++++++++++++++----------
 net/netfilter/xt_CT.c              | 11 ------
 4 files changed, 37 insertions(+), 44 deletions(-)
 delete mode 100644 include/net/netns/x_tables.h

-- 
2.31.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH nf-next 1/3] netfilter: ipt_CLUSTERIP: only add arp mangle hook when required
  2021-07-22  8:48 [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Florian Westphal
@ 2021-07-22  8:48 ` Florian Westphal
  2021-07-22  8:48 ` [PATCH nf-next 2/3] netfilter: ipt_CLUSTERIP: use clusterip_net to store pernet warning Florian Westphal
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2021-07-22  8:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Do not register the arp mangling hooks from pernet init path.

As-is, load of the module is enough for these hooks to become active
in each net namespace.

Use checkentry instead so hook is only added if a CLUSTERIP rule is used.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 51 ++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 8f7ca67475b7..11bcf599358d 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -71,6 +71,16 @@ struct clusterip_net {
 	/* mutex protects the config->pde*/
 	struct mutex mutex;
 #endif
+	unsigned int hook_users;
+};
+
+static unsigned int clusterip_arp_mangle(void *priv, struct sk_buff *skb, const struct nf_hook_state *state);
+
+static const struct nf_hook_ops cip_arp_ops = {
+	.hook = clusterip_arp_mangle,
+	.pf = NFPROTO_ARP,
+	.hooknum = NF_ARP_OUT,
+	.priority = -1
 };
 
 static unsigned int clusterip_net_id __read_mostly;
@@ -458,6 +468,7 @@ clusterip_tg(struct sk_buff *skb, const struct xt_action_param *par)
 static int clusterip_tg_check(const struct xt_tgchk_param *par)
 {
 	struct ipt_clusterip_tgt_info *cipinfo = par->targinfo;
+	struct clusterip_net *cn = clusterip_pernet(par->net);
 	const struct ipt_entry *e = par->entryinfo;
 	struct clusterip_config *config;
 	int ret, i;
@@ -467,6 +478,9 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 		return -EOPNOTSUPP;
 	}
 
+	if (cn->hook_users == UINT_MAX)
+		return -EOVERFLOW;
+
 	if (cipinfo->hash_mode != CLUSTERIP_HASHMODE_SIP &&
 	    cipinfo->hash_mode != CLUSTERIP_HASHMODE_SIP_SPT &&
 	    cipinfo->hash_mode != CLUSTERIP_HASHMODE_SIP_SPT_DPT) {
@@ -517,6 +531,19 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 		return ret;
 	}
 
+	if (cn->hook_users == 0) {
+		ret = nf_register_net_hook(par->net, &cip_arp_ops);
+
+		if (ret < 0) {
+			clusterip_config_entry_put(cipinfo->config);
+			clusterip_config_put(cipinfo->config);
+			nf_ct_netns_put(par->net, par->family);
+			return ret;
+		}
+	}
+
+	cn->hook_users++;
+
 	if (!par->net->xt.clusterip_deprecated_warning) {
 		pr_info("ipt_CLUSTERIP is deprecated and it will removed soon, "
 			"use xt_cluster instead\n");
@@ -531,6 +558,7 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 static void clusterip_tg_destroy(const struct xt_tgdtor_param *par)
 {
 	const struct ipt_clusterip_tgt_info *cipinfo = par->targinfo;
+	struct clusterip_net *cn = clusterip_pernet(par->net);
 
 	/* if no more entries are referencing the config, remove it
 	 * from the list and destroy the proc entry */
@@ -539,6 +567,10 @@ static void clusterip_tg_destroy(const struct xt_tgdtor_param *par)
 	clusterip_config_put(cipinfo->config);
 
 	nf_ct_netns_put(par->net, par->family);
+	cn->hook_users--;
+
+	if (cn->hook_users == 0)
+		nf_unregister_net_hook(par->net, &cip_arp_ops);
 }
 
 #ifdef CONFIG_NETFILTER_XTABLES_COMPAT
@@ -602,9 +634,8 @@ static void arp_print(struct arp_payload *payload)
 #endif
 
 static unsigned int
-arp_mangle(void *priv,
-	   struct sk_buff *skb,
-	   const struct nf_hook_state *state)
+clusterip_arp_mangle(void *priv, struct sk_buff *skb,
+		     const struct nf_hook_state *state)
 {
 	struct arphdr *arp = arp_hdr(skb);
 	struct arp_payload *payload;
@@ -654,13 +685,6 @@ arp_mangle(void *priv,
 	return NF_ACCEPT;
 }
 
-static const struct nf_hook_ops cip_arp_ops = {
-	.hook = arp_mangle,
-	.pf = NFPROTO_ARP,
-	.hooknum = NF_ARP_OUT,
-	.priority = -1
-};
-
 /***********************************************************************
  * PROC DIR HANDLING
  ***********************************************************************/
@@ -817,20 +841,14 @@ static const struct proc_ops clusterip_proc_ops = {
 static int clusterip_net_init(struct net *net)
 {
 	struct clusterip_net *cn = clusterip_pernet(net);
-	int ret;
 
 	INIT_LIST_HEAD(&cn->configs);
 
 	spin_lock_init(&cn->lock);
 
-	ret = nf_register_net_hook(net, &cip_arp_ops);
-	if (ret < 0)
-		return ret;
-
 #ifdef CONFIG_PROC_FS
 	cn->procdir = proc_mkdir("ipt_CLUSTERIP", net->proc_net);
 	if (!cn->procdir) {
-		nf_unregister_net_hook(net, &cip_arp_ops);
 		pr_err("Unable to proc dir entry\n");
 		return -ENOMEM;
 	}
@@ -850,7 +868,6 @@ static void clusterip_net_exit(struct net *net)
 	cn->procdir = NULL;
 	mutex_unlock(&cn->mutex);
 #endif
-	nf_unregister_net_hook(net, &cip_arp_ops);
 }
 
 static struct pernet_operations clusterip_net_ops = {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH nf-next 2/3] netfilter: ipt_CLUSTERIP: use clusterip_net to store pernet warning
  2021-07-22  8:48 [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Florian Westphal
  2021-07-22  8:48 ` [PATCH nf-next 1/3] netfilter: ipt_CLUSTERIP: only add arp mangle hook when required Florian Westphal
@ 2021-07-22  8:48 ` Florian Westphal
  2021-07-22  8:48 ` [PATCH nf-next 3/3] netfilter: remove xt pernet data Florian Westphal
  2021-08-01  9:46 ` [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2021-07-22  8:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

No need to use struct net for this.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 11bcf599358d..e6f65885d12b 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -66,6 +66,7 @@ struct clusterip_net {
 	/* lock protects the configs list */
 	spinlock_t lock;
 
+	bool clusterip_deprecated_warning;
 #ifdef CONFIG_PROC_FS
 	struct proc_dir_entry *procdir;
 	/* mutex protects the config->pde*/
@@ -544,10 +545,10 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 
 	cn->hook_users++;
 
-	if (!par->net->xt.clusterip_deprecated_warning) {
+	if (!cn->clusterip_deprecated_warning) {
 		pr_info("ipt_CLUSTERIP is deprecated and it will removed soon, "
 			"use xt_cluster instead\n");
-		par->net->xt.clusterip_deprecated_warning = true;
+		cn->clusterip_deprecated_warning = true;
 	}
 
 	cipinfo->config = config;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH nf-next 3/3] netfilter: remove xt pernet data
  2021-07-22  8:48 [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Florian Westphal
  2021-07-22  8:48 ` [PATCH nf-next 1/3] netfilter: ipt_CLUSTERIP: only add arp mangle hook when required Florian Westphal
  2021-07-22  8:48 ` [PATCH nf-next 2/3] netfilter: ipt_CLUSTERIP: use clusterip_net to store pernet warning Florian Westphal
@ 2021-07-22  8:48 ` Florian Westphal
  2021-08-01  9:46 ` [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Florian Westphal @ 2021-07-22  8:48 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

clusterip is now handled via net_generic.

NOTRACK is tiny compared to rest of xt_CT feature set, even the existing
deprecation warning is bigger than the actual functionality.

Just remove the warning, its not worth keeping/adding a net_generic one.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/net_namespace.h  |  2 --
 include/net/netns/x_tables.h | 12 ------------
 net/netfilter/xt_CT.c        | 11 -----------
 3 files changed, 25 deletions(-)
 delete mode 100644 include/net/netns/x_tables.h

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 12cf6d7ea62c..60693fd5de45 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -23,7 +23,6 @@
 #include <net/netns/ieee802154_6lowpan.h>
 #include <net/netns/sctp.h>
 #include <net/netns/netfilter.h>
-#include <net/netns/x_tables.h>
 #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
 #include <net/netns/conntrack.h>
 #endif
@@ -132,7 +131,6 @@ struct net {
 #endif
 #ifdef CONFIG_NETFILTER
 	struct netns_nf		nf;
-	struct netns_xt		xt;
 #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
 	struct netns_ct		ct;
 #endif
diff --git a/include/net/netns/x_tables.h b/include/net/netns/x_tables.h
deleted file mode 100644
index d02316ec2906..000000000000
--- a/include/net/netns/x_tables.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __NETNS_X_TABLES_H
-#define __NETNS_X_TABLES_H
-
-#include <linux/list.h>
-#include <linux/netfilter_defs.h>
-
-struct netns_xt {
-	bool notrack_deprecated_warning;
-	bool clusterip_deprecated_warning;
-};
-#endif
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 12404d221026..0a913ce07425 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -351,21 +351,10 @@ notrack_tg(struct sk_buff *skb, const struct xt_action_param *par)
 	return XT_CONTINUE;
 }
 
-static int notrack_chk(const struct xt_tgchk_param *par)
-{
-	if (!par->net->xt.notrack_deprecated_warning) {
-		pr_info("netfilter: NOTRACK target is deprecated, "
-			"use CT instead or upgrade iptables\n");
-		par->net->xt.notrack_deprecated_warning = true;
-	}
-	return 0;
-}
-
 static struct xt_target notrack_tg_reg __read_mostly = {
 	.name		= "NOTRACK",
 	.revision	= 0,
 	.family		= NFPROTO_UNSPEC,
-	.checkentry	= notrack_chk,
 	.target		= notrack_tg,
 	.table		= "raw",
 	.me		= THIS_MODULE,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns
  2021-07-22  8:48 [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Florian Westphal
                   ` (2 preceding siblings ...)
  2021-07-22  8:48 ` [PATCH nf-next 3/3] netfilter: remove xt pernet data Florian Westphal
@ 2021-08-01  9:46 ` Pablo Neira Ayuso
  3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2021-08-01  9:46 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Thu, Jul 22, 2021 at 10:48:31AM +0200, Florian Westphal wrote:
> This series stops ipt_CLUSTERIP from registering arp mangling hook
> unconditionally.
> 
> Hook gets installed/removed from checkentry/destroy callbacks.
> 
> Before this, modprobe ipt_CLUSTERIP would add a hook in each netns.
> While at it, also get rid of x_tables.h/xt storage space in struct net,
> there is no need for this.

Series applied, thanks.

I made a small update to 2/3 here:

@@ -517,6 +531,19 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
                return ret;
        }

+       if (cn->hook_users == 0) {
+               ret = nf_register_net_hook(par->net, &cip_arp_ops);
+
+               if (ret < 0) {
+                       clusterip_config_entry_put(config); <--
+                       clusterip_config_put(config); <--

to use config instead cipinfo->config which is set on later.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-01  9:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22  8:48 [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Florian Westphal
2021-07-22  8:48 ` [PATCH nf-next 1/3] netfilter: ipt_CLUSTERIP: only add arp mangle hook when required Florian Westphal
2021-07-22  8:48 ` [PATCH nf-next 2/3] netfilter: ipt_CLUSTERIP: use clusterip_net to store pernet warning Florian Westphal
2021-07-22  8:48 ` [PATCH nf-next 3/3] netfilter: remove xt pernet data Florian Westphal
2021-08-01  9:46 ` [PATCH nf-next 0/3] netfilter: clusterip: don't register hook in all netns Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.