Re: OpenVPN isolation using NetworkNamespaces



On Sat, 2016-04-02 at 21:49 +0200, Stjepan Groš wrote:
On 30.03.2016 17:26, Thomas Haller wrote:
Hi,


6. Certain aspects of NMManager are global for every network
namespace, others are not. For example, sleeping state (or
should
it
be separate for every network namespace so that some network
namespaces can be suspended?).

7. Related to 7, the best approach would be to refactor
NMManager
itself, but that would make very hard to keep HEAD and MIF
branches
in sync.
Right, this was the biggest confusion I had. Your NMNetns takes
over
some work of NMManager, while NMManager is global. I guess that
makes
sense.
But currently it's unclear which instance (NMManager,
NMNetnsController, NMNetns, NMPolicy) does what, and how the
interact
with namespaces.

I think this is difficult to get right. I think that NMNetns
should
be
as simple as possible and more tell NMManager what to do.
 
If you make NMNetns as simple as possible then NMManager must
take
care of network related changes within different network
namespaces,
i.e. new devices, IP addresses,etc. This, in turn, migh require
all
the methods from NMManager to be extended with additional
argument -
network namespace - which could possibly complicate things.

It seems to me that it is lot easier to basically transform
NMManager
into NMNetns because root network namespace is just one possible
network namespace. All the current functionality done by
NMManager
within root network namespace should be done in every network
namespace. Of course, NMManager should stay, but only with the
functionality common to all network namespaces.
 
Now, it could be that the approach of making NMNetns as simple as
possible is better approach, but only way to find out is to try to
make it so - which I think is non-trivial.
So NMNetns is basically becoming what NMManager is with all the per-
namespace responsibilities. Makes sense.

NMManager stays to be a global manager (intra-namespace).

I dislike that the name "NMNetns" is a prefix of "NMNetnsController",
but why is NMNetnsController a separate object? It doesn't have much
code and it also has it's own D-Bus path. Maybe those properties and
functions should be merged into the NMManager object.


Moving large parts of NMManager to NMNetns is probably the right
solution, but a bit hard to develop in parallel as master progresses.
 
If that is done, then there is no need for NMNetnsController because
its functionality should be implemented in NMManager.

I have one problem now. Namely, function _is_root in NMNetns doesn't
work, or am I doing something wrong? When constructing
NMNetnsController object the first NMNetns is created and that one
should be for root network namespace. I'm using _is_root() to skip
device initialization (for root that is done by NMManager now) but
_is_root() returns FALSE?

Hi Stjepan,


The NMPNetns instance that represents the root namespace
is nmp_netns_get_initial(). You don't create that one, it's always
there.


You change to call 
  priv->nmp_netns = nmp_netns_new();
is wrong.
This creates a new namespace and switches to it. When you switch
namespace at a certain moment, you *must* switch back before you return
from the function; that is, you must always call nmp_netns_pop() before
your function returns.

The reason for that is, that at any point it must be known which
namespace is currently active. If you call a function and that function
switchs namespace, that function is required to restore the callers
namespace before returning.


For that reason, I think it's better that nm_netns_new() /
nm_netns_init() does *not* call nmp_netns_new() to create a new
namespace.
Instead, it seems more logical that it takes over the *current*
namespace nmp_netns_get_current(). So it's the job of the caller of
nm_netns_new() -- create_new_namespace() -- to prepare the namespace.

Then, when creating the NMNetns instance for the root namespace, the
caller just does not call nmp_netns_new() but stays on the root
namespace. At that point, nmp_netns_get_currenty() equals
nmp_netns_get_initial() and it just works.



Does that make sense?

Thomas






The code with my changes to your branch is on the following URL:

https://github.com/sgros/NETNS_NetworkManager

SG

Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]