* [PATCH 1/1] gtp: support SGSN-side tunnels @ 2017-02-03 9:12 Jonas Bonn 2017-02-06 11:08 ` Pablo Neira Ayuso ` (4 more replies) 0 siblings, 5 replies; 26+ messages in thread From: Jonas Bonn @ 2017-02-03 9:12 UTC (permalink / raw) To: laforge, pablo, netdev; +Cc: Jonas Bonn 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 <jonas@southpole.se> --- 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 int flags; unsigned int hash_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", &iph->daddr); @@ -666,12 +675,23 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, int hashsize, err, fd0, fd1; struct gtp_dev *gtp; struct gtp_net *gn; + unsigned int flags; + + if (data[IFLA_GTP_FLAGS]) { + flags = nla_get_u32(data[IFLA_GTP_FLAGS]); + if (flags & ~GTP_FLAGS_MASK) + return -EINVAL; + } else { + flags = 0; + } if (!data[IFLA_GTP_FD0] || !data[IFLA_GTP_FD1]) return -EINVAL; gtp = netdev_priv(dev); + gtp->flags = flags; + fd0 = nla_get_u32(data[IFLA_GTP_FD0]); fd1 = nla_get_u32(data[IFLA_GTP_FD1]); @@ -723,6 +743,7 @@ static const struct nla_policy gtp_policy[IFLA_GTP_MAX + 1] = { [IFLA_GTP_FD0] = { .type = NLA_U32 }, [IFLA_GTP_FD1] = { .type = NLA_U32 }, [IFLA_GTP_PDP_HASHSIZE] = { .type = NLA_U32 }, + [IFLA_GTP_FLAGS] = { .type = NLA_U32 }, }; static int gtp_validate(struct nlattr *tb[], struct nlattr *data[]) diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h index 72a04a0..79037cc 100644 --- a/include/uapi/linux/gtp.h +++ b/include/uapi/linux/gtp.h @@ -19,7 +19,7 @@ enum gtp_attrs { GTPA_LINK, GTPA_VERSION, GTPA_TID, /* for GTPv0 only */ - GTPA_SGSN_ADDRESS, + GTPA_SGSN_ADDRESS, /* Remote GSN, either SGSN or GGSN */ GTPA_MS_ADDRESS, GTPA_FLOW, GTPA_NET_NS_FD, diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index ccde456..a446e7b 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -533,11 +533,16 @@ enum { #define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) /* GTP section */ + +#define GTP_FLAGS_SGSN (1U << 0) +#define GTP_FLAGS_MASK (GTP_FLAGS_SGSN) + enum { IFLA_GTP_UNSPEC, IFLA_GTP_FD0, IFLA_GTP_FD1, IFLA_GTP_PDP_HASHSIZE, + IFLA_GTP_FLAGS, __IFLA_GTP_MAX, }; #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1) -- 2.9.3 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-03 9:12 [PATCH 1/1] gtp: support SGSN-side tunnels Jonas Bonn @ 2017-02-06 11:08 ` Pablo Neira Ayuso 2017-02-06 13:33 ` Jonas Bonn 2017-02-06 17:25 ` Andreas Schultz 2017-02-06 13:44 ` Harald Welte ` (3 subsequent siblings) 4 siblings, 2 replies; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-02-06 11:08 UTC (permalink / raw) To: Jonas Bonn; +Cc: laforge, netdev 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! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-06 11:08 ` Pablo Neira Ayuso @ 2017-02-06 13:33 ` Jonas Bonn 2017-02-06 14:16 ` Harald Welte ` (2 more replies) 2017-02-06 17:25 ` Andreas Schultz 1 sibling, 3 replies; 26+ messages in thread From: Jonas Bonn @ 2017-02-06 13:33 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: laforge, netdev 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-06 13:33 ` Jonas Bonn @ 2017-02-06 14:16 ` Harald Welte 2017-02-06 18:15 ` Pablo Neira Ayuso 2017-02-06 17:27 ` Andreas Schultz 2017-02-06 17:56 ` Pablo Neira Ayuso 2 siblings, 1 reply; 26+ messages in thread From: Harald Welte @ 2017-02-06 14:16 UTC (permalink / raw) To: Jonas Bonn; +Cc: Pablo Neira Ayuso, netdev 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 Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-06 14:16 ` Harald Welte @ 2017-02-06 18:15 ` Pablo Neira Ayuso 0 siblings, 0 replies; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-02-06 18:15 UTC (permalink / raw) To: Jonas Bonn, netdev; +Cc: Harald Welte 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! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-06 13:33 ` Jonas Bonn 2017-02-06 14:16 ` Harald Welte @ 2017-02-06 17:27 ` Andreas Schultz 2017-02-06 17:56 ` Pablo Neira Ayuso 2 siblings, 0 replies; 26+ messages in thread From: Andreas Schultz @ 2017-02-06 17:27 UTC (permalink / raw) To: Jonas Bonn; +Cc: pablo, laforge, netdev 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 jonas@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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-06 13:33 ` Jonas Bonn 2017-02-06 14:16 ` Harald Welte 2017-02-06 17:27 ` Andreas Schultz @ 2017-02-06 17:56 ` Pablo Neira Ayuso 2 siblings, 0 replies; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-02-06 17:56 UTC (permalink / raw) To: Jonas Bonn; +Cc: laforge, netdev 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! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-06 11:08 ` Pablo Neira Ayuso 2017-02-06 13:33 ` Jonas Bonn @ 2017-02-06 17:25 ` Andreas Schultz 1 sibling, 0 replies; 26+ messages in thread From: Andreas Schultz @ 2017-02-06 17:25 UTC (permalink / raw) To: pablo; +Cc: Jonas Bonn, laforge, netdev ----- On Feb 6, 2017, at 12:08 PM, pablo pablo@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! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-03 9:12 [PATCH 1/1] gtp: support SGSN-side tunnels Jonas Bonn 2017-02-06 11:08 ` Pablo Neira Ayuso @ 2017-02-06 13:44 ` Harald Welte 2017-02-13 9:25 ` Andreas Schultz ` (2 subsequent siblings) 4 siblings, 0 replies; 26+ messages in thread From: Harald Welte @ 2017-02-06 13:44 UTC (permalink / raw) To: Jonas Bonn; +Cc: pablo, netdev 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 Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-03 9:12 [PATCH 1/1] gtp: support SGSN-side tunnels Jonas Bonn 2017-02-06 11:08 ` Pablo Neira Ayuso 2017-02-06 13:44 ` Harald Welte @ 2017-02-13 9:25 ` Andreas Schultz 2017-02-13 11:16 ` Pablo Neira Ayuso 2017-03-15 16:39 ` Harald Welte 2017-03-21 15:04 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Jonas Bonn 4 siblings, 1 reply; 26+ messages in thread From: Andreas Schultz @ 2017-02-13 9:25 UTC (permalink / raw) To: Jonas Bonn; +Cc: laforge, pablo, netdev 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 jonas@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 <jonas@southpole.se> > --- > > 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 int flags; 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&m=148638992010344&w=2). > unsigned int hash_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", > &iph->daddr); > @@ -666,12 +675,23 @@ static int gtp_newlink(struct net *src_net, struct > net_device *dev, > int hashsize, err, fd0, fd1; > struct gtp_dev *gtp; > struct gtp_net *gn; > + unsigned int flags; > + > + if (data[IFLA_GTP_FLAGS]) { > + flags = nla_get_u32(data[IFLA_GTP_FLAGS]); > + if (flags & ~GTP_FLAGS_MASK) > + return -EINVAL; > + } else { > + flags = 0; > + } > > if (!data[IFLA_GTP_FD0] || !data[IFLA_GTP_FD1]) > return -EINVAL; > > gtp = netdev_priv(dev); > > + gtp->flags = flags; > + > fd0 = nla_get_u32(data[IFLA_GTP_FD0]); > fd1 = nla_get_u32(data[IFLA_GTP_FD1]); > > @@ -723,6 +743,7 @@ static const struct nla_policy gtp_policy[IFLA_GTP_MAX + 1] > = { > [IFLA_GTP_FD0] = { .type = NLA_U32 }, > [IFLA_GTP_FD1] = { .type = NLA_U32 }, > [IFLA_GTP_PDP_HASHSIZE] = { .type = NLA_U32 }, > + [IFLA_GTP_FLAGS] = { .type = NLA_U32 }, > }; > > static int gtp_validate(struct nlattr *tb[], struct nlattr *data[]) > diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h > index 72a04a0..79037cc 100644 > --- a/include/uapi/linux/gtp.h > +++ b/include/uapi/linux/gtp.h > @@ -19,7 +19,7 @@ enum gtp_attrs { > GTPA_LINK, > GTPA_VERSION, > GTPA_TID, /* for GTPv0 only */ > - GTPA_SGSN_ADDRESS, > + GTPA_SGSN_ADDRESS, /* Remote GSN, either SGSN or GGSN */ > GTPA_MS_ADDRESS, > GTPA_FLOW, > GTPA_NET_NS_FD, > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h > index ccde456..a446e7b 100644 > --- a/include/uapi/linux/if_link.h > +++ b/include/uapi/linux/if_link.h > @@ -533,11 +533,16 @@ enum { > #define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) > > /* GTP section */ > + > +#define GTP_FLAGS_SGSN (1U << 0) > +#define GTP_FLAGS_MASK (GTP_FLAGS_SGSN) > + > enum { > IFLA_GTP_UNSPEC, > IFLA_GTP_FD0, > IFLA_GTP_FD1, > IFLA_GTP_PDP_HASHSIZE, > + IFLA_GTP_FLAGS, > __IFLA_GTP_MAX, > }; > #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1) > -- > 2.9.3 Andreas ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-13 9:25 ` Andreas Schultz @ 2017-02-13 11:16 ` Pablo Neira Ayuso 2017-02-13 11:52 ` Andreas Schultz 0 siblings, 1 reply; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-02-13 11:16 UTC (permalink / raw) To: Andreas Schultz; +Cc: Jonas Bonn, laforge, netdev 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 jonas@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 <jonas@southpole.se> > > --- > > > > 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 int flags; > > 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-13 11:16 ` Pablo Neira Ayuso @ 2017-02-13 11:52 ` Andreas Schultz 2017-02-13 14:23 ` Harald Welte 0 siblings, 1 reply; 26+ messages in thread From: Andreas Schultz @ 2017-02-13 11:52 UTC (permalink / raw) To: pablo; +Cc: Jonas Bonn, laforge, netdev Hi, ----- On Feb 13, 2017, at 12:16 PM, pablo pablo@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 jonas@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 <jonas@southpole.se> >> > --- >> > >> > 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 int flags; >> >> 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-13 11:52 ` Andreas Schultz @ 2017-02-13 14:23 ` Harald Welte 0 siblings, 0 replies; 26+ messages in thread From: Harald Welte @ 2017-02-13 14:23 UTC (permalink / raw) To: Andreas Schultz; +Cc: pablo, Jonas Bonn, netdev, osmocom-net-gprs 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 Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-02-03 9:12 [PATCH 1/1] gtp: support SGSN-side tunnels Jonas Bonn ` (2 preceding siblings ...) 2017-02-13 9:25 ` Andreas Schultz @ 2017-03-15 16:39 ` Harald Welte 2017-03-15 17:23 ` Pablo Neira Ayuso 2017-03-21 15:11 ` Jonas Bonn 2017-03-21 15:04 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Jonas Bonn 4 siblings, 2 replies; 26+ messages in thread From: Harald Welte @ 2017-03-15 16:39 UTC (permalink / raw) To: Jonas Bonn; +Cc: pablo, netdev 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 Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6) ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-03-15 16:39 ` Harald Welte @ 2017-03-15 17:23 ` Pablo Neira Ayuso 2017-03-15 19:10 ` Harald Welte 2017-03-21 15:11 ` Jonas Bonn 1 sibling, 1 reply; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-03-15 17:23 UTC (permalink / raw) To: Harald Welte; +Cc: Jonas Bonn, netdev 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-03-15 17:23 ` Pablo Neira Ayuso @ 2017-03-15 19:10 ` Harald Welte 2017-03-15 19:27 ` Harald Welte 2017-03-15 21:42 ` Pablo Neira Ayuso 0 siblings, 2 replies; 26+ messages in thread From: Harald Welte @ 2017-03-15 19:10 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Jonas Bonn, netdev, osmocom-net-gprs 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 Bonn <jonas@southpole.se> Date: 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 <jonas@southpole.se> Signed-off-by: Harald Welte <laforge@gnumonks.org> 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 int hash_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 (!gtp_check_ms(skb, pctx, hdrlen, gtp->role)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); ret = -1; goto out_rcu; @@ -490,7 +494,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) { netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n", &iph->daddr); @@ -665,12 +673,26 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, int hashsize, err, fd0, fd1; struct gtp_dev *gtp; struct gtp_net *gn; + unsigned int role; + + /* The default role is GGSN, and for backwards compatibility + * reasons, the lack of a IFLA_GTP_ROLE IE means that we're + * operating in GGSN role. */ + if (data[IFLA_GTP_ROLE]) { + role = nla_get_u32(data[IFLA_GTP_ROLE]); + if (role != GTP_ROLE_GGSN && + role != GTP_ROLE_SGSN) + return -EINVAL; + } else + role = GTP_ROLE_GGSN; if (!data[IFLA_GTP_FD0] || !data[IFLA_GTP_FD1]) return -EINVAL; gtp = netdev_priv(dev); + gtp->role = role; + fd0 = nla_get_u32(data[IFLA_GTP_FD0]); fd1 = nla_get_u32(data[IFLA_GTP_FD1]); @@ -722,6 +744,7 @@ static const struct nla_policy gtp_policy[IFLA_GTP_MAX + 1] = { [IFLA_GTP_FD0] = { .type = NLA_U32 }, [IFLA_GTP_FD1] = { .type = NLA_U32 }, [IFLA_GTP_PDP_HASHSIZE] = { .type = NLA_U32 }, + [IFLA_GTP_ROLE] = { .type = NLA_U32 }, }; static int gtp_validate(struct nlattr *tb[], struct nlattr *data[]) diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h index 72a04a0e8cce..79037cca6b51 100644 --- a/include/uapi/linux/gtp.h +++ b/include/uapi/linux/gtp.h @@ -19,7 +19,7 @@ enum gtp_attrs { GTPA_LINK, GTPA_VERSION, GTPA_TID, /* for GTPv0 only */ - GTPA_SGSN_ADDRESS, + GTPA_SGSN_ADDRESS, /* Remote GSN, either SGSN or GGSN */ GTPA_MS_ADDRESS, GTPA_FLOW, GTPA_NET_NS_FD, diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 6b13e591abc9..de7bea223749 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -536,11 +536,18 @@ enum { #define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) /* GTP section */ + +enum ifla_gtp_role { + GTP_ROLE_GGSN = 0, + GTP_ROLE_SGSN, +}; + enum { IFLA_GTP_UNSPEC, IFLA_GTP_FD0, IFLA_GTP_FD1, IFLA_GTP_PDP_HASHSIZE, + IFLA_GTP_ROLE, __IFLA_GTP_MAX, }; #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1) -- 2.11.0 -- - Harald Welte <laforge@netfilter.org> http://netfilter.org/ ============================================================================ "Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed." -- Paul Vixie ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-03-15 19:10 ` Harald Welte @ 2017-03-15 19:27 ` Harald Welte 2017-03-15 21:42 ` Pablo Neira Ayuso 1 sibling, 0 replies; 26+ messages in thread From: Harald Welte @ 2017-03-15 19:27 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Jonas Bonn, netdev, osmocom-net-gprs 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 Bonn <jonas@southpole.se> Date: 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 <jonas@southpole.se> Signed-off-by: Harald Welte <laforge@gnumonks.org> --- 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 int hash_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) { netdev_dbg(dev, "no PDP ctx found for %pI4, skip\n", &iph->daddr); @@ -631,13 +640,27 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, { struct gtp_dev *gtp; struct gtp_net *gn; + unsigned int role; int hashsize, err; + /* The default role is GGSN, and for backwards compatibility + * reasons, the lack of a IFLA_GTP_ROLE IE means that we're + * operating in GGSN role. */ + if (data[IFLA_GTP_ROLE]) { + role = nla_get_u32(data[IFLA_GTP_ROLE]); + if (role != GTP_ROLE_GGSN && + role != GTP_ROLE_SGSN) + return -EINVAL; + } else + role = GTP_ROLE_GGSN; + if (!data[IFLA_GTP_FD0] && !data[IFLA_GTP_FD1]) return -EINVAL; gtp = netdev_priv(dev); + gtp->role = role; + err = gtp_encap_enable(gtp, data); if (err < 0) return err; @@ -685,6 +708,7 @@ static const struct nla_policy gtp_policy[IFLA_GTP_MAX + 1] = { [IFLA_GTP_FD0] = { .type = NLA_U32 }, [IFLA_GTP_FD1] = { .type = NLA_U32 }, [IFLA_GTP_PDP_HASHSIZE] = { .type = NLA_U32 }, + [IFLA_GTP_ROLE] = { .type = NLA_U32 }, }; static int gtp_validate(struct nlattr *tb[], struct nlattr *data[]) diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h index 72a04a0e8cce..79037cca6b51 100644 --- a/include/uapi/linux/gtp.h +++ b/include/uapi/linux/gtp.h @@ -19,7 +19,7 @@ enum gtp_attrs { GTPA_LINK, GTPA_VERSION, GTPA_TID, /* for GTPv0 only */ - GTPA_SGSN_ADDRESS, + GTPA_SGSN_ADDRESS, /* Remote GSN, either SGSN or GGSN */ GTPA_MS_ADDRESS, GTPA_FLOW, GTPA_NET_NS_FD, diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 320fc1e747ee..8b405afb2376 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -538,11 +538,18 @@ enum { #define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) /* GTP section */ + +enum ifla_gtp_role { + GTP_ROLE_GGSN = 0, + GTP_ROLE_SGSN, +}; + enum { IFLA_GTP_UNSPEC, IFLA_GTP_FD0, IFLA_GTP_FD1, IFLA_GTP_PDP_HASHSIZE, + IFLA_GTP_ROLE, __IFLA_GTP_MAX, }; #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1) -- 2.11.0 -- - Harald Welte <laforge@netfilter.org> http://netfilter.org/ ============================================================================ "Fragmentation is like classful addressing -- an interesting early architectural error that shows how much experimentation was going on while IP was being designed." -- Paul Vixie ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-03-15 19:10 ` Harald Welte 2017-03-15 19:27 ` Harald Welte @ 2017-03-15 21:42 ` Pablo Neira Ayuso 1 sibling, 0 replies; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-03-15 21:42 UTC (permalink / raw) To: Harald Welte; +Cc: Jonas Bonn, netdev, osmocom-net-gprs 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 1/1] gtp: support SGSN-side tunnels 2017-03-15 16:39 ` Harald Welte 2017-03-15 17:23 ` Pablo Neira Ayuso @ 2017-03-21 15:11 ` Jonas Bonn 1 sibling, 0 replies; 26+ messages in thread From: Jonas Bonn @ 2017-03-21 15:11 UTC (permalink / raw) To: Harald Welte; +Cc: pablo, netdev 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/2] gtp: rename SGSN netlink attribute 2017-02-03 9:12 [PATCH 1/1] gtp: support SGSN-side tunnels Jonas Bonn ` (3 preceding siblings ...) 2017-03-15 16:39 ` Harald Welte @ 2017-03-21 15:04 ` Jonas Bonn 2017-03-21 15:04 ` [PATCH v2 2/2] gtp: support SGSN-side tunnels Jonas Bonn ` (2 more replies) 4 siblings, 3 replies; 26+ messages in thread From: Jonas Bonn @ 2017-03-21 15:04 UTC (permalink / raw) To: laforge, pablo, netdev; +Cc: Jonas Bonn This is a mostly cosmetic rename of the SGSN netlink attribute to the GTP link. The justification for this is that we will be making the module support decapsulation of "downstream" SGSN packets, in which case the netlink parameter actually refers to the upstream GGSN peer. Renaming the parameter makes the relationship clearer. The legacy name is maintained as a define in the header file in order to not break existing code. Signed-off-by: Jonas Bonn <jonas@southpole.se> --- drivers/net/gtp.c | 22 +++++++++++----------- include/uapi/linux/gtp.h | 3 ++- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 50349a9..3806be6 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -56,7 +56,7 @@ struct pdp_ctx { u16 af; struct in_addr ms_addr_ip4; - struct in_addr sgsn_addr_ip4; + struct in_addr peer_addr_ip4; atomic_t tx_seq; struct rcu_head rcu_head; @@ -522,17 +522,17 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev, } rt = ip4_route_output_gtp(sock_net(sk), &fl4, gtp->sock0->sk, - pctx->sgsn_addr_ip4.s_addr); + pctx->peer_addr_ip4.s_addr); if (IS_ERR(rt)) { netdev_dbg(dev, "no route to SSGN %pI4\n", - &pctx->sgsn_addr_ip4.s_addr); + &pctx->peer_addr_ip4.s_addr); dev->stats.tx_carrier_errors++; goto err; } if (rt->dst.dev == dev) { netdev_dbg(dev, "circular route to SSGN %pI4\n", - &pctx->sgsn_addr_ip4.s_addr); + &pctx->peer_addr_ip4.s_addr); dev->stats.collisions++; goto err_rt; } @@ -894,8 +894,8 @@ static void ipv4_pdp_fill(struct pdp_ctx *pctx, struct genl_info *info) { pctx->gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]); pctx->af = AF_INET; - pctx->sgsn_addr_ip4.s_addr = - nla_get_be32(info->attrs[GTPA_SGSN_ADDRESS]); + pctx->peer_addr_ip4.s_addr = + nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]); pctx->ms_addr_ip4.s_addr = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]); @@ -981,13 +981,13 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) switch (pctx->gtp_version) { case GTP_V0: netdev_dbg(dev, "GTPv0-U: new PDP ctx id=%llx ssgn=%pI4 ms=%pI4 (pdp=%p)\n", - pctx->u.v0.tid, &pctx->sgsn_addr_ip4, + pctx->u.v0.tid, &pctx->peer_addr_ip4, &pctx->ms_addr_ip4, pctx); break; case GTP_V1: netdev_dbg(dev, "GTPv1-U: new PDP ctx id=%x/%x ssgn=%pI4 ms=%pI4 (pdp=%p)\n", pctx->u.v1.i_tei, pctx->u.v1.o_tei, - &pctx->sgsn_addr_ip4, &pctx->ms_addr_ip4, pctx); + &pctx->peer_addr_ip4, &pctx->ms_addr_ip4, pctx); break; } @@ -1001,7 +1001,7 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info) if (!info->attrs[GTPA_VERSION] || !info->attrs[GTPA_LINK] || - !info->attrs[GTPA_SGSN_ADDRESS] || + !info->attrs[GTPA_PEER_ADDRESS] || !info->attrs[GTPA_MS_ADDRESS]) return -EINVAL; @@ -1114,7 +1114,7 @@ static int gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq, goto nlmsg_failure; if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) || - nla_put_be32(skb, GTPA_SGSN_ADDRESS, pctx->sgsn_addr_ip4.s_addr) || + nla_put_be32(skb, GTPA_PEER_ADDRESS, pctx->peer_addr_ip4.s_addr) || nla_put_be32(skb, GTPA_MS_ADDRESS, pctx->ms_addr_ip4.s_addr)) goto nla_put_failure; @@ -1267,7 +1267,7 @@ static struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = { [GTPA_LINK] = { .type = NLA_U32, }, [GTPA_VERSION] = { .type = NLA_U32, }, [GTPA_TID] = { .type = NLA_U64, }, - [GTPA_SGSN_ADDRESS] = { .type = NLA_U32, }, + [GTPA_PEER_ADDRESS] = { .type = NLA_U32, }, [GTPA_MS_ADDRESS] = { .type = NLA_U32, }, [GTPA_FLOW] = { .type = NLA_U16, }, [GTPA_NET_NS_FD] = { .type = NLA_U32, }, diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h index 72a04a0..c51ebb0 100644 --- a/include/uapi/linux/gtp.h +++ b/include/uapi/linux/gtp.h @@ -19,7 +19,7 @@ enum gtp_attrs { GTPA_LINK, GTPA_VERSION, GTPA_TID, /* for GTPv0 only */ - GTPA_SGSN_ADDRESS, + GTPA_PEER_ADDRESS, /* Remote GSN peer, either SGSN or GGSN */ GTPA_MS_ADDRESS, GTPA_FLOW, GTPA_NET_NS_FD, @@ -29,5 +29,6 @@ enum gtp_attrs { __GTPA_MAX, }; #define GTPA_MAX (__GTPA_MAX + 1) +#define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS /* maintain legacy attr name */ #endif /* _UAPI_LINUX_GTP_H_ */ -- 2.9.3 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/2] gtp: support SGSN-side tunnels 2017-03-21 15:04 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Jonas Bonn @ 2017-03-21 15:04 ` Jonas Bonn 2017-03-21 15:07 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Pablo Neira Ayuso 2017-03-23 21:16 ` David Miller 2 siblings, 0 replies; 26+ messages in thread From: Jonas Bonn @ 2017-03-21 15:04 UTC (permalink / raw) To: laforge, pablo, netdev; +Cc: Jonas Bonn 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 "role" 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 <jonas@southpole.se> --- drivers/net/gtp.c | 41 +++++++++++++++++++++++++++++++---------- include/uapi/linux/if_link.h | 7 +++++++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 3806be6..b54d1a3 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 int role; unsigned int hash_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 role) { 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 (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 +/* 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 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; } @@ -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->role)) { netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); ret = -1; goto out_rcu; @@ -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->role)) { 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->role == GTP_ROLE_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", &iph->daddr); @@ -666,12 +675,23 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, int hashsize, err, fd0, fd1; struct gtp_dev *gtp; struct gtp_net *gn; + unsigned int role; + + if (data[IFLA_GTP_ROLE]) { + role = nla_get_u32(data[IFLA_GTP_ROLE]); + if (role > GTP_ROLE_SGSN) + return -EINVAL; + } else { + role = GTP_ROLE_GGSN; + } if (!data[IFLA_GTP_FD0] || !data[IFLA_GTP_FD1]) return -EINVAL; gtp = netdev_priv(dev); + gtp->role = role; + fd0 = nla_get_u32(data[IFLA_GTP_FD0]); fd1 = nla_get_u32(data[IFLA_GTP_FD1]); @@ -723,6 +743,7 @@ static const struct nla_policy gtp_policy[IFLA_GTP_MAX + 1] = { [IFLA_GTP_FD0] = { .type = NLA_U32 }, [IFLA_GTP_FD1] = { .type = NLA_U32 }, [IFLA_GTP_PDP_HASHSIZE] = { .type = NLA_U32 }, + [IFLA_GTP_ROLE] = { .type = NLA_U32 }, }; static int gtp_validate(struct nlattr *tb[], struct nlattr *data[]) diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index ccde456..da62d8c 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -533,11 +533,18 @@ enum { #define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1) /* GTP section */ + +enum ifla_gtp_role { + GTP_ROLE_GGSN = 0, + GTP_ROLE_SGSN, +}; + enum { IFLA_GTP_UNSPEC, IFLA_GTP_FD0, IFLA_GTP_FD1, IFLA_GTP_PDP_HASHSIZE, + IFLA_GTP_ROLE, __IFLA_GTP_MAX, }; #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1) -- 2.9.3 ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] gtp: rename SGSN netlink attribute 2017-03-21 15:04 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Jonas Bonn 2017-03-21 15:04 ` [PATCH v2 2/2] gtp: support SGSN-side tunnels Jonas Bonn @ 2017-03-21 15:07 ` Pablo Neira Ayuso 2017-03-21 15:10 ` Jonas Bonn 2017-03-23 21:16 ` David Miller 2 siblings, 1 reply; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-03-21 15:07 UTC (permalink / raw) To: Jonas Bonn; +Cc: laforge, netdev On Tue, Mar 21, 2017 at 04:04:29PM +0100, Jonas Bonn wrote: > diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h > index 72a04a0..c51ebb0 100644 > --- a/include/uapi/linux/gtp.h > +++ b/include/uapi/linux/gtp.h > @@ -19,7 +19,7 @@ enum gtp_attrs { > GTPA_LINK, > GTPA_VERSION, > GTPA_TID, /* for GTPv0 only */ > - GTPA_SGSN_ADDRESS, > + GTPA_PEER_ADDRESS, /* Remote GSN peer, either SGSN or GGSN */ We need here: #define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS for backward compatibility. Anything that is exposed through uapi cannot be changed. > GTPA_MS_ADDRESS, > GTPA_FLOW, > GTPA_NET_NS_FD, > @@ -29,5 +29,6 @@ enum gtp_attrs { > __GTPA_MAX, > }; > #define GTPA_MAX (__GTPA_MAX + 1) > +#define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS /* maintain legacy attr name */ > > #endif /* _UAPI_LINUX_GTP_H_ */ > -- > 2.9.3 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] gtp: rename SGSN netlink attribute 2017-03-21 15:07 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Pablo Neira Ayuso @ 2017-03-21 15:10 ` Jonas Bonn 2017-03-21 15:15 ` Pablo Neira Ayuso 0 siblings, 1 reply; 26+ messages in thread From: Jonas Bonn @ 2017-03-21 15:10 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: laforge, netdev On 03/21/2017 04:07 PM, Pablo Neira Ayuso wrote: > On Tue, Mar 21, 2017 at 04:04:29PM +0100, Jonas Bonn wrote: >> diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h >> index 72a04a0..c51ebb0 100644 >> --- a/include/uapi/linux/gtp.h >> +++ b/include/uapi/linux/gtp.h >> @@ -19,7 +19,7 @@ enum gtp_attrs { >> GTPA_LINK, >> GTPA_VERSION, >> GTPA_TID, /* for GTPv0 only */ >> - GTPA_SGSN_ADDRESS, >> + GTPA_PEER_ADDRESS, /* Remote GSN peer, either SGSN or GGSN */ > We need here: > > #define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS > > for backward compatibility. Anything that is exposed through uapi > cannot be changed. Yes... look a couple of lines further down... >> GTPA_MS_ADDRESS, >> GTPA_FLOW, >> GTPA_NET_NS_FD, >> @@ -29,5 +29,6 @@ enum gtp_attrs { >> __GTPA_MAX, >> }; >> #define GTPA_MAX (__GTPA_MAX + 1) >> +#define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS /* maintain legacy attr name */ ^^^^... there it is! :) /Jonas >> >> #endif /* _UAPI_LINUX_GTP_H_ */ >> -- >> 2.9.3 >> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] gtp: rename SGSN netlink attribute 2017-03-21 15:10 ` Jonas Bonn @ 2017-03-21 15:15 ` Pablo Neira Ayuso 0 siblings, 0 replies; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-03-21 15:15 UTC (permalink / raw) To: Jonas Bonn; +Cc: laforge, netdev On Tue, Mar 21, 2017 at 04:10:26PM +0100, Jonas Bonn wrote: > On 03/21/2017 04:07 PM, Pablo Neira Ayuso wrote: > >On Tue, Mar 21, 2017 at 04:04:29PM +0100, Jonas Bonn wrote: > >>diff --git a/include/uapi/linux/gtp.h b/include/uapi/linux/gtp.h > >>index 72a04a0..c51ebb0 100644 > >>--- a/include/uapi/linux/gtp.h > >>+++ b/include/uapi/linux/gtp.h > >>@@ -19,7 +19,7 @@ enum gtp_attrs { > >> GTPA_LINK, > >> GTPA_VERSION, > >> GTPA_TID, /* for GTPv0 only */ > >>- GTPA_SGSN_ADDRESS, > >>+ GTPA_PEER_ADDRESS, /* Remote GSN peer, either SGSN or GGSN */ > >We need here: > > > >#define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS > > > >for backward compatibility. Anything that is exposed through uapi > >cannot be changed. > > Yes... look a couple of lines further down... > > >> GTPA_MS_ADDRESS, > >> GTPA_FLOW, > >> GTPA_NET_NS_FD, > >>@@ -29,5 +29,6 @@ enum gtp_attrs { > >> __GTPA_MAX, > >> }; > >> #define GTPA_MAX (__GTPA_MAX + 1) > >>+#define GTPA_SGSN_ADDRESS GTPA_PEER_ADDRESS /* maintain legacy attr name */ > > ^^^^... there it is! :) Oh right! Please, move it there to the enum definition, just after the new GTPA_PEER_ADDRESS. We usually do this in other areas of the networking code. You also have to resubmit indicating net-next in your patch subject, ie. [PATCH net-next v3 1/2] gtp: rename SGSN netlink attribute David usually requests that you explicitly indicate the target tree in some way. Thanks! ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] gtp: rename SGSN netlink attribute 2017-03-21 15:04 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Jonas Bonn 2017-03-21 15:04 ` [PATCH v2 2/2] gtp: support SGSN-side tunnels Jonas Bonn 2017-03-21 15:07 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Pablo Neira Ayuso @ 2017-03-23 21:16 ` David Miller 2017-03-23 21:28 ` Pablo Neira Ayuso 2 siblings, 1 reply; 26+ messages in thread From: David Miller @ 2017-03-23 21:16 UTC (permalink / raw) To: jonas; +Cc: laforge, pablo, netdev Can I get a review of these two patches from some GTP experts? Thanks. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] gtp: rename SGSN netlink attribute 2017-03-23 21:16 ` David Miller @ 2017-03-23 21:28 ` Pablo Neira Ayuso 0 siblings, 0 replies; 26+ messages in thread From: Pablo Neira Ayuso @ 2017-03-23 21:28 UTC (permalink / raw) To: David Miller; +Cc: jonas, laforge, netdev On Thu, Mar 23, 2017 at 02:16:22PM -0700, David Miller wrote: > Can I get a review of these two patches from some GTP experts? I asked Jonas to resend indicating [PATCH net-next] in the subject and some minor comestic change. Apart from patches look good so Jonas, you can add this in your follow up version. Acked-by: Pablo Neira Ayuso <pablo@netfilter.org> ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-03-23 21:28 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-03 9:12 [PATCH 1/1] gtp: support SGSN-side tunnels Jonas Bonn 2017-02-06 11:08 ` Pablo Neira Ayuso 2017-02-06 13:33 ` Jonas Bonn 2017-02-06 14:16 ` Harald Welte 2017-02-06 18:15 ` Pablo Neira Ayuso 2017-02-06 17:27 ` Andreas Schultz 2017-02-06 17:56 ` Pablo Neira Ayuso 2017-02-06 17:25 ` Andreas Schultz 2017-02-06 13:44 ` Harald Welte 2017-02-13 9:25 ` Andreas Schultz 2017-02-13 11:16 ` Pablo Neira Ayuso 2017-02-13 11:52 ` Andreas Schultz 2017-02-13 14:23 ` Harald Welte 2017-03-15 16:39 ` Harald Welte 2017-03-15 17:23 ` Pablo Neira Ayuso 2017-03-15 19:10 ` Harald Welte 2017-03-15 19:27 ` Harald Welte 2017-03-15 21:42 ` Pablo Neira Ayuso 2017-03-21 15:11 ` Jonas Bonn 2017-03-21 15:04 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Jonas Bonn 2017-03-21 15:04 ` [PATCH v2 2/2] gtp: support SGSN-side tunnels Jonas Bonn 2017-03-21 15:07 ` [PATCH v2 1/2] gtp: rename SGSN netlink attribute Pablo Neira Ayuso 2017-03-21 15:10 ` Jonas Bonn 2017-03-21 15:15 ` Pablo Neira Ayuso 2017-03-23 21:16 ` David Miller 2017-03-23 21:28 ` Pablo Neira Ayuso
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.