netns: struct net content re-work
Subsystems (3)
PATCH 1/1 netns: struct net content re-work
Recently David Miller and Herbert Xu pointed out that struct net becomes overbloated and un-maintainable. There are two solutions:
- provide a pointer to a network subsystem definition from struct net. This costs an additional dereferrence
- place sub-system definition into the structure itself. This will speedup run-time access at the cost of recompilation time
The second approach looks better for us. Other sub-systems will be converted to this approach if this will be accepted :)
---
include/net/net_namespace.h +3 -2
@@ -8,6 +8,8 @@#include <linux/workqueue.h> #include <linux/list.h> +#include <net/netns/unix.h> +struct proc_dir_entry ;struct net_device ;struct sock ;
@@ -46,8 +48,7 @@ struct net {struct hlist_head packet_sklist;/* unix sockets */ -int sysctl_unix_max_dgram_qlen; -struct ctl_table_header *unix_ctl; +struct netns_unix unx; };#ifdef CONFIG_NET
include/net/netns/unix.h new file +13 -0
@@ -0,0 +1,13 @@ +/* +* Unix network namespace +*/ +#ifndef __NETNS_UNIX_H__ +#define __NETNS_UNIX_H__ + +struct ctl_table_header ; +struct netns_unix { +int sysctl_unix_max_dgram_qlen; +struct ctl_table_header *unix_ctl; +}; + +#endif /* __NETNS_UNIX_H__ */
net/unix/af_unix.c +2 -2
@@ -592,7 +592,7 @@ static struct sock * unix_create1(struct net *net, struct socket *sock) &af_unix_sk_receive_queue_lock_key); sk->sk_write_space = unix_write_space; - sk->sk_max_ack_backlog = net->sysctl_unix_max_dgram_qlen; + sk->sk_max_ack_backlog = net->unx.sysctl_unix_max_dgram_qlen; sk->sk_destruct = unix_sock_destructor; u =unix_sk (sk); u->dentry =NULL ;
@@ -2138,7 +2138,7 @@ static int unix_net_init(struct net *net) {int error = -ENOMEM ; - net->sysctl_unix_max_dgram_qlen =10 ; + net->unx.sysctl_unix_max_dgram_qlen =10 ;if (unix_sysctl_register (net)) goto out;
net/unix/sysctl_net_unix.c +6 -6
@@ -18,7 +18,7 @@ static ctl_table unix_table[] = { { .ctl_name = NET_UNIX_MAX_DGRAM_QLEN, .procname = "max_dgram_qlen", - .data = &init_net.sysctl_unix_max_dgram_qlen, + .data = &init_net.unx.sysctl_unix_max_dgram_qlen, .maxlen = sizeof(int), .mode =0644 , .proc_handler = &proc_dointvec
@@ -40,9 +40,9 @@ int unix_sysctl_register(struct net *net)if (table ==NULL ) goto err_alloc; - table[0 ].data = &net->sysctl_unix_max_dgram_qlen; - net->unix_ctl =register_net_sysctl_table (net, unix_path, table); -if (net->unix_ctl ==NULL ) + table[0 ].data = &net->unx.sysctl_unix_max_dgram_qlen; + net->unx.unix_ctl =register_net_sysctl_table (net, unix_path, table); +if (net->unx.unix_ctl ==NULL ) goto err_reg;return 0 ;
@@ -57,8 +57,8 @@ void unix_sysctl_unregister(struct net *net) {struct ctl_table *table; - table = net->unix_ctl->ctl_table_arg; -unregister_sysctl_table (net->unix_ctl); + table = net->unx.unix_ctl->ctl_table_arg; +unregister_sysctl_table (net->unx.unix_ctl);kfree (table); }
Quoted 8 lines · Denis V. Lunev
Recently David Miller and Herbert Xu pointed out that struct net becomes overbloated and un-maintainable. There are two solutions:
- provide a pointer to a network subsystem definition from struct net. This costs an additional dereferrence
- place sub-system definition into the structure itself. This will speedup run-time access at the cost of recompilation timeThe second approach looks better for us.
Yes, we do not need/want a pointer in this structure and add more dereference in the network code.
Quoted 19 lines
Other sub-systems will be converted
to this approach if this will be accepted :)Signed-off-by Denis V. Lunev <[email protected]>---
include/net/net_namespace.h +2
-0@@ -8,6 +8,8 @@#include <linux/workqueue.h> #include <linux/list.h> +#include <net/netns/unix.h> +struct proc_dir_entry ;struct net_device ;struct sock ;
include/net/net_namespace.h struct net { · line 51
Quoted 6 lines
include/net/net_namespace.h +1
-2@@ -46,8 +48,7 @@ struct net {struct hlist_head packet_sklist;/* unix sockets */ -int sysctl_unix_max_dgram_qlen; -struct ctl_table_header *unix_ctl; +struct netns_unix unx;
Can you change this from unx to unix ?
If you encapsulate the structure definitions per subsystem, you can drop the unix prefix in the variable declaration.
Instead of having:
netns->unix->unix_ctl you will have: netns->unix->ctl
Quoted 12 lines
include/net/net_namespace.h +0
-0};#ifdef CONFIG_NET include/net/netns/unix.h new file +3
-0@@ -0,0 +1,13 @@ +/* +* Unix network namespace +*/
include/net/netns/unix.h line 13
Quoted 10 lines
include/net/netns/unix.h new file +10
-0+#ifndef __NETNS_UNIX_H__ +#define __NETNS_UNIX_H__ + +struct ctl_table_header ; +struct netns_unix { +int sysctl_unix_max_dgram_qlen; +struct ctl_table_header *unix_ctl; +}; + +#endif /* __NETNS_UNIX_H__ */
If I follow the logic, we will have a file per subsystem. These files will be very small, no ? IMHO, having the structure defined in net_namespace.h is enough.
Quoted 64 lines
net/unix/af_unix.c +2
-2@@ -592,7 +592,7 @@ static struct sock * unix_create1(struct net *net, struct socket *sock) &af_unix_sk_receive_queue_lock_key); sk->sk_write_space = unix_write_space; - sk->sk_max_ack_backlog = net->sysctl_unix_max_dgram_qlen; + sk->sk_max_ack_backlog = net->unx.sysctl_unix_max_dgram_qlen; sk->sk_destruct = unix_sock_destructor; u =unix_sk (sk); u->dentry =NULL ;@@ -2138,7 +2138,7 @@ static int unix_net_init(struct net *net) {int error = -ENOMEM ; - net->sysctl_unix_max_dgram_qlen =10 ; + net->unx.sysctl_unix_max_dgram_qlen =10 ;if (unix_sysctl_register (net)) goto out;net/unix/sysctl_net_unix.c +6
-7@@ -18,7 +18,7 @@ static ctl_table unix_table[] = { { .ctl_name = NET_UNIX_MAX_DGRAM_QLEN, .procname = "max_dgram_qlen", - .data = &init_net.sysctl_unix_max_dgram_qlen, + .data = &init_net.unx.sysctl_unix_max_dgram_qlen, .maxlen = sizeof(int), .mode =0644 , .proc_handler = &proc_dointvec@@ -40,9 +40,9 @@ int unix_sysctl_register(struct net *net)if (table ==NULL ) goto err_alloc; - table[0 ].data = &net->sysctl_unix_max_dgram_qlen; - net->unix_ctl =register_net_sysctl_table (net, unix_path, table); -if (net->unix_ctl ==NULL ) + table[0 ].data = &net->unx.sysctl_unix_max_dgram_qlen; + net->unx.unix_ctl =register_net_sysctl_table (net, unix_path, table); +if (net->unx.unix_ctl ==NULL ) goto err_reg;return 0 ;@@ -57,8 +57,8 @@ void unix_sysctl_unregister(struct net *net) {struct ctl_table *table; - table = net->unix_ctl->ctl_table_arg; -unregister_sysctl_table (net->unix_ctl); + table = net->unx.unix_ctl->ctl_table_arg; +unregister_sysctl_table (net->unx.unix_ctl);kfree (table); } --To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 37 lines · Daniel Lezcano
Quoted 8 lines · Denis V. Lunev
Recently David Miller and Herbert Xu pointed out that struct net becomes overbloated and un-maintainable. There are two solutions:
- provide a pointer to a network subsystem definition from struct net. This costs an additional dereferrence
- place sub-system definition into the structure itself. This will speedup run-time access at the cost of recompilation timeThe second approach looks better for us.
Yes, we do not need/want a pointer in this structure and add more dereference in the network code.
Quoted 21 lines
Other sub-systems will be converted
to this approach if this will be accepted :)Signed-off-by Denis V. Lunev <[email protected]>---
include/net/net_namespace.h +2
-0@@ -8,6 +8,8 @@#include <linux/workqueue.h> #include <linux/list.h> +#include <net/netns/unix.h> +struct proc_dir_entry ;struct net_device ;struct sock ;@@ -46,8 +48,7 @@ struct net {struct hlist_head packet_sklist;
Quoted 7 lines
include/net/net_namespace.h struct net { · line 51
Quoted 4 lines
include/net/net_namespace.h +1
-2/* unix sockets */ -int sysctl_unix_max_dgram_qlen; -struct ctl_table_header *unix_ctl; +struct netns_unix unx;Can you change this from unx to unix ?
no, it won't compile. Guess why :)
Quoted 7 lines
If you encapsulate the structure definitions per subsystem, you can drop the unix prefix in the variable declaration.
Instead of having:
netns->unix->unix_ctl you will have: netns->unix->ctl
agree.
Kirill
Quoted 10 lines · Kirill Korotaev
Quoted 9 lines · Daniel Lezcano
Quoted 7 lines · Denis V. Lunev
Recently David Miller and Herbert Xu pointed out that struct net becomes overbloated and un-maintainable. There are two solutions:
- provide a pointer to a network subsystem definition from struct net. This costs an additional dereferrence
- place sub-system definition into the structure itself. This will speedup run-time access at the cost of recompilation time
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
The second approach looks better for us.
Yes, we do not need/want a pointer in this structure and add more dereference in the network code.
If it does go that way we just carefully pass around a properly typed structure in that subsystem to reduce the cost. Still it would be nice not to need to add the extra pointer.
Quoted 23 lines
Quoted 15 lines
Quoted 15 lines
index b62e31f..f60e1ce 100644
include/net/net_namespace.h +2
-0@@ -8,6 +8,8 @@#include <linux/workqueue.h> #include <linux/list.h> +#include <net/netns/unix.h> +struct proc_dir_entry ;struct net_device ;struct sock ;@@ -46,8 +48,7 @@ struct net {struct hlist_head packet_sklist;Quoted 7 lines
include/net/net_namespace.h struct net { · line 51
Quoted 4 lines
include/net/net_namespace.h +1
-2/* unix sockets */ -int sysctl_unix_max_dgram_qlen; -struct ctl_table_header *unix_ctl; +struct netns_unix unx;Can you change this from unx to unix ?
Quoted 1 line
no, it won't compile. Guess why :)
Hmm. It looks like it is a #define somewhere gcc? Eric
David Miller
maintainerFrom: [email protected] (Eric W. Biederman)
Date: Mon, 10 Dec 2007 21:04:07 -0700
Quoted 9 lines
Quoted 5 lines · Kirill Korotaev
Quoted 2 lines · Daniel Lezcano
Denis V. Lunev wrote:
Can you change this from unx to unix ?no, it won't compile. Guess why :)
Hmm. It looks like it is a #define somewhere gcc?
It is a platform CPP pre-define for UNIX platforms.
The idea of separate structures make sense, and seems needed and useful.
include/net/netns/unix.h
Quoted 5 lines · Denis V. Lunev
include/net/netns/unix.h new file +0
-0
^^^^^^
Given that we are making this per protocol adding a separate directory to hold them seems to be the wrong grouping. Ideally we want everything for the protocol all together in the same location so it is easy to find. Possibly with a user/kernel split.
So perhaps unix_net.h
include/net/netns/unix.h line 11
Quoted 12 lines
include/net/netns/unix.h new file +11
-0@@ -0,0 +1,13 @@ +/* +* Unix network namespace +*/ +#ifndef __NETNS_UNIX_H__ +#define __NETNS_UNIX_H__ + +struct ctl_table_header ; +struct netns_unix { +int sysctl_unix_max_dgram_qlen; +struct ctl_table_header *unix_ctl; +};
How about struct unix_net? I think that tracks a little better with how we have done struct in_device, ip6_dev and their friends.
Eric
A message that is not in the archive (sent off-list or not yet received): m1prxd28h3.fsf-T1Yj925okcoyDheHMi7gv2pdwda3JcWeAL8bYrjMMd8@public.gmane.org
Quoted 10 lines · Eric W. Biederman
The idea of separate structures make sense, and seems needed and useful.
include/net/netns/unix.h
Quoted 5 lines · Denis V. Lunev
include/net/netns/unix.h new file +0
-0^^^^^^
Quoted 6 lines
Given that we are making this per protocol adding a separate directory to hold them seems to be the wrong grouping. Ideally we want everything for the protocol all together in the same location so it is easy to find. Possibly with a user/kernel split.
So perhaps
unix_net.h
The idea was simple:
- I can name 5 files right now
- I want them to be shown to gather by ls
- so, there are 2 ways, namely:
# include/net/netns/unix.h # include/net/netns-unix.h
Regards,
Den
Quoted 19 lines
include/net/netns/unix.h line 11
Quoted 13 lines
include/net/netns/unix.h new file +11
-0@@ -0,0 +1,13 @@ +/* +* Unix network namespace +*/ +#ifndef __NETNS_UNIX_H__ +#define __NETNS_UNIX_H__ + +struct ctl_table_header ; +struct netns_unix { +int sysctl_unix_max_dgram_qlen; +struct ctl_table_header *unix_ctl; +};How about struct
unix_net? I think that tracks a little better with how we have done structin_device,ip6_devand their friends.Eric
David Miller
maintainerFrom: "Denis V. Lunev" <[email protected]>
Date: Tue, 11 Dec 2007 10:33:45 +0300
Quoted 16 lines
Quoted 15 lines · Eric W. Biederman
The idea of separate structures make sense, and seems needed and useful.
include/net/netns/unix.h
Quoted 5 lines · Denis V. Lunev
include/net/netns/unix.h new file +0
-0^^^^^^Given that we are making this per protocol adding a separate directory to hold them seems to be the wrong grouping. Ideally we want everything for the protocol all together in the same location so it is easy to find. Possibly with a user/kernel split.
Quoted 7 lines
Quoted 1 line
So perhaps
unix_net.hThe idea was simple:
- I can name 5 files right now
- I want them to be shown to gather by ls
- so, there are 2 ways, namely:# include/net/netns/unix.h # include/net/netns-unix.h
I have no real objection to the new directory.