netdev - Re: [RFC PATCH 1/9] ipvs network name space aware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4CBC3182.80605@free.fr>
Date:	Mon, 18 Oct 2010 13:37:38 +0200
From:	Daniel Lezcano <daniel.lezcano@...e.fr>
To:	Hans Schillstrom <hans.schillstrom@...csson.com>
CC:	"lvs-devel@...r.kernel.org" <lvs-devel@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"netfilter-devel@...r.kernel.org" <netfilter-devel@...r.kernel.org>,
	"horms@...ge.net.au" <horms@...ge.net.au>, "ja@....bg" <ja@....bg>,
	"wensong@...ux-vs.org" <wensong@...ux-vs.org>
Subject: Re: [RFC PATCH 1/9] ipvs network name space aware

On 10/18/2010 11:54 AM, Hans Schillstrom wrote:
> On Monday 18 October 2010 10:59:25 Daniel Lezcano wrote:
>    
>> On 10/08/2010 01:16 PM, Hans Schillstrom wrote:
>>      
>>> This part contains the include files
>>> where include/net/netns/ip_vs.h is new and contains all moved vars.
>>>
>>> SUMMARY
>>>
>>>    include/net/ip_vs.h                     |  136 ++++---
>>>    include/net/net_namespace.h             |    2 +
>>>    include/net/netns/ip_vs.h               |  112 +++++
>>>
>>> Signed-off-by:Hans Schillstrom<hans.schillstrom@...csson.com>
>>> ---
>>>
>>>
>>>        
>> [ ... ]
>>
>>      
>>>    #ifdef CONFIG_IP_VS_IPV6
>>> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
>>> index bd10a79..b59cdc5 100644
>>> --- a/include/net/net_namespace.h
>>> +++ b/include/net/net_namespace.h
>>> @@ -15,6 +15,7 @@
>>>    #include<net/netns/ipv4.h>
>>>    #include<net/netns/ipv6.h>
>>>    #include<net/netns/dccp.h>
>>> +#include<net/netns/ip_vs.h>
>>>    #include<net/netns/x_tables.h>
>>>    #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
>>>    #include<net/netns/conntrack.h>
>>> @@ -91,6 +92,7 @@ struct net {
>>>    	struct sk_buff_head	wext_nlevents;
>>>    #endif
>>>    	struct net_generic	*gen;
>>> +	struct netns_ipvs       *ipvs;
>>>    };
>>>
>>>        
>> IMHO, it would be better to use the net_generic infra-structure instead
>> of adding a new field in the netns structure.
>>
>>
>>      
> I realized that to, but the performance penalty is quite high with net_generic :-(
> But on the other hand if you are going to backport it, (without recompiling the kernel)
> you gonna need it!
>    

Hmm, yes. We don't want to have the init_net_ns performances to be impacted.

You use here a pointer which will be dereferenced like the net_generic, 
I don't think there will be
a big difference between using net_generic and using a pointer in the 
net namespace structure.

The difference is the id usage, but this one is based on the idr which 
is quite fast.

We should experiment a bit here to compare both solutions.

IMHO, we can (1) create a non-pointer netns_ipvs field in the net 
namespace structure or (2) use a pointer [with net_generic].

(1) is the faster but with the drawback of having a bigger memory 
footprint even if the ipvs module is not loaded.
In this case we have to take care of what we store in the netns_ipvs 
structure, that is reduce the per namespace table and so. At the first 
glance, I think we can reduce this to the sysctls and a very few data, 
for example using global tables tagged with the namespace and we don't 
break the cacheline alignment optimization.

(2) is slower but as the memory is allocated and freed when the module 
is loaded/unloaded. What I don't like with this approach is we add some 
overhead even if the netns is not compiled in the kernel.

> My sugestion, take both with a configuration switch like:
> (i.e. avoid the rcu locking)
>
> --- net/ip_vs.h ---
> ...
> extern int ip_vs_net_id;		/* net id for ip_vs */
>
>
> static inline struct netns_ipvs * net_ipvs(struct net* net, int id) {
> #ifdef CONFIG_IP_VS_FAST_NETNS
> 	return net->ipvs;
> #else
> 	return (struct netns_ipvs *)net_generic(net, id);
> #endif
> }
> ...
>
> and where you need the netns_ipvs struct ptr,
> [snip]
> struct ip_vs_conn *ip_vs_conn_in_get(struct net *net, ....
> {
> 	struct netns_ipvs *ipvs = net_ipvs(net, ip_vs_net_id);
> ...
>    

It is a nice way to wrap both solutions but at this point I don't think 
it is worth to add a 3rd option to compile ipvs.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html