Make nicer CONFIG_NET_NS=n case code

Currently we have the NET_NS config option, but the only change it makes is just return ERR_PTR(-EINVAL) inside the cloning call thus introducing a bunch of a dead code and making the reference counting unneeded. This is not very good.

So clean the net_namespace.c to fix this.

I have sent a set of patches to Andrew to make similar thing for other namespaces, which introduces the NAMESPACES option to turn all the namespaces off at once (to make embedded people suffer less). So after that stuff is in, there will be some more patches to tie all this together.

What is to be done after this set is to make the register_pernet_xxx stuff smaller. Currently this code weights approximately 500 bytes, so it worths reducing it, but I haven't found a good solution yet.

Quoted 6 lines · On Wed, 31 Oct 2007 22:19:43 +0300 Pavel Emelyanov <[email protected]> wrote:

Currently we have the NET_NS config option, but the only change it makes is just return ERR_PTR(-EINVAL) inside the cloning call thus introducing a bunch of a dead code and making the reference counting
unneeded. This is not very good.

So clean the net_namespace.c to fix this.

Quoted 7 lines

I have sent a set of patches to Andrew to make similar thing for
other namespaces, which introduces the NAMESPACES option to turn
all the namespaces off at once (to make embedded people suffer
less). So after that stuff is in, there will be some more patches to tie all this together.

What is to be done after this set is to make the register_pernet_xxx
stuff smaller. Currently this code weights approximately 500 bytes, so it worths reducing it, but I haven't found a good solution yet.

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Maybe we can add a macro to get nd_net from a "struct net_device" so that every instance of

if (dev->nd_net != &init_net)

    goto drop;

can also be optimized away if !CONFIG_NET_NS

extern inline netdev_get_ns(struct netdevice *dev) {

#ifdef CONFIG_NET_NS
	return dev->nd_net;
#else
	return &init_net;

#endif
}

Currently we have the NET_NS config option, but the only change it makes is just return ERR_PTR(-EINVAL) inside the cloning call thus introducing a bunch of a dead code and making the reference counting
unneeded. This is not very good.

So clean the net_namespace.c to fix this.

I have sent a set of patches to Andrew to make similar thing for
other namespaces, which introduces the NAMESPACES option to turn
all the namespaces off at once (to make embedded people suffer
less). So after that stuff is in, there will be some more patches to tie all this together.

What is to be done after this set is to make the register_pernet_xxx
stuff smaller. Currently this code weights approximately 500 bytes, so it worths reducing it, but I haven't found a good solution yet.

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Maybe we can add a macro to get nd_net from a "struct net_device" so that every instance of

if (dev->nd_net != &init_net)

    goto drop;

can also be optimized away if !CONFIG_NET_NS

Quoted 2 lines · Eric Dumazet

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Really? Have you actually measured that? If the overhead is measurable and expensive we may want to look at per cpu counters or something like that. So far I don't have any numbers that say any of the network namespace work inherently has any overhead.

Quoted 8 lines

Maybe we can add a macro to get nd_net from a "struct net_device" so that every instance of

if (dev->nd_net != &init_net)

    goto drop;

can also be optimized away if !CONFIG_NET_NS

Well that extra check should be removed once we finish converting those code paths. So I'm not too worried.

If this becomes a big issue I can dig up my old code that replaced struct net * with a net_t typedef and used functions for all of the comparisons and allowed everything to be compiled away.

Trouble was it was sufficiently different that it was just enough different that people could not immediately understand the code.

Quoted 6 lines · Eric W. Biederman

Quoted 2 lines · Eric Dumazet

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Quoted 4 lines

Really? Have you actually measured that? If the overhead is measurable and expensive we may want to look at per cpu counters or something like that. So far I don't have any numbers that say any of the network namespace work inherently has any overhead.

It seems that on some old opterons (two 246 for example), "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(

Quoted 6 lines · Eric W. Biederman

Quoted 2 lines · Eric Dumazet

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Quoted 4 lines

Really? Have you actually measured that? If the overhead is measurable and expensive we may want to look at per cpu counters or something like that. So far I don't have any numbers that say any of the network namespace work inherently has any overhead.

It seems that on some old opterons (two 246 for example), "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(

I am not sure per cpu counters help : I tried this and got no speedup. (This was on net_device refcnt at that time)

(on this machines, the access through fs/gs selector seems expensive too)

Maybe a lazy mode could be done, ie only do a atomic_dec(), as done in dev_put() ?

Also, "count" sits in a cache line that contains mostly read and shared fields, you might want to put it in a separate cache line in SMP, to avoid cache line ping-pongs.

From: Eric Dumazet <[email protected]>
Date: Wed, 31 Oct 2007 23:40:59 +0100

Quoted 12 lines

Quoted 10 lines · Eric W. Biederman

Quoted 2 lines · Eric Dumazet

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Really? Have you actually measured that? If the overhead is measurable and expensive we may want to look at per cpu counters or something like that. So far I don't have any numbers that say any of the network namespace work inherently has any overhead.

Quoted 2 lines

It seems that on some old opterons (two 246 for example), "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(

P4 chips are generally very poor at mispredicted branches and atomics. So every atomic you remove from the socket paths gives a noticable improvement on them.

Network device reference counting is such a stupid problem. There has to be a way to get rid of it on the packet side.

I think we could get rid of all of the device refcounting from packets if we:

1) Formalize "SKB roots". This is every place a packet could sit in the transmit path.

2) On device unregister:

	a) wait for RCU quiesce period
	b) stop_machine_run(skb_walk_roots, netdev, NR_CPUS);

skb_walk_roots is a function that walks all the places in #1, rewriting the packet to point to loopback or whatever instead of 'netdev' which we are trying to unregister.

This gives us two things.

First, we no longer would need to rectount net devices for packet references.

Second, we have a debugging framework for all those dreaded SKB leaks that keep devices from being unloadable. As we walk the roots we'll see where all packets referencing a device actually are.

Quoted 4 lines · Eric Dumazet

Quoted 3 lines · Eric W. Biederman

Eric Dumazet <[email protected]> writes:

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Definitly wanted here. Thank you.
One more refcounting on each socket creation/deletion was expensive.

Really? Have you actually measured that? If the overhead is measurable and expensive we may want to look at per cpu counters or something like that. So far I don't have any numbers that say any of the network namespace work inherently has any overhead.

It seems that on some old opterons (two 246 for example), "if (atomic_dec_and_test(&net->count))" is rather expensive yes :(

I won't argue that atomic_dec_and_test is costly. My gut feel is that socket creation/destruction is sufficiently rare that such a test would be lost in the noise. Doing anything more sophisticated is likely to be less readable, and unless we can measure some overhead my preference right now is to keep the code stupid and simple. Which usually has a good icache footprint.

I agree with you that with current state, this atomic_inc/atomic_dec_and_test wont come in profiles unless a trivial bench is writen

for(;;){close(socket(....));}

When the new pernet something (subsys, device or operations) is being registered, the init callback is to be called for each namespace, that currently exitst in the system. During the unregister, the same is to be done with the exit callback.

However, not every pernet something has both calls, but the check for the appropriate pointer to be not NULL is performed inside the for_each_net() loop.

This is (at least) strange, so tune this.

Signed-off-by Pavel Emelyanov <[email protected]>

When the CONFIG_NET_NS is n there's no need in refcounting the initial net namespace. So relax this code by making a stupid stubs for the "n" case.

Signed-off-by Pavel Emelyanov <[email protected]>

The namespace creation/destruction code is never called if the CONFIG_NET_NS is n, so it's OK to move it under appropriate ifdef.

The copy_net_ns() in the "n" case checks for flags and returns -EINVAL when new net ns is requested. In a perfect world this stub must be in net_namespace.h, but this function need to know the CLONE_NEWNET value and thus requires sched.h. On the other hand this header is to be injected into almost every .c file in the networking code, and making all this code depend on the sched.h is a suicidal attempt.

Signed-off-by Pavel Emelyanov <[email protected]>

The setup_net is called for the init net namespace only (int the CONFIG_NET_NS=n of course) from the __init function, so mark it as __net_init to disappear with the caller after the boot.

Yet again, in the perfect world this has to be under #ifdef CONFIG_NET_NS, but it isn't guaranteed that every subsystem is registered *after* the init_net_ns is set up. After we are sure, that we don't start registering them before the init net setup, we'll be able to move this code under the ifdef.

Signed-off-by Pavel Emelyanov <[email protected]>

This cache is only required to create new namespaces, but we won't have them in CONFIG_NET_NS=n case.

Hide it under the appropriate ifdef.

Signed-off-by Pavel Emelyanov <[email protected]>