Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Harald, On 03/15/2017 05:39 PM, Harald Welte wrote: Hi Jonas, are you working on the review feedback that was provided back in early February? I think there were some comments like * remove unrelated cosmetic change in comment * change from FLAGS to a dedicated MODE netlink attribute * add libgtpnl code and some usage information or even sample scripts I would definitely like to see this move forward, particularly in order to test the GGSN-side code. Sorry for the delay in this. I've sent, just now, revised patches to the kernel module. I was going to send some libgtpnl patches but I noticed, when gathering these up, that you have already made most of the necessary changes on a new branch. What you've got there is almost identical to what I've got. Since you used the terminology "role" instead of "mode" in your libgtpnl branch, I made the corresponding change in the kernel module, too, so it now calls this role instead of mode. Regards, Jonas Regards, Harald
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Harald, On Wed, Mar 15, 2017 at 08:10:38PM +0100, Harald Welte wrote: > I've modified the patch slightly, see below (compile-tested, but not > otherwise tested yet). Basically rename the flags attribute to 'role', > expand the commit log and removed unrelated cosmetic changes. > > I've also prepared a corresponding change to libgtpnl into the > laforge/sgsn-rol branch, see > http://git.osmocom.org/libgtpnl/commit/?h=laforge/sgsn-role > > This is not yet tested in any way, but I'm planning to add some > associated support to the command line tools and then give it some > testing (both against the kernel GTP in GGSN mode, as well as an > independent userspace GTP implementation). Thanks Harald. > > It would be good if we provide a way to configure GTP via iproute2 for > > testing purposes. > > I don't really care about which tool is used, as long as it is easily > available [and FOSS, of course]. > > > We would need to create some dummy socket from > > kernel too though so we don't need any userspace daemon for this > > testing mode. > > I don't really like that latter idea. It sounds too much like a hack to > me. But then, I don't have enough phantasy right now ti imagine how an > actual implementation would look like. It's not that far away, we can just create the udp socket from kernelspace via udp_sock_create() in the test mode. So we don't need to pass the file descriptor from userspace. But not asking you to work on this, just an idea. > To me, it is perfectly fine to run a simple, small utility in userspace > even for testing. No problem.
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
On Wed, Mar 15, 2017 at 08:10:38PM +0100, Harald Welte wrote: > I've modified the patch slightly, see below (compile-tested, but not > otherwise tested yet). Basically rename the flags attribute to 'role', > expand the commit log and removed unrelated cosmetic changes. I also have a version against current net-next/master, in case anyone is interested.. >From 3274a3303d1ec997392a07a92666d57b13997658 Mon Sep 17 00:00:00 2001 From: Jonas BonnDate: Wed, 15 Mar 2017 20:24:28 +0100 Subject: [PATCH] gtp: support SGSN-side tunnels The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP contexts based on the incoming packets _destination_ address. For real-world use cases, this is sufficient, as the other side of a GTP tunnel is not in fact implemented by GTP, but by the protocol stacking of a mobile station / user equipment on the radio interface (like PDCP, SNDCP). However, if we want to simulate the mobile station, radio access network and SGSN (for example to test the GGSN side implementation), then we want to be identifying PDP contexts based on _source_ address. This patch adds a "role" attribute at GTP-link creation time to specify whether we behave like the GGSN or SGSN role of the tunnel; this attribute is then used to determine which part of the IP packet to use in determining the PDP context. Signed-off-by: Jonas Bonn Signed-off-by: Harald Welte --- drivers/net/gtp.c| 46 +--- include/uapi/linux/gtp.h | 2 +- include/uapi/linux/if_link.h | 7 +++ 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 3e1854f34420..3ab593b9be85 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -74,6 +74,7 @@ struct gtp_dev { struct net_device *dev; + enum ifla_gtp_role role; unsigned inthash_size; struct hlist_head *tid_hash; struct hlist_head *addr_hash; @@ -154,8 +155,8 @@ static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr) return NULL; } -static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, - unsigned int hdrlen) +static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, + unsigned int hdrlen, enum ifla_gtp_role role) { struct iphdr *iph; @@ -164,27 +165,31 @@ static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, iph = (struct iphdr *)(skb->data + hdrlen); - return iph->saddr == pctx->ms_addr_ip4.s_addr; + if (role == GTP_ROLE_SGSN) + return iph->daddr == pctx->ms_addr_ip4.s_addr; + else + return iph->saddr == pctx->ms_addr_ip4.s_addr; } /* Check if the inner IP source address in this packet is assigned to any * existing mobile subscriber. */ -static bool gtp_check_src_ms(struct sk_buff *skb, struct pdp_ctx *pctx, -unsigned int hdrlen) +static bool gtp_check_ms(struct sk_buff *skb, struct pdp_ctx *pctx, +unsigned int hdrlen, enum ifla_gtp_role role) { switch (ntohs(skb->protocol)) { case ETH_P_IP: - return gtp_check_src_ms_ipv4(skb, pctx, hdrlen); + return gtp_check_ms_ipv4(skb, pctx, hdrlen, role); } return false; } -static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, unsigned int hdrlen) +static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, unsigned int hdrlen, + enum ifla_gtp_role role) { struct pcpu_sw_netstats *stats; - if (!gtp_check_src_ms(skb, pctx, hdrlen)) { + if (!gtp_check_ms(skb, pctx, hdrlen, role)) { netdev_dbg(pctx->dev, "No PDP ctx for this MS\n"); return 1; } @@ -239,7 +244,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) return 1; } - return gtp_rx(pctx, skb, hdrlen); + return gtp_rx(pctx, skb, hdrlen, gtp->role); } static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) @@ -281,7 +286,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) return 1; } - return gtp_rx(pctx, skb, hdrlen); + return gtp_rx(pctx, skb, hdrlen, gtp->role); } static void gtp_encap_destroy(struct sock *sk) @@ -481,7 +486,11 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev, * Prepend PDP header with TEI/TID from PDP ctx. */ iph = ip_hdr(skb); - pctx = ipv4_pdp_find(gtp, iph->daddr); + if (gtp->role == GTP_ROLE_SGSN) + pctx = ipv4_pdp_find(gtp, iph->saddr); + else + pctx = ipv4_pdp_find(gtp, iph->daddr); + if (!pctx) {
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Pablo, On Wed, Mar 15, 2017 at 06:23:48PM +0100, Pablo Neira Ayuso wrote: > On Wed, Mar 15, 2017 at 05:39:16PM +0100, Harald Welte wrote: > > > > I would definitely like to see this move forward, particularly in order > > to test the GGSN-side code. > > Agreed. I've modified the patch slightly, see below (compile-tested, but not otherwise tested yet). Basically rename the flags attribute to 'role', expand the commit log and removed unrelated cosmetic changes. I've also prepared a corresponding change to libgtpnl into the laforge/sgsn-rol branch, see http://git.osmocom.org/libgtpnl/commit/?h=laforge/sgsn-role This is not yet tested in any way, but I'm planning to add some associated support to the command line tools and then give it some testing (both against the kernel GTP in GGSN mode, as well as an independent userspace GTP implementation). > It would be good if we provide a way to configure GTP via iproute2 for > testing purposes. I don't really care about which tool is used, as long as it is easily available [and FOSS, of course]. > We would need to create some dummy socket from > kernel too though so we don't need any userspace daemon for this > testing mode. I don't really like that latter idea. It sounds too much like a hack to me. But then, I don't have enough phantasy right now ti imagine how an actual implementation would look like. To me, it is perfectly fine to run a simple, small utility in userspace even for testing. Regards, Harald >From 63920950f9498069993def78e178bde85c174e0c Mon Sep 17 00:00:00 2001 From: Jonas BonnDate: Wed, 15 Mar 2017 17:52:28 +0100 Subject: [PATCH] gtp: support SGSN-side tunnels The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP contexts based on the incoming packets _destination_ address. For real-world use cases, this is sufficient, as the other side of a GTP tunnel is not in fact implemented by GTP, but by the protocol stacking of a mobile station / user equipment on the radio interface (like PDCP, SNDCP). However, if we want to simulate the mobile station, radio access network and SGSN (for example to test the GGSN side implementation), then we want to be identifying PDP contexts based on _source_ address. This patch adds a "role" attribute at GTP-link creation time to specify whether we behave like the GGSN or SGSN role of the tunnel; this attribute is then used to determine which part of the IP packet to use in determining the PDP context. Signed-off-by: Jonas Bonn Signed-off-by: Harald Welte diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 99d3df788ce8..9aef4217f6e1 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -71,6 +71,7 @@ struct gtp_dev { struct net_device *dev; + enum ifla_gtp_role role; unsigned inthash_size; struct hlist_head *tid_hash; struct hlist_head *addr_hash; @@ -149,8 +150,8 @@ static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, __be32 ms_addr) return NULL; } -static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, - unsigned int hdrlen) +static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, + unsigned int hdrlen, enum ifla_gtp_role role) { struct iphdr *iph; @@ -159,18 +160,21 @@ static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, iph = (struct iphdr *)(skb->data + hdrlen); - return iph->saddr == pctx->ms_addr_ip4.s_addr; + if (role == GTP_ROLE_SGSN) + return iph->daddr == pctx->ms_addr_ip4.s_addr; + else + return iph->saddr == pctx->ms_addr_ip4.s_addr; } /* Check if the inner IP source address in this packet is assigned to any * existing mobile subscriber. */ -static bool gtp_check_src_ms(struct sk_buff *skb, struct pdp_ctx *pctx, -unsigned int hdrlen) +static bool gtp_check_ms(struct sk_buff *skb, struct pdp_ctx *pctx, +unsigned int hdrlen, enum ifla_gtp_role role) { switch (ntohs(skb->protocol)) { case ETH_P_IP: - return gtp_check_src_ms_ipv4(skb, pctx, hdrlen); + return gtp_check_ms_ipv4(skb, pctx, hdrlen, role); } return false; } @@ -204,7 +208,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, goto out_rcu; } - if (!gtp_check_src_ms(skb, pctx, hdrlen)) { + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->role)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); ret = -1; goto out_rcu; @@ -261,7 +265,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb, goto out_rcu; } - if (!gtp_check_src_ms(skb, pctx, hdrlen)) { + if
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
On Wed, Mar 15, 2017 at 05:39:16PM +0100, Harald Welte wrote: > Hi Jonas, > > are you working on the review feedback that was provided back in early > February? I think there were some comments like > * remove unrelated cosmetic change in comment > * change from FLAGS to a dedicated MODE netlink attribute > * add libgtpnl code and some usage information or even sample scripts > > I would definitely like to see this move forward, particularly in order > to test the GGSN-side code. Agreed. It would be good if we provide a way to configure GTP via iproute2 for testing purposes. We would need to create some dummy socket from kernel too though so we don't need any userspace daemon for this testing mode.
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Jonas, are you working on the review feedback that was provided back in early February? I think there were some comments like * remove unrelated cosmetic change in comment * change from FLAGS to a dedicated MODE netlink attribute * add libgtpnl code and some usage information or even sample scripts I would definitely like to see this move forward, particularly in order to test the GGSN-side code. 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 1/1] gtp: support SGSN-side tunnels
Hi Andreas, Pablo, Jonas, I think that the SGSN/GGSN role flag (or whatever it may end up being called) logically belongs in the gtp-device at this point, and will in the future belong to the UDP/GTP-socket (with Andreas' proposed changes). Having it per-pdp-context indeed seems odd and just provide a way to create broken configurations (and increase the memory use per pdp context, of which you have many more than netdevs or gtp-sockets). -- - Harald Weltehttp://laforge.gnumonks.org/ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi, - On Feb 13, 2017, at 12:16 PM, pablo pa...@netfilter.org wrote: > On Mon, Feb 13, 2017 at 10:25:19AM +0100, Andreas Schultz wrote: >> Hi, >> >> I'm a bit late to comment, but maybe you can consider an additional >> change for v2... >> >> - On Feb 3, 2017, at 10:12 AM, Jonas Bonn jo...@southpole.se wrote: >> >> > The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP >> > contexts based on the incoming packets _destination_ address. If we >> > want to write an SGSN, then we want to be idenityfing PDP contexts >> > based on _source_ address. >> > >> > This patch adds a "flags" argument at GTP-link creation time to specify >> > whether we are on the GGSN or SGSN side of the tunnel; this flag is then >> > used to determine which part of the IP packet to use in determining >> > the PDP context. >> > >> > Signed-off-by: Jonas Bonn>> > --- >> > >> > drivers/net/gtp.c| 43 >> > --- >> > include/uapi/linux/gtp.h | 2 +- >> > include/uapi/linux/if_link.h | 5 + >> > 3 files changed, 38 insertions(+), 12 deletions(-) >> > >> > diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c >> > index 50349a9..1bbac69 100644 >> > --- a/drivers/net/gtp.c >> > +++ b/drivers/net/gtp.c >> > @@ -72,6 +72,7 @@ struct gtp_dev { >> >struct net *net; >> >struct net_device *dev; >> > >> > + unsigned intflags; >> >> This should IMHO not go into the gtp_dev, the right place >> is the PDP context. > > So you want to allow mixed configurations where some PDP ctx may be in > SGSN mode while others in GGSN. > > This doesn't make any sense to me. On top of this, don't forget this > is just for testing, so I don't see any valid usecase for such a fine > grain thing. You are right, running such a configuration does not make sense. However, when I wrote this the PDP context looked like the most sensible palace to me. Anyhow, thinking about this again, I think that integrating that flag in a rewrite of the validation logic in the Rx path make more sense. Currently we validate the MS as soon as we have found the PDP context. This should be delayed a bit and the validation should happen after pulling the GTP header and right before injecting the payload into the net device. The flag would then indeed go into the gtp_dev. Andreas
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
On Mon, Feb 13, 2017 at 10:25:19AM +0100, Andreas Schultz wrote: > Hi, > > I'm a bit late to comment, but maybe you can consider an additional > change for v2... > > - On Feb 3, 2017, at 10:12 AM, Jonas Bonn jo...@southpole.se wrote: > > > The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP > > contexts based on the incoming packets _destination_ address. If we > > want to write an SGSN, then we want to be idenityfing PDP contexts > > based on _source_ address. > > > > This patch adds a "flags" argument at GTP-link creation time to specify > > whether we are on the GGSN or SGSN side of the tunnel; this flag is then > > used to determine which part of the IP packet to use in determining > > the PDP context. > > > > Signed-off-by: Jonas Bonn> > --- > > > > drivers/net/gtp.c| 43 > > --- > > include/uapi/linux/gtp.h | 2 +- > > include/uapi/linux/if_link.h | 5 + > > 3 files changed, 38 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c > > index 50349a9..1bbac69 100644 > > --- a/drivers/net/gtp.c > > +++ b/drivers/net/gtp.c > > @@ -72,6 +72,7 @@ struct gtp_dev { > > struct net *net; > > struct net_device *dev; > > > > + unsigned intflags; > > This should IMHO not go into the gtp_dev, the right place > is the PDP context. So you want to allow mixed configurations where some PDP ctx may be in SGSN mode while others in GGSN. This doesn't make any sense to me. On top of this, don't forget this is just for testing, so I don't see any valid usecase for such a fine grain thing.
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi, I'm a bit late to comment, but maybe you can consider an additional change for v2... - On Feb 3, 2017, at 10:12 AM, Jonas Bonn jo...@southpole.se wrote: > The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP > contexts based on the incoming packets _destination_ address. If we > want to write an SGSN, then we want to be idenityfing PDP contexts > based on _source_ address. > > This patch adds a "flags" argument at GTP-link creation time to specify > whether we are on the GGSN or SGSN side of the tunnel; this flag is then > used to determine which part of the IP packet to use in determining > the PDP context. > > Signed-off-by: Jonas Bonn> --- > > drivers/net/gtp.c| 43 --- > include/uapi/linux/gtp.h | 2 +- > include/uapi/linux/if_link.h | 5 + > 3 files changed, 38 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c > index 50349a9..1bbac69 100644 > --- a/drivers/net/gtp.c > +++ b/drivers/net/gtp.c > @@ -72,6 +72,7 @@ struct gtp_dev { > struct net *net; > struct net_device *dev; > > + unsigned intflags; This should IMHO not go into the gtp_dev, the right place is the PDP context. In the current design the netdevice might seem like the logical place, but the relation between tunnels and netdevices is actually wrong. This leaves the PDP context the only correct place. I currently have an ongoing discussion with Pablo about this. Harald seems already convinced (http://marc.info/?l=linux-netdev=148638992010344=2). > unsigned inthash_size; > struct hlist_head *tid_hash; > struct hlist_head *addr_hash; > @@ -150,8 +151,8 @@ static struct pdp_ctx *ipv4_pdp_find(struct gtp_dev *gtp, > __be32 ms_addr) > return NULL; > } > > -static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, > - unsigned int hdrlen) > +static bool gtp_check_ms_ipv4(struct sk_buff *skb, struct pdp_ctx *pctx, > + unsigned int hdrlen, unsigned int flags) > { > struct iphdr *iph; > > @@ -160,18 +161,22 @@ static bool gtp_check_src_ms_ipv4(struct sk_buff *skb, > struct pdp_ctx *pctx, > > iph = (struct iphdr *)(skb->data + hdrlen); > > - return iph->saddr == pctx->ms_addr_ip4.s_addr; > + if (flags & GTP_FLAGS_SGSN) { > + return iph->daddr == pctx->ms_addr_ip4.s_addr; > + } else { > + return iph->saddr == pctx->ms_addr_ip4.s_addr; > + } > } > > -/* Check if the inner IP source address in this packet is assigned to any > +/* Check if the inner IP address in this packet is assigned to any > * existing mobile subscriber. > */ > -static bool gtp_check_src_ms(struct sk_buff *skb, struct pdp_ctx *pctx, > - unsigned int hdrlen) > +static bool gtp_check_ms(struct sk_buff *skb, struct pdp_ctx *pctx, > + unsigned int hdrlen, unsigned int flags) > { > switch (ntohs(skb->protocol)) { > case ETH_P_IP: > - return gtp_check_src_ms_ipv4(skb, pctx, hdrlen); > + return gtp_check_ms_ipv4(skb, pctx, hdrlen, flags); > } > return false; > } > @@ -205,7 +210,7 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct > sk_buff *skb, > goto out_rcu; > } > > - if (!gtp_check_src_ms(skb, pctx, hdrlen)) { > + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->flags)) { > netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); > ret = -1; > goto out_rcu; > @@ -248,7 +253,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, > struct > sk_buff *skb, > if (gtp1->flags & GTP1_F_MASK) > hdrlen += 4; > > - /* Make sure the header is larger enough, including extensions. */ > + /* Make sure the header is large enough, including extensions. */ > if (!pskb_may_pull(skb, hdrlen)) > return -1; > > @@ -262,7 +267,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, > struct > sk_buff *skb, > goto out_rcu; > } > > - if (!gtp_check_src_ms(skb, pctx, hdrlen)) { > + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->flags)) { > netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); > ret = -1; > goto out_rcu; > @@ -491,7 +496,11 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct > net_device *dev, >* Prepend PDP header with TEI/TID from PDP ctx. >*/ > iph = ip_hdr(skb); > - pctx = ipv4_pdp_find(gtp, iph->daddr); > + if (gtp->flags & GTP_FLAGS_SGSN) { > + pctx = ipv4_pdp_find(gtp, iph->saddr); > + } else { > + pctx = ipv4_pdp_find(gtp, iph->daddr); > + } > if (!pctx) { > netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n", >
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
On Mon, Feb 06, 2017 at 03:16:22PM +0100, Harald Welte wrote: > Hi Jonas, > > On Mon, Feb 06, 2017 at 02:33:07PM +0100, Jonas Bonn wrote: > > Fair enough. The use-case I am looking at involves PGW load-testing where > > the simulated load is generated locally on the SGSN so it _is_ seeing IP > > packets and the SNDCP is left out altogether. > > Ok, it would have been useful to document that test-only feature in the > changelog and/or code. Like "support simulated RAN-side tunnels" or > "support SGSN/S-GW simulation". Right. Please, include this in your follow up v2 patch description. BTW, please also indicate [PATCH net-next] for new features. > > Perhaps this is too pathological to warrant messing with the upstream > > driver... I don't know: the symmetry does not cost much even if it's > > of limited use. > > There are plenty of features in the mainline kernel related to testing, > see pktgen for example. So I think if it doesn't impose complexity, > performance issues or stretches the existing architecture, I think > there's no reason to keep it out. > > Looking at the code, I think the one conditional on the flags is not > going to kill significant performance of the "normal" use case. But > that's of course just guessing, without any benchmark to back that up. > > Semantically, I'm not sure if the FLAGS and the re-use of the > SGSN_ADDRESS TLV is the best choice. If suddenly the meaning of the TLV > is "Peer GSN Address" then it should be called that way. We could have > a #define SGSN_ADDRESS to GSN_PEER_ADDRESS to make old code compile. > I'll let Pablo respond to this as he came up with the netlink interface, > as far as I can remember :) I agree with Harald that a new netlink TLV, ie. IFLA_GTP_MODE, to indicate if this is expecting to operate on the GGSN or SGSN side would be better. See include/uapi/linux/if_link.h. Flags allows us to combine different features, in this case we won't combine anything since these two modes are mutually exclusive. > Also, like with any changes to the kernel and netlink interface code, I > think we should always mandate similar changes to be made to libgtpnl so > the feature can actually be used/tested with the standard > tools/utilities available to anyone. Yes, at least some scripts and short text file example would suffice. Thanks!
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Jonas, On Mon, Feb 06, 2017 at 02:33:07PM +0100, Jonas Bonn wrote: > Hi Pablo, > > On 02/06/2017 12:08 PM, Pablo Neira Ayuso wrote: > >Hi Jonas, > > > >On Fri, Feb 03, 2017 at 10:12:31AM +0100, Jonas Bonn wrote: > >>The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP > >>contexts based on the incoming packets _destination_ address. If we > >>want to write an SGSN, then we want to be idenityfing PDP contexts > >>based on _source_ address. > >> > >>This patch adds a "flags" argument at GTP-link creation time to specify > >>whether we are on the GGSN or SGSN side of the tunnel; this flag is then > >>used to determine which part of the IP packet to use in determining > >>the PDP context. > >So far the implementation that I saw in osmocom relies on userspace code > >to tunnel data from ME to the SSGN/SGW running on the base station. > > > >The data we get from GGSN -> SGSN needs to be places into a SN-PDU (via > >SNDCP) when sending it to the BTS, right? So I wonder how this can be > >useful given that we would need to see real IP packets coming to the > >SSGN that we tunnel into GTP. > > Fair enough. The use-case I am looking at involves PGW load-testing where > the simulated load is generated locally on the SGSN so it _is_ seeing IP > packets and the SNDCP is left out altogether. Perhaps this is too > pathological to warrant messing with the upstream driver... I don't know: > the symmetry does not cost much even if it's of limited use. Thanks for explaining your use-case. If some basic form of this load-testing tool ends up serving everyone, ie. landing some code in the libgtpnl library that we can all use to benchmark/stress test this driver, then I would be glad to take this. Thanks!
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Jonas, Sorry, for later reply, I'm currently on vacation with almost no internet access. - On Feb 6, 2017, at 2:33 PM, Jonas Bonn jo...@southpole.se wrote: > Hi Pablo, > > On 02/06/2017 12:08 PM, Pablo Neira Ayuso wrote: >> Hi Jonas, >> >> On Fri, Feb 03, 2017 at 10:12:31AM +0100, Jonas Bonn wrote: >>> The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP >>> contexts based on the incoming packets _destination_ address. If we >>> want to write an SGSN, then we want to be idenityfing PDP contexts >>> based on _source_ address. >>> >>> This patch adds a "flags" argument at GTP-link creation time to specify >>> whether we are on the GGSN or SGSN side of the tunnel; this flag is then >>> used to determine which part of the IP packet to use in determining >>> the PDP context. >> So far the implementation that I saw in osmocom relies on userspace code >> to tunnel data from ME to the SSGN/SGW running on the base station. >> >> The data we get from GGSN -> SGSN needs to be places into a SN-PDU (via >> SNDCP) when sending it to the BTS, right? So I wonder how this can be >> useful given that we would need to see real IP packets coming to the >> SSGN that we tunnel into GTP. > > Fair enough. The use-case I am looking at involves PGW load-testing > where the simulated load is generated locally on the SGSN so it _is_ > seeing IP packets and the SNDCP is left out altogether. Perhaps this is > too pathological to warrant messing with the upstream driver... I don't > know: the symmetry does not cost much even if it's of limited use. Sounds reasonable. I'll review change with that in mind next week. Andreas > Couldn't the SNDCP theoretically be a separate node and push IP packets > to the SGSN, thus making this useful? Perhaps it's a stretch... > > /Jonas
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
- On Feb 6, 2017, at 12:08 PM, pablo pa...@netfilter.org wrote: > Hi Jonas, > > On Fri, Feb 03, 2017 at 10:12:31AM +0100, Jonas Bonn wrote: >> The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP >> contexts based on the incoming packets _destination_ address. If we >> want to write an SGSN, then we want to be idenityfing PDP contexts >> based on _source_ address. >> >> This patch adds a "flags" argument at GTP-link creation time to specify >> whether we are on the GGSN or SGSN side of the tunnel; this flag is then >> used to determine which part of the IP packet to use in determining >> the PDP context. > > So far the implementation that I saw in osmocom relies on userspace code > to tunnel data from ME to the SSGN/SGW running on the base station. > > The data we get from GGSN -> SGSN needs to be places into a SN-PDU (via > SNDCP) when sending it to the BTS, right? So I wonder how this can be > useful given that we would need to see real IP packets coming to the > SSGN that we tunnel into GTP. Uhm, no, absolutely not. The SGSN is not seeing IP packets, it's seeing stuff that is supposed to put into GTP tunnels. The only instance in an EPC (apart from a UE), that knows about the payload content of a GTP tunnel is the GGSN/PGW. Even with Rel 13 Non IP bearers and CIoT optimization, the interpretation of the content of an bearer is only done at the PGW. Andreas > > Thanks!
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Jonas, On Mon, Feb 06, 2017 at 02:33:07PM +0100, Jonas Bonn wrote: > Fair enough. The use-case I am looking at involves PGW load-testing where > the simulated load is generated locally on the SGSN so it _is_ seeing IP > packets and the SNDCP is left out altogether. Ok, it would have been useful to document that test-only feature in the changelog and/or code. Like "support simulated RAN-side tunnels" or "support SGSN/S-GW simulation". > Perhaps this is too pathological to warrant messing with the upstream > driver... I don't know: the symmetry does not cost much even if it's > of limited use. There are plenty of features in the mainline kernel related to testing, see pktgen for example. So I think if it doesn't impose complexity, performance issues or stretches the existing architecture, I think there's no reason to keep it out. Looking at the code, I think the one conditional on the flags is not going to kill significant performance of the "normal" use case. But that's of course just guessing, without any benchmark to back that up. Semantically, I'm not sure if the FLAGS and the re-use of the SGSN_ADDRESS TLV is the best choice. If suddenly the meaning of the TLV is "Peer GSN Address" then it should be called that way. We could have a #define SGSN_ADDRESS to GSN_PEER_ADDRESS to make old code compile. I'll let Pablo respond to this as he came up with the netlink interface, as far as I can remember :) Also, like with any changes to the kernel and netlink interface code, I think we should always mandate similar changes to be made to libgtpnl so the feature can actually be used/tested with the standard tools/utilities available to anyone. > Couldn't the SNDCP theoretically be a separate node and push IP packets to > the SGSN, thus making this useful? Perhaps it's a stretch... No, because you would introduce an hop (or even two!) at the IP level, breaking * the notion of who the remote IP address is (remote poin-to-point address) of the PDP context * packets get modified (TTL decrement, ...) where they are not supposed to * you suddenly might get TTL exceeded, dest unreachable, ...) out of nowhere into your user IP * you introduce serious security issues by having the kernel IP routing code between the outer IP (the operator RAN/core network) and the inner user IP payload. 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 1/1] gtp: support SGSN-side tunnels
Dear Jonas, On Fri, Feb 03, 2017 at 10:12:31AM +0100, Jonas Bonn wrote: > The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP > contexts based on the incoming packets _destination_ address. If we > want to write an SGSN, then we want to be idenityfing PDP contexts > based on _source_ address. A SGSN should never see the "native" User-IP payload. It either has a Gb interface towards the BSS which has a BSSGP/NS/UDP/IP protocol stacking (for GERAN) or an IuUP or GTP stacking (for UTRAN). The user-ip tunnel (PDP context) exists between the mobile station and the GGSN. Any intermediate nodes (BTS, BSC, NodeB, RNC, SGSN, ...) do not appear as intermediate IP Layer nodes in that User IP tunnel, but only serve to transparently pass the user-ip inside that tunnel between the two tunnel end-points. As such, I am very surprised by your patch. Exposing the User IP to the Linux network stack in the SGSN seems to be a severe layering violation and contradict everything I know about 3GPP network architecture. But maybe I'm missing something here? Please explain. The only SGSN-level user plane acceleration that I can think of is quite different: For an UMTS SGSN that only supports IuPS, and only supports IuPS over IP (which is a sub-class of a sub-class of all use cases), what would make sense is some Kernel-level support to map from one GTP socket/tunnel to another GTP socket/tunnel based on the TEIDs. So basically you have a GTP tunnel on the RAN side and another GTP tunnel on the CN side, without any decapsultaion. The TEIDs on both sides *could* be identical, or *cold* be separate, as a matter of implementation policy. The IP addresses /port numbers on both sides will in almost all non-laboratory use cases be separate, as an operator typically doesn't want a transparent/routed IP network between the RAN and Core Network (CN). So the GTP tunnels between RNC/hNodeB/heNodeB on the RAN side get mapped 1:1 to GTP tunnels between SGSN and GGSN on the CN side. However, as no encapsulation/decapsulation is performed, this is outside of the scope of the current kernel GTP tunneling module. Rather, it's more something similar to static NAT between two pairs of addresses. 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 1/1] gtp: support SGSN-side tunnels
Hi Pablo, On 02/06/2017 12:08 PM, Pablo Neira Ayuso wrote: Hi Jonas, On Fri, Feb 03, 2017 at 10:12:31AM +0100, Jonas Bonn wrote: The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP contexts based on the incoming packets _destination_ address. If we want to write an SGSN, then we want to be idenityfing PDP contexts based on _source_ address. This patch adds a "flags" argument at GTP-link creation time to specify whether we are on the GGSN or SGSN side of the tunnel; this flag is then used to determine which part of the IP packet to use in determining the PDP context. So far the implementation that I saw in osmocom relies on userspace code to tunnel data from ME to the SSGN/SGW running on the base station. The data we get from GGSN -> SGSN needs to be places into a SN-PDU (via SNDCP) when sending it to the BTS, right? So I wonder how this can be useful given that we would need to see real IP packets coming to the SSGN that we tunnel into GTP. Fair enough. The use-case I am looking at involves PGW load-testing where the simulated load is generated locally on the SGSN so it _is_ seeing IP packets and the SNDCP is left out altogether. Perhaps this is too pathological to warrant messing with the upstream driver... I don't know: the symmetry does not cost much even if it's of limited use. Couldn't the SNDCP theoretically be a separate node and push IP packets to the SGSN, thus making this useful? Perhaps it's a stretch... /Jonas
Re: [PATCH 1/1] gtp: support SGSN-side tunnels
Hi Jonas, On Fri, Feb 03, 2017 at 10:12:31AM +0100, Jonas Bonn wrote: > The GTP-tunnel driver is explicitly GGSN-side as it searches for PDP > contexts based on the incoming packets _destination_ address. If we > want to write an SGSN, then we want to be idenityfing PDP contexts > based on _source_ address. > > This patch adds a "flags" argument at GTP-link creation time to specify > whether we are on the GGSN or SGSN side of the tunnel; this flag is then > used to determine which part of the IP packet to use in determining > the PDP context. So far the implementation that I saw in osmocom relies on userspace code to tunnel data from ME to the SSGN/SGW running on the base station. The data we get from GGSN -> SGSN needs to be places into a SN-PDU (via SNDCP) when sending it to the BTS, right? So I wonder how this can be useful given that we would need to see real IP packets coming to the SSGN that we tunnel into GTP. Thanks!