[PATCH 1/2] nfnetlink: add netns support

[PATCH 1/2] nfnetlink: add netns support

[Alexey Dobriyan]

Make nfnetlink socket per-netns.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 include/linux/netfilter/nfnetlink.h  |    8 ++--
 include/net/net_namespace.h          |    2 +
 net/netfilter/nf_conntrack_netlink.c |   13 +++----
 net/netfilter/nfnetlink.c            |   65 +++++++++++++++++++++++------------
 net/netfilter/nfnetlink_log.c        |    2 -
 net/netfilter/nfnetlink_queue.c      |    2 -
 6 files changed, 58 insertions(+), 34 deletions(-)

--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -73,11 +73,11 @@ struct nfnetlink_subsystem {
 extern int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n);
 extern int nfnetlink_subsys_unregister(const struct nfnetlink_subsystem *n);
 
-extern int nfnetlink_has_listeners(unsigned int group);
-extern int nfnetlink_send(struct sk_buff *skb, u32 pid, unsigned group, 
+extern int nfnetlink_has_listeners(struct net *net, unsigned int group);
+extern int nfnetlink_send(struct sk_buff *skb, struct net *net, u32 pid, unsigned group,
 			  int echo, gfp_t flags);
-extern void nfnetlink_set_err(u32 pid, u32 group, int error);
-extern int nfnetlink_unicast(struct sk_buff *skb, u_int32_t pid, int flags);
+extern void nfnetlink_set_err(struct net *net, u32 pid, u32 group, int error);
+extern int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u_int32_t pid, int flags);
 
 extern void nfnl_lock(void);
 extern void nfnl_unlock(void);
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -81,6 +81,8 @@ struct net {
 #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
 	struct netns_ct		ct;
 #endif
+	struct sock		*nfnl;
+	struct sock		*nfnl_stash;
 #endif
 #ifdef CONFIG_XFRM
 	struct netns_xfrm	xfrm;
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -482,7 +482,7 @@ ctnetlink_conntrack_event(unsigned int events, struct nf_ct_event *item)
 	} else
 		return 0;
 
-	if (!item->report && !nfnetlink_has_listeners(group))
+	if (!item->report && !nfnetlink_has_listeners(&init_net, group))
 		return 0;
 
 	skb = nlmsg_new(ctnetlink_nlmsg_size(ct), GFP_ATOMIC);
@@ -559,7 +559,8 @@ ctnetlink_conntrack_event(unsigned int events, struct nf_ct_event *item)
 	rcu_read_unlock();
 
 	nlmsg_end(skb, nlh);
-	err = nfnetlink_send(skb, item->pid, group, item->report, GFP_ATOMIC);
+	err = nfnetlink_send(skb, &init_net, item->pid, group, item->report,
+			     GFP_ATOMIC);
 	if (err == -ENOBUFS || err == -EAGAIN)
 		return -ENOBUFS;
 
@@ -571,7 +572,7 @@ nla_put_failure:
 nlmsg_failure:
 	kfree_skb(skb);
 errout:
-	nfnetlink_set_err(0, group, -ENOBUFS);
+	nfnetlink_set_err(&init_net, 0, group, -ENOBUFS);
 	return 0;
 }
 #endif /* CONFIG_NF_CONNTRACK_EVENTS */
@@ -1539,7 +1540,7 @@ ctnetlink_expect_event(unsigned int events, struct nf_exp_event *item)
 		return 0;
 
 	if (!item->report &&
-	    !nfnetlink_has_listeners(NFNLGRP_CONNTRACK_EXP_NEW))
+	    !nfnetlink_has_listeners(&init_net, NFNLGRP_CONNTRACK_EXP_NEW))
 		return 0;
 
 	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
@@ -1562,7 +1563,7 @@ ctnetlink_expect_event(unsigned int events, struct nf_exp_event *item)
 	rcu_read_unlock();
 
 	nlmsg_end(skb, nlh);
-	nfnetlink_send(skb, item->pid, NFNLGRP_CONNTRACK_EXP_NEW,
+	nfnetlink_send(skb, &init_net, item->pid, NFNLGRP_CONNTRACK_EXP_NEW,
 		       item->report, GFP_ATOMIC);
 	return 0;
 
@@ -1572,7 +1573,7 @@ nla_put_failure:
 nlmsg_failure:
 	kfree_skb(skb);
 errout:
-	nfnetlink_set_err(0, 0, -ENOBUFS);
+	nfnetlink_set_err(&init_net, 0, 0, -ENOBUFS);
 	return 0;
 }
 #endif
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -40,7 +40,6 @@ MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_NETFILTER);
 
 static char __initdata nfversion[] = "0.30";
 
-static struct sock *nfnl = NULL;
 static const struct nfnetlink_subsystem *subsys_table[NFNL_SUBSYS_COUNT];
 static DEFINE_MUTEX(nfnl_mutex);
 
@@ -101,34 +100,35 @@ nfnetlink_find_client(u_int16_t type, const struct nfnetlink_subsystem *ss)
 	return &ss->cb[cb_id];
 }
 
-int nfnetlink_has_listeners(unsigned int group)
+int nfnetlink_has_listeners(struct net *net, unsigned int group)
 {
-	return netlink_has_listeners(nfnl, group);
+	return netlink_has_listeners(net->nfnl, group);
 }
 EXPORT_SYMBOL_GPL(nfnetlink_has_listeners);
 
-int nfnetlink_send(struct sk_buff *skb, u32 pid,
+int nfnetlink_send(struct sk_buff *skb, struct net *net, u32 pid,
 		   unsigned group, int echo, gfp_t flags)
 {
-	return nlmsg_notify(nfnl, skb, pid, group, echo, flags);
+	return nlmsg_notify(net->nfnl, skb, pid, group, echo, flags);
 }
 EXPORT_SYMBOL_GPL(nfnetlink_send);
 
-void nfnetlink_set_err(u32 pid, u32 group, int error)
+void nfnetlink_set_err(struct net *net, u32 pid, u32 group, int error)
 {
-	netlink_set_err(nfnl, pid, group, error);
+	netlink_set_err(net->nfnl, pid, group, error);
 }
 EXPORT_SYMBOL_GPL(nfnetlink_set_err);
 
-int nfnetlink_unicast(struct sk_buff *skb, u_int32_t pid, int flags)
+int nfnetlink_unicast(struct sk_buff *skb, struct net *net, u_int32_t pid, int flags)
 {
-	return netlink_unicast(nfnl, skb, pid, flags);
+	return netlink_unicast(net->nfnl, skb, pid, flags);
 }
 EXPORT_SYMBOL_GPL(nfnetlink_unicast);
 
 /* Process one complete nfnetlink message. */
 static int nfnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
+	struct net *net = sock_net(skb->sk);
 	const struct nfnl_callback *nc;
 	const struct nfnetlink_subsystem *ss;
 	int type, err;
@@ -170,7 +170,7 @@ replay:
 		if (err < 0)
 			return err;
 
-		err = nc->call(nfnl, skb, nlh, (const struct nlattr **)cda);
+		err = nc->call(net->nfnl, skb, nlh, (const struct nlattr **)cda);
 		if (err == -EAGAIN)
 			goto replay;
 		return err;
@@ -184,26 +184,47 @@ static void nfnetlink_rcv(struct sk_buff *skb)
 	nfnl_unlock();
 }
 
-static void __exit nfnetlink_exit(void)
+static int __net_init nfnetlink_net_init(struct net *net)
 {
-	printk("Removing netfilter NETLINK layer.\n");
-	netlink_kernel_release(nfnl);
-	return;
+	struct sock *nfnl;
+
+	nfnl = netlink_kernel_create(net, NETLINK_NETFILTER, NFNLGRP_MAX,
+				     nfnetlink_rcv, NULL, THIS_MODULE);
+	if (!nfnl)
+		return -ENOMEM;
+	net->nfnl_stash = nfnl;
+	rcu_assign_pointer(net->nfnl, nfnl);
+	return 0;
 }
 
-static int __init nfnetlink_init(void)
+static void __net_exit nfnetlink_net_exit_batch(struct list_head *net_exit_list)
 {
-	printk("Netfilter messages via NETLINK v%s.\n", nfversion);
+	struct net *net;
 
-	nfnl = netlink_kernel_create(&init_net, NETLINK_NETFILTER, NFNLGRP_MAX,
-				     nfnetlink_rcv, NULL, THIS_MODULE);
-	if (!nfnl) {
-		printk(KERN_ERR "cannot initialize nfnetlink!\n");
-		return -ENOMEM;
+	list_for_each_entry(net, net_exit_list, exit_list) {
+		rcu_assign_pointer(net->nfnl, NULL);
+	}
+	synchronize_net();
+	list_for_each_entry(net, net_exit_list, exit_list) {
+		netlink_kernel_release(net->nfnl_stash);
 	}
+}
 
-	return 0;
+static struct pernet_operations nfnetlink_net_ops = {
+	.init		= nfnetlink_net_init,
+	.exit_batch	= nfnetlink_net_exit_batch,
+};
+
+static int __init nfnetlink_init(void)
+{
+	printk("Netfilter messages via NETLINK v%s.\n", nfversion);
+	return register_pernet_subsys(&nfnetlink_net_ops);
 }
 
+static void __exit nfnetlink_exit(void)
+{
+	printk("Removing netfilter NETLINK layer.\n");
+	unregister_pernet_subsys(&nfnetlink_net_ops);
+}
 module_init(nfnetlink_init);
 module_exit(nfnetlink_exit);
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -323,7 +323,7 @@ __nfulnl_send(struct nfulnl_instance *inst)
 			  NLMSG_DONE,
 			  sizeof(struct nfgenmsg));
 
-	status = nfnetlink_unicast(inst->skb, inst->peer_pid, MSG_DONTWAIT);
+	status = nfnetlink_unicast(inst->skb, &init_net, inst->peer_pid, MSG_DONTWAIT);
 
 	inst->qlen = 0;
 	inst->skb = NULL;
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -420,7 +420,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
 	}
 
 	/* nfnetlink_unicast will either free the nskb or add it to a socket */
-	err = nfnetlink_unicast(nskb, queue->peer_pid, MSG_DONTWAIT);
+	err = nfnetlink_unicast(nskb, &init_net, queue->peer_pid, MSG_DONTWAIT);
 	if (err < 0) {
 		queue->queue_user_dropped++;
 		goto err_out_unlock;

Re: [PATCH 1/2] nfnetlink: add netns support

[Patrick McHardy]

Alexey Dobriyan wrote:
> Make nfnetlink socket per-netns.
> 
> --- a/include/net/net_namespace.h
> +++ b/include/net/net_namespace.h
> @@ -81,6 +81,8 @@ struct net {
>  #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
>  	struct netns_ct		ct;
>  #endif
> +	struct sock		*nfnl;
> +	struct sock		*nfnl_stash;

Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
(_MODULE) ?

>  #endif

> +static void __net_exit nfnetlink_net_exit_batch(struct list_head *net_exit_list)
>  {
> -	printk("Netfilter messages via NETLINK v%s.\n", nfversion);
> +	struct net *net;
>  
> -	nfnl = netlink_kernel_create(&init_net, NETLINK_NETFILTER, NFNLGRP_MAX,
> -				     nfnetlink_rcv, NULL, THIS_MODULE);
> -	if (!nfnl) {
> -		printk(KERN_ERR "cannot initialize nfnetlink!\n");
> -		return -ENOMEM;
> +	list_for_each_entry(net, net_exit_list, exit_list) {
> +		rcu_assign_pointer(net->nfnl, NULL);
> +	}
> +	synchronize_net();
> +	list_for_each_entry(net, net_exit_list, exit_list) {
> +		netlink_kernel_release(net->nfnl_stash);
>  	}

No parenthesis around single statements please.

> --- a/net/netfilter/nfnetlink_log.c
> +++ b/net/netfilter/nfnetlink_log.c
> @@ -323,7 +323,7 @@ __nfulnl_send(struct nfulnl_instance *inst)
>  			  NLMSG_DONE,
>  			  sizeof(struct nfgenmsg));
>  
> -	status = nfnetlink_unicast(inst->skb, inst->peer_pid, MSG_DONTWAIT);
> +	status = nfnetlink_unicast(inst->skb, &init_net, inst->peer_pid, MSG_DONTWAIT);

Overly long line.

The remaining parts look good to me.

Re: [PATCH 1/2] nfnetlink: add netns support

[Alexey Dobriyan]

On Wed, Jan 13, 2010 at 02:02:33PM +0100, Patrick McHardy wrote:
> Alexey Dobriyan wrote:

> > +	struct sock		*nfnl;
> > +	struct sock		*nfnl_stash;
> 
> Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
> (_MODULE) ?

No, otherwise you won't be able to start using, e. g., nfnetlink_log
module on NETFILTER_NETLINK=n kernels without reboot.

> The remaining parts look good to me.

OK, I'll resend shortly.

Re: [PATCH 1/2] nfnetlink: add netns support

[Patrick McHardy]

Alexey Dobriyan wrote:
> On Wed, Jan 13, 2010 at 02:02:33PM +0100, Patrick McHardy wrote:
>> Alexey Dobriyan wrote:
> 
>>> +	struct sock		*nfnl;
>>> +	struct sock		*nfnl_stash;
>> Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
>> (_MODULE) ?
> 
> No, otherwise you won't be able to start using, e. g., nfnetlink_log
> module on NETFILTER_NETLINK=n kernels without reboot.

You mean after enabling CONFIG_NETFILTER_NETLINK? That seems like a
pretty poor argument, you have to do a reboot after changing a lot
of config options.

>> The remaining parts look good to me.
> 
> OK, I'll resend shortly.

Thanks.

Re: [PATCH 1/2] nfnetlink: add netns support

[Alexey Dobriyan]

On Wed, Jan 13, 2010 at 02:27:08PM +0100, Patrick McHardy wrote:
> Alexey Dobriyan wrote:
> > On Wed, Jan 13, 2010 at 02:02:33PM +0100, Patrick McHardy wrote:
> >> Alexey Dobriyan wrote:
> > 
> >>> +	struct sock		*nfnl;
> >>> +	struct sock		*nfnl_stash;
> >> Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
> >> (_MODULE) ?
> > 
> > No, otherwise you won't be able to start using, e. g., nfnetlink_log
> > module on NETFILTER_NETLINK=n kernels without reboot.
> 
> You mean after enabling CONFIG_NETFILTER_NETLINK?

Yes, and it can't be directly enabled, because it's hidden option.

> That seems like a pretty poor argument, you have to do a reboot after
> changing a lot of config options.

It's still a step back if I add #ifdef and a regression if you ask me.
The price is only two pointers per-netns.

Re: [PATCH 1/2] nfnetlink: add netns support

[Patrick McHardy]

Alexey Dobriyan wrote:
> On Wed, Jan 13, 2010 at 02:27:08PM +0100, Patrick McHardy wrote:
>> Alexey Dobriyan wrote:
>>> On Wed, Jan 13, 2010 at 02:02:33PM +0100, Patrick McHardy wrote:
>>>> Alexey Dobriyan wrote:
>>>>> +	struct sock		*nfnl;
>>>>> +	struct sock		*nfnl_stash;
>>>> Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
>>>> (_MODULE) ?
>>> No, otherwise you won't be able to start using, e. g., nfnetlink_log
>>> module on NETFILTER_NETLINK=n kernels without reboot.
>> You mean after enabling CONFIG_NETFILTER_NETLINK?
> 
> Yes, and it can't be directly enabled, because it's hidden option.
> 
>> That seems like a pretty poor argument, you have to do a reboot after
>> changing a lot of config options.
> 
> It's still a step back if I add #ifdef and a regression if you ask me.
> The price is only two pointers per-netns.

Yes, I don't mind much, but I still think this is not an argument
at all. Whether you have to reboot after changing your configuration
has never been "stable" and we don't consider it when making changes.

Re: [PATCH 1/2] nfnetlink: add netns support

[Alexey Dobriyan]

On Wed, Jan 13, 2010 at 02:42:00PM +0100, Patrick McHardy wrote:
> Alexey Dobriyan wrote:
> > On Wed, Jan 13, 2010 at 02:27:08PM +0100, Patrick McHardy wrote:
> >> Alexey Dobriyan wrote:
> >>> On Wed, Jan 13, 2010 at 02:02:33PM +0100, Patrick McHardy wrote:
> >>>> Alexey Dobriyan wrote:
> >>>>> +	struct sock		*nfnl;
> >>>>> +	struct sock		*nfnl_stash;
> >>>> Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
> >>>> (_MODULE) ?
> >>> No, otherwise you won't be able to start using, e. g., nfnetlink_log
> >>> module on NETFILTER_NETLINK=n kernels without reboot.
> >> You mean after enabling CONFIG_NETFILTER_NETLINK?
> > 
> > Yes, and it can't be directly enabled, because it's hidden option.
> > 
> >> That seems like a pretty poor argument, you have to do a reboot after
> >> changing a lot of config options.
> > 
> > It's still a step back if I add #ifdef and a regression if you ask me.
> > The price is only two pointers per-netns.
> 
> Yes, I don't mind much, but I still think this is not an argument
> at all. Whether you have to reboot after changing your configuration
> has never been "stable" and we don't consider it when making changes.

Perhaps we should start considering it.

What's bool is bool and fair game.
What's tristate better be fully tristate.

Re: [PATCH 1/2] nfnetlink: add netns support

[Patrick McHardy]

Alexey Dobriyan wrote:
> On Wed, Jan 13, 2010 at 02:42:00PM +0100, Patrick McHardy wrote:
>> Alexey Dobriyan wrote:
>>> On Wed, Jan 13, 2010 at 02:27:08PM +0100, Patrick McHardy wrote:
>>>> Alexey Dobriyan wrote:
>>>>> On Wed, Jan 13, 2010 at 02:02:33PM +0100, Patrick McHardy wrote:
>>>>>> Alexey Dobriyan wrote:
>>>>>>> +	struct sock		*nfnl;
>>>>>>> +	struct sock		*nfnl_stash;
>>>>>> Shouldn't this be contained in an ifdef(CONFIG_NETFILTER_NETLINK)
>>>>>> (_MODULE) ?
>>>>> No, otherwise you won't be able to start using, e. g., nfnetlink_log
>>>>> module on NETFILTER_NETLINK=n kernels without reboot.
>>>> You mean after enabling CONFIG_NETFILTER_NETLINK?
>>> Yes, and it can't be directly enabled, because it's hidden option.
>>>
>>>> That seems like a pretty poor argument, you have to do a reboot after
>>>> changing a lot of config options.
>>> It's still a step back if I add #ifdef and a regression if you ask me.
>>> The price is only two pointers per-netns.
>> Yes, I don't mind much, but I still think this is not an argument
>> at all. Whether you have to reboot after changing your configuration
>> has never been "stable" and we don't consider it when making changes.
> 
> Perhaps we should start considering it.
> 
> What's bool is bool and fair game.
> What's tristate better be fully tristate.

That's silly. Any configuration option is allowed to change
data structure layout, at which point you'll often need to
reboot.