Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-20 Thread Tom Herbert
On Wed, Sep 20, 2017 at 5:04 PM, Harald Welte  wrote:
> Hi Tom,
>
> On Wed, Sep 20, 2017 at 01:40:54PM -0700, Tom Herbert wrote:
>> On Wed, Sep 20, 2017 at 12:45 PM, David Miller  wrote:
>> > There is a socket associated with the tunnel to do the encapsulation
>> > and it has an address family, right?
>>
>> If fd's are set from userspace for the sockets then we could derive
>> the address family from them. I'll change that. Although, looking at
>> now I am wondering why were passing fds into GTP instead of just
>> having the kernel create the UDP port like is done for other encaps.
>
> because the userspace process has to take care of those bits of GTP-U
> that the kernel doesn't, such as responding to GTP ECHO requests with
> GTP echo responses.  Only the "GTP Message type G-PDU" is handled in the
> kernel, as only those frames contain user plane.  See table 1 of Section
> 7.1 of 3GPP TS 29.060.
>
Okay, thanks for the explanation.

Tom


Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-20 Thread Harald Welte
Hi Tom,

On Wed, Sep 20, 2017 at 01:40:54PM -0700, Tom Herbert wrote:
> On Wed, Sep 20, 2017 at 12:45 PM, David Miller  wrote:
> > There is a socket associated with the tunnel to do the encapsulation
> > and it has an address family, right?
> 
> If fd's are set from userspace for the sockets then we could derive
> the address family from them. I'll change that. Although, looking at
> now I am wondering why were passing fds into GTP instead of just
> having the kernel create the UDP port like is done for other encaps.

because the userspace process has to take care of those bits of GTP-U
that the kernel doesn't, such as responding to GTP ECHO requests with
GTP echo responses.  Only the "GTP Message type G-PDU" is handled in the
kernel, as only those frames contain user plane.  See table 1 of Section
7.1 of 3GPP TS 29.060.

If you create the socket in the kernel, how would you hand the socket to
the userspace process later on?

IMHO, it feels more natural to simply create it in userspace (like you
would do in the non-kernel-accelerated case) and then simply handle the
G-PDU messages in the kernel while doing the rest in userspace.

But if there's another method that feels more usual to the kernel
community, I'm not against any changes - but given kernel policies, we'd
have to keep userspace compatbility, right?

Regards,
Harald
-- 
- Harald Weltehttp://laforge.gnumonks.org/

"Privacy in residential applications is a desirable marketing option."
  (ETSI EN 300 175-7 Ch. A6)


Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-20 Thread Tom Herbert
On Wed, Sep 20, 2017 at 12:45 PM, David Miller  wrote:
> From: Tom Herbert 
> Date: Wed, 20 Sep 2017 11:03:52 -0700
>
>> On Mon, Sep 18, 2017 at 9:19 PM, David Miller  wrote:
>>> From: Tom Herbert 
>>> Date: Mon, 18 Sep 2017 17:38:58 -0700
>>>
 Allow peers to be specified by IPv6 addresses.

 Signed-off-by: Tom Herbert 
>>>
>>> Hmmm, can you just check the socket family or something like that?
>>
>> I'm not sure what code you're referring to.
>
> There is a socket associated with the tunnel to do the encapsulation
> and it has an address family, right?

If fd's are set from userspace for the sockets then we could derive
the address family from them. I'll change that. Although, looking at
now I am wondering why were passing fds into GTP instead of just
having the kernel create the UDP port like is done for other encaps.

Tom


Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-20 Thread David Miller
From: Tom Herbert 
Date: Wed, 20 Sep 2017 11:03:52 -0700

> On Mon, Sep 18, 2017 at 9:19 PM, David Miller  wrote:
>> From: Tom Herbert 
>> Date: Mon, 18 Sep 2017 17:38:58 -0700
>>
>>> Allow peers to be specified by IPv6 addresses.
>>>
>>> Signed-off-by: Tom Herbert 
>>
>> Hmmm, can you just check the socket family or something like that?
> 
> I'm not sure what code you're referring to.

There is a socket associated with the tunnel to do the encapsulation
and it has an address family, right?


Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-20 Thread Tom Herbert
On Mon, Sep 18, 2017 at 9:19 PM, David Miller  wrote:
> From: Tom Herbert 
> Date: Mon, 18 Sep 2017 17:38:58 -0700
>
>> Allow peers to be specified by IPv6 addresses.
>>
>> Signed-off-by: Tom Herbert 
>
> Hmmm, can you just check the socket family or something like that?

I'm not sure what code you're referring to.

Thanks


Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-19 Thread Harald Welte
Hi Tom,

On Mon, Sep 18, 2017 at 05:38:58PM -0700, Tom Herbert wrote:
> Allow peers to be specified by IPv6 addresses.

> + u16 peer_af;
> + union {
> + struct in_addr  peer_addr_ip4;
> + struct in6_addr peer_addr_ip6;
> + };

this will not really work, as an union means that a PDP context
will be either IPv4-only or IPV6-only, while in reality there
are three types, see my other mail.  So you have to  deal
with v4-only, v6-only or v4v6.

The v6-only is legacy by now, and all modern phones I've tested in
recent years can do v4v6 rather than having a v4-only and a v6-only
PDP context in parallel.

>From the operator point of view, v4v6 is very desirable, as it basically
halves the amount of PDP contexts compared to the old approach, which
significantly reduces signalling load across your network, as well as
the amount of memory (and thus capacity) in your core network elements.

I've recently implemented v6 + v4v6 support in osmo-ggsn (see
http://git.osmocom.org/osmo-ggsn/) in case you would like to see another
FOSS implementation for v6 + v4v6 - though in userspace, of course.

-- 
- Harald Weltehttp://laforge.gnumonks.org/

"Privacy in residential applications is a desirable marketing option."
  (ETSI EN 300 175-7 Ch. A6)


Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-18 Thread David Miller
From: Tom Herbert 
Date: Mon, 18 Sep 2017 17:38:58 -0700

> Allow peers to be specified by IPv6 addresses.
> 
> Signed-off-by: Tom Herbert 

Hmmm, can you just check the socket family or something like that?


[PATCH net-next 08/14] gtp: Support encpasulating over IPv6

2017-09-18 Thread Tom Herbert
Allow peers to be specified by IPv6 addresses.

Signed-off-by: Tom Herbert 
---
 drivers/net/gtp.c| 198 +--
 include/uapi/linux/gtp.h |   1 +
 include/uapi/linux/if_link.h |   3 +
 3 files changed, 158 insertions(+), 44 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 62c0c968efa6..121b41e7a901 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -61,7 +62,11 @@ struct pdp_ctx {
struct in6_addr ms_addr_ip6;
};
 
-   struct in_addr  peer_addr_ip4;
+   u16 peer_af;
+   union {
+   struct in_addr  peer_addr_ip4;
+   struct in6_addr peer_addr_ip6;
+   };
 
struct sock *sk;
struct net_device   *dev;
@@ -76,6 +81,8 @@ struct pdp_ctx {
 struct gtp_dev {
struct list_headlist;
 
+   unsigned intis_ipv6:1;
+
struct sock *sk0;
struct sock *sk1u;
 
@@ -515,8 +522,6 @@ static int gtp_xmit(struct sk_buff *skb, struct net_device 
*dev,
struct pdp_ctx *pctx)
 {
struct sock *sk = pctx->sk;
-   __be32 saddr = inet_sk(sk)->inet_saddr;
-   struct rtable *rt;
int err = 0;
 
/* Ensure there is sufficient headroom. */
@@ -526,28 +531,63 @@ static int gtp_xmit(struct sk_buff *skb, struct 
net_device *dev,
 
skb_reset_inner_headers(skb);
 
-   rt = ip_tunnel_get_route(dev, skb, sk->sk_protocol,
-sk->sk_bound_dev_if, RT_CONN_FLAGS(sk),
-pctx->peer_addr_ip4.s_addr, &saddr,
-pctx->gtp_port, pctx->gtp_port,
-&pctx->dst_cache, NULL);
+   if (pctx->peer_af == AF_INET) {
+   __be32 saddr = inet_sk(sk)->inet_saddr;
+   struct rtable *rt;
 
-   if (IS_ERR(rt)) {
-   err = PTR_ERR(rt);
-   goto out_err;
-   }
+   rt = ip_tunnel_get_route(dev, skb, sk->sk_protocol,
+sk->sk_bound_dev_if, RT_CONN_FLAGS(sk),
+pctx->peer_addr_ip4.s_addr, &saddr,
+pctx->gtp_port, pctx->gtp_port,
+&pctx->dst_cache, NULL);
+
+   if (IS_ERR(rt)) {
+   err = PTR_ERR(rt);
+   goto out_err;
+   }
+
+   skb_dst_drop(skb);
+
+   gtp_push_header(skb, pctx);
+   udp_tunnel_xmit_skb(rt, sk, skb, saddr,
+   pctx->peer_addr_ip4.s_addr,
+   0, ip4_dst_hoplimit(&rt->dst), 0,
+   pctx->gtp_port, pctx->gtp_port,
+   false, false);
 
-   skb_dst_drop(skb);
+   netdev_dbg(dev, "gtp -> IP src: %pI4 dst: %pI4\n",
+  &saddr, &pctx->peer_addr_ip4.s_addr);
 
-   gtp_push_header(skb, pctx);
-   udp_tunnel_xmit_skb(rt, sk, skb, saddr,
-   pctx->peer_addr_ip4.s_addr,
-   0, ip4_dst_hoplimit(&rt->dst), 0,
-   pctx->gtp_port, pctx->gtp_port,
-   false, false);
+#if IS_ENABLED(CONFIG_IPV6)
+   } else if (pctx->peer_af == AF_INET6) {
+   struct in6_addr saddr = inet6_sk(sk)->saddr;
+   struct dst_entry *dst;
 
-   netdev_dbg(dev, "gtp -> IP src: %pI4 dst: %pI4\n",
-  &saddr, &pctx->peer_addr_ip4.s_addr);
+   dst = ip6_tnl_get_route(dev, skb, sk, sk->sk_protocol,
+   sk->sk_bound_dev_if, 0,
+   0, &pctx->peer_addr_ip6, &saddr,
+   pctx->gtp_port, pctx->gtp_port,
+   &pctx->dst_cache, NULL);
+
+   if (IS_ERR(dst)) {
+   err = PTR_ERR(dst);
+   goto out_err;
+   }
+
+   skb_dst_drop(skb);
+
+   gtp_push_header(skb, pctx);
+   udp_tunnel6_xmit_skb(dst, sk, skb, dev,
+&saddr, &pctx->peer_addr_ip6,
+0, ip6_dst_hoplimit(dst), 0,
+pctx->gtp_port, pctx->gtp_port,
+true);
+
+   netdev_dbg(dev, "gtp -> IP src: %pI6 dst: %pI6\n",
+  &saddr, &pctx->peer_addr_ip6);
+
+#endif
+   }
 
return 0;
 
@@ -652,7 +692,8 @@ static void gtp_link_setup(struct net_device *dev)
 
/* Assume largest header, ie. GTPv0. */
dev->needed_headroom= LL_MAX_HEADER +
-