Re: [RFC PATCH 0/9] ipvs network name space (netns) aware

To: Hans Schillstrom <hans.schillstrom@xxxxxxxxxxxx>
Subject: Re: [RFC PATCH 0/9] ipvs network name space (netns) aware
Cc: Simon Horman <horms@xxxxxxxxxxxx>, "lvs-devel@xxxxxxxxxxxxxxx" <lvs-devel@xxxxxxxxxxxxxxx>, "netdev@xxxxxxxxxxxxxxx" <netdev@xxxxxxxxxxxxxxx>, "netfilter-devel@xxxxxxxxxxxxxxx" <netfilter-devel@xxxxxxxxxxxxxxx>, "ja@xxxxxx" <ja@xxxxxx>, "wensong@xxxxxxxxxxxx" <wensong@xxxxxxxxxxxx>
From: Daniel Lezcano <daniel.lezcano@xxxxxxx>
Date: Mon, 18 Oct 2010 14:16:38 +0200
On 10/18/2010 01:55 PM, Hans Schillstrom wrote:
On Sunday 17 October 2010 08:47:31 Simon Horman wrote:
On Fri, Oct 08, 2010 at 01:16:36PM +0200, Hans Schillstrom wrote:
This patch series adds network name space (netns) support to the LVS.


This is version 1


The patch doesn't remove or add any functionality except for netns.
For users that don't use network name space (netns) this patch is
completely transparent.

No it's possible to run LVS in a Linux container (see lxc-tools)
i.e.  a light weight virtualization. For example it's possible to run
one or several lvs on a real server in their own network name spaces.
> From the LVS point of view it looks like it runs on it's own machine.

Basic requirements for netns awareness
  - Global variables has to be moved to dyn. allocated memory.

Most global variables now resides in a struct ipvs { } in netns/ip_vs.h.
What is moved and what is not ?

Some cache aligned locks are still in global, module init params and some 

Algorithm files they are untouched.

Drop rate in ip_vs_ctl per netns or grand total ?
This is a tricky one (I think),
if the interface is shared with root name-space and/or other name-spaces
  - use grand total
if it's an "own interface"
  - drop rate can/should be in netns...

My gut-feeling is that per netns makes more sense.

Should more lock variables be moved (or less) ?
I'm unsure what you are asking here but I will make a general statement
about locking in IPVS: it needs work.
Some locks still resides as global variables, and others in netns_ipvs struct.
Since you have a lot of experience with IPVS locks,
you might have ideas what to move and what to not move.

This patch set is based upon net-next-2.6 (2.6.36-rc3) from 4 oct 2010
and [patch v4] ipvs: IPv6 tunnel mode

Note: ip_vs_xmit.c will not work without "[patch v4] ipvs: IPv6 tunnel mode"
Unfortunately the patches don't apply with the persistence engine
patches which were recently merged into nf-next-2.6 (although
"[patch v4.1 ]ipvs: IPv6 tunnel mode" is still unmerged).

I do have a patch based on the nf-next without the SIP/PE patch

I'm happy to work with you to make the required changes there.
I would appreciate that.

(I realise those patches weren't merged when you made your post.
  But regardless, either your or me will need to update the patches).

Another issue is that your patches seem to be split in a way
where the build breaks along the way. E.g. after applying
patch 1, the build breaks. Could you please split things up
in a manner such that this doesn't happen. The reason being
that it breaks bisection.

Hmm, Daniel also pointed at this,
The Patch is quite large, and will become even larger with pe and sip.
My Idea was to review the patch in pieces and put it together in one or two 
large patches when submitting it.
I don't know that might be a stupid ?
It's hard to break it up, making the code reentrant causes changes every where.

Daniel L, had another approach break it into many many tiny patches.

These patches can be folded if the result is an acceptable patch size. The more important is to not break the init_net. Introducing the namespace concept step by step, from the bottom-up in the call stack is good because we focus first on making the namespace available in the code while keeping the init_net functional. The small patches help to git bisect and spot the patch which lead to a regression. As the patches are small, it is easy to quickly find the culprit and fix it, especially for the people who are not author of these patches.

During this step of namespace introduction, we prevent to unshare the network for ipvs, with a non-op when the namespace is not init_net.

When everything is ready and the init_net is running without problem, we remove the non-op and consolidate the namespace part of ipvs.

This is how I am used to do, but that does not imply it is the best way to do that :)

Lastly, could you provide a unique subject for each patch.
I know its a bit tedious, but it does make a difference when
browsing the changelog.

Yepp, no problem

To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at

To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at

<Prev in Thread] Current Thread [Next in Thread>