[RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[François Cachereul]

Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
if the device is connected to a l2tp session socket.
Restore the flag in ppp_disconnect_channel().

Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
---
I'm trying to get rid of this flag for ppp device connected to l2tp
session, it's seem to be safe to do it for as l2tp_ppp module hasn't any
reference to the ppp device except to the device name. We can probably
do it for others modules but pppoe and pptp will require more work.

I remove the flag for l2tp in ppp_generic.c because I couldn't find a
place like a callback to do it in l2tp_ppp.c. The best will be to 
remove the flag for all ppp devices.

François

 drivers/net/ppp/ppp_generic.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 72ff14b..7ccf2ae 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -54,6 +54,8 @@
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 
+#include <linux/if_pppox.h>
+
 #define PPP_VERSION	"2.4.2"
 
 /*
@@ -2861,6 +2863,7 @@ ppp_connect_channel(struct channel *pch, int unit)
 	struct ppp_net *pn;
 	int ret = -ENXIO;
 	int hdrlen;
+	struct sock *sk;
 
 	pn = ppp_pernet(pch->chan_net);
 
@@ -2883,6 +2886,13 @@ ppp_connect_channel(struct channel *pch, int unit)
 	++ppp->n_channels;
 	pch->ppp = ppp;
 	atomic_inc(&ppp->file.refcnt);
+
+	/* allow ppp net device to be moved in another network namespace
+	 * if it's connected to an l2tp session */
+	sk = (struct sock *)pch->chan->private;
+	if (sk && sk->sk_protocol == PX_PROTO_OL2TP)
+		ppp->dev->features &= ~NETIF_F_NETNS_LOCAL;
+
 	ppp_unlock(ppp);
 	ret = 0;
 
@@ -2912,6 +2922,7 @@ ppp_disconnect_channel(struct channel *pch)
 		list_del(&pch->clist);
 		if (--ppp->n_channels == 0)
 			wake_up_interruptible(&ppp->file.rwait);
+		ppp->dev->features |= NETIF_F_NETNS_LOCAL;
 		ppp_unlock(ppp);
 		if (atomic_dec_and_test(&ppp->file.refcnt))
 			ppp_destroy_interface(ppp);
-- 
1.7.10.4

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[James Chapman]

On 24/10/13 11:30, François Cachereul wrote:
> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
> if the device is connected to a l2tp session socket.
> Restore the flag in ppp_disconnect_channel().

What about pppd's network namespace? Also, L2TP's tunnel socket (UDP or
L2TP/IP) will be in a different namespace if the ppp interface is moved.

> 
> Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
> ---
> I'm trying to get rid of this flag for ppp device connected to l2tp
> session, it's seem to be safe to do it for as l2tp_ppp module hasn't any
> reference to the ppp device except to the device name. We can probably
> do it for others modules but pppoe and pptp will require more work.
> 
> I remove the flag for l2tp in ppp_generic.c because I couldn't find a
> place like a callback to do it in l2tp_ppp.c. The best will be to 
> remove the flag for all ppp devices.
> 
> François
> 
>  drivers/net/ppp/ppp_generic.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index 72ff14b..7ccf2ae 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
> @@ -54,6 +54,8 @@
>  #include <net/net_namespace.h>
>  #include <net/netns/generic.h>
>  
> +#include <linux/if_pppox.h>
> +
>  #define PPP_VERSION	"2.4.2"
>  
>  /*
> @@ -2861,6 +2863,7 @@ ppp_connect_channel(struct channel *pch, int unit)
>  	struct ppp_net *pn;
>  	int ret = -ENXIO;
>  	int hdrlen;
> +	struct sock *sk;
>  
>  	pn = ppp_pernet(pch->chan_net);
>  
> @@ -2883,6 +2886,13 @@ ppp_connect_channel(struct channel *pch, int unit)
>  	++ppp->n_channels;
>  	pch->ppp = ppp;
>  	atomic_inc(&ppp->file.refcnt);
> +
> +	/* allow ppp net device to be moved in another network namespace
> +	 * if it's connected to an l2tp session */
> +	sk = (struct sock *)pch->chan->private;
> +	if (sk && sk->sk_protocol == PX_PROTO_OL2TP)
> +		ppp->dev->features &= ~NETIF_F_NETNS_LOCAL;
> +
>  	ppp_unlock(ppp);
>  	ret = 0;
>  
> @@ -2912,6 +2922,7 @@ ppp_disconnect_channel(struct channel *pch)
>  		list_del(&pch->clist);
>  		if (--ppp->n_channels == 0)
>  			wake_up_interruptible(&ppp->file.rwait);
> +		ppp->dev->features |= NETIF_F_NETNS_LOCAL;
>  		ppp_unlock(ppp);
>  		if (atomic_dec_and_test(&ppp->file.refcnt))
>  			ppp_destroy_interface(ppp);
> 



-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[François Cachereul]

On 10/24/2013 12:55 PM, James Chapman wrote:
> On 24/10/13 11:30, François Cachereul wrote:
>> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
>> if the device is connected to a l2tp session socket.
>> Restore the flag in ppp_disconnect_channel().
> 
> What about pppd's network namespace? Also, L2TP's tunnel socket (UDP or
> L2TP/IP) will be in a different namespace if the ppp interface is moved.

That's what I'm trying to achieve. I'm not using pppd and my problem is
as follow:  I need to isolate ppp devices from each other, even when
they are connected to sessions carried by the same L2TP tunnel. Also, I
need the authentication to be terminated to know the namespace in which
the ppp will be moved. For that, the process runs in a namespace with
its l2tp sockets (tunnel and session) in that same namespace and each
ppp device is moved in a specific namespace after authentication.
 
Regards
François

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[James Chapman]

On 24/10/13 14:41, François Cachereul wrote:
> On 10/24/2013 12:55 PM, James Chapman wrote:
>> On 24/10/13 11:30, François Cachereul wrote:
>>> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
>>> if the device is connected to a l2tp session socket.
>>> Restore the flag in ppp_disconnect_channel().
>>
>> What about pppd's network namespace? Also, L2TP's tunnel socket (UDP or
>> L2TP/IP) will be in a different namespace if the ppp interface is moved.
> 
> That's what I'm trying to achieve. I'm not using pppd and my problem is
> as follow:  I need to isolate ppp devices from each other, even when
> they are connected to sessions carried by the same L2TP tunnel.

I'm thinking about the implications of a skb in the net namespace of the
ppp interface passing through a tunnel socket which is in another
namespace. I think net namespaces are completely isolated.

To keep your ppp interfaces isolated from each other, have you
considered using netfilter to prevent data being passed between ppp
interfaces?

> Also, I
> need the authentication to be terminated to know the namespace in which
> the ppp will be moved. For that, the process runs in a namespace with
> its l2tp sockets (tunnel and session) in that same namespace and each
> ppp device is moved in a specific namespace after authentication.
>  
> Regards
> François
> 

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[Benjamin LaHaise]

On Thu, Oct 24, 2013 at 04:43:42PM +0100, James Chapman wrote:
> I'm thinking about the implications of a skb in the net namespace of the
> ppp interface passing through a tunnel socket which is in another
> namespace. I think net namespaces are completely isolated.
> 
> To keep your ppp interfaces isolated from each other, have you
> considered using netfilter to prevent data being passed between ppp
> interfaces?

Using network namespaces for this is far more efficient.  We've already 
added support for doing this to other tunneling interfaces.  This approach 
also makes creating VPNs where there is re-use of the private address space 
between different customers far easier to implement.

		-ben
-- 
"Thought is the essence of where you are now."

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[James Chapman]

On 24/10/13 16:53, Benjamin LaHaise wrote:
> On Thu, Oct 24, 2013 at 04:43:42PM +0100, James Chapman wrote:
>> I'm thinking about the implications of a skb in the net namespace of the
>> ppp interface passing through a tunnel socket which is in another
>> namespace. I think net namespaces are completely isolated.
>>
>> To keep your ppp interfaces isolated from each other, have you
>> considered using netfilter to prevent data being passed between ppp
>> interfaces?
> 
> Using network namespaces for this is far more efficient.  We've already 
> added support for doing this to other tunneling interfaces.  This approach 
> also makes creating VPNs where there is re-use of the private address space 
> between different customers far easier to implement.
> 
> 		-ben

Yes, it's definitely more efficient and potentially useful, I agree.

But unlike the other tunneling interfaces for which this has already
been done, L2TP uses a socket for its tunnel and a skb will cross net
namespace boundaries while passing through the socket. I remember a
similar discussion came up several months ago with vxlan which also uses
UDP sockets. See http://www.spinics.net/lists/netdev/msg221498.html.

Changing the behaviour of ppp interfaces only when they are created by
l2tp feels wrong to me, unless it is the first step in doing the same
for all ppp interfaces.

-- 
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[François Cachereul]

On 10/24/2013 06:51 PM, James Chapman wrote:
> On 24/10/13 16:53, Benjamin LaHaise wrote:
>> On Thu, Oct 24, 2013 at 04:43:42PM +0100, James Chapman wrote:
>>> I'm thinking about the implications of a skb in the net namespace of the
>>> ppp interface passing through a tunnel socket which is in another
>>> namespace. I think net namespaces are completely isolated.
>>>
>>> To keep your ppp interfaces isolated from each other, have you
>>> considered using netfilter to prevent data being passed between ppp
>>> interfaces?
>>
>> Using network namespaces for this is far more efficient.  We've already 
>> added support for doing this to other tunneling interfaces.  This approach 
>> also makes creating VPNs where there is re-use of the private address space 
>> between different customers far easier to implement.
>>
>> 		-ben
> 
> Yes, it's definitely more efficient and potentially useful, I agree.
> 
> But unlike the other tunneling interfaces for which this has already
> been done, L2TP uses a socket for its tunnel and a skb will cross net
> namespace boundaries while passing through the socket. I remember a
> similar discussion came up several months ago with vxlan which also uses
> UDP sockets. See http://www.spinics.net/lists/netdev/msg221498.html.
> 
> Changing the behaviour of ppp interfaces only when they are created by
> l2tp feels wrong to me, unless it is the first step in doing the same
> for all ppp interfaces.


I agree, I only took care of l2TP first because it seemed safe and that's
why I posted the patch as RFC in the first place.

François

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[François Cachereul]

On 10/24/2013 05:53 PM, Benjamin LaHaise wrote:
> On Thu, Oct 24, 2013 at 04:43:42PM +0100, James Chapman wrote:
>> I'm thinking about the implications of a skb in the net namespace of the
>> ppp interface passing through a tunnel socket which is in another
>> namespace. I think net namespaces are completely isolated.
>>
>> To keep your ppp interfaces isolated from each other, have you
>> considered using netfilter to prevent data being passed between ppp
>> interfaces?
> 
> Using network namespaces for this is far more efficient.  We've already 
> added support for doing this to other tunneling interfaces.  This approach 
> also makes creating VPNs where there is re-use of the private address space 
> between different customers far easier to implement.
> 
> 		-ben

That's indeed on of the problems we have to deal with and net namespaces
seems to be the right answer.

François

Re: [RFC PATCH net-next] ppp: Allow ppp device connected to an l2tp session to change of namespace

[Sergei Shtylyov]

Hello.

On 10/24/2013 02:30 PM, François Cachereul wrote:

> Remove NETIF_F_NETNS_LOCAL flag from ppp device in ppp_connect_channel()
> if the device is connected to a l2tp session socket.
> Restore the flag in ppp_disconnect_channel().

> Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
> ---
> I'm trying to get rid of this flag for ppp device connected to l2tp
> session, it's seem to be safe to do it for as l2tp_ppp module hasn't any
> reference to the ppp device except to the device name. We can probably
> do it for others modules but pppoe and pptp will require more work.

> I remove the flag for l2tp in ppp_generic.c because I couldn't find a
> place like a callback to do it in l2tp_ppp.c. The best will be to
> remove the flag for all ppp devices.

> François

>   drivers/net/ppp/ppp_generic.c |   11 +++++++++++
>   1 file changed, 11 insertions(+)

> diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> index 72ff14b..7ccf2ae 100644
> --- a/drivers/net/ppp/ppp_generic.c
> +++ b/drivers/net/ppp/ppp_generic.c
[...]
> @@ -2883,6 +2886,13 @@ ppp_connect_channel(struct channel *pch, int unit)
>   	++ppp->n_channels;
>   	pch->ppp = ppp;
>   	atomic_inc(&ppp->file.refcnt);
> +
> +	/* allow ppp net device to be moved in another network namespace
> +	 * if it's connected to an l2tp session */

    Acording to Documentation/CodingStyle, the preferred comment style in the 
networking code is:

/* bla
  * bla
  */

WBR, Sergei