[PATCH net-next v1 0/3] Flow Based GTP Tunneling
This patch series augmented the existing GTP module to support flow based GTP tunneling and modified the openvswitch datapath to support the GTP vport type. A flow based GTP net device enables that, 1) on the RX path, the outer (IP/UDP/GTP) header information could to be stored in the metadata_dst struct, and embedded into the skb. 2) on the TX path, packets are encapsulated following instructions in the metadata_dst field of the skb. A flow based GTP net device can be integrated with Open vSwitch, which allows SDN controllers to program GTP tunnels via Open vSwitch. Open vSwitch changes are based on patch set [PATCH] Add GTP vport based on upstream datapath Example usage with OVS: ovs-vsctl add-port br0 gtp-vport -- set interface gtp-vport \ ofport_request=2 type=gtp option:remote_ip=flow options:key=flow ovs-ofctl add-flow br0 "in_port=2,tun_src=192.168.60.141,tun_id=123, \ actions=set_field:02:00:00:00:00:00->eth_src, \ set_field:ff:ff:ff:ff:ff:ff->eth_dst,LOCAL" ovs-ofctl add-flow br0 \ "in_port=LOCAL,actions=set_tunnel:888, \ set_field:192.168.60.141->tun_dst,2" arp -s 10.1.1.122 02:00:00:00:00:00 Jiannan Ouyang (3): gtp: refactor to support flow-based gtp encap and decap gtp: Support creating flow-based gtp net_device openvswitch: Add GPRS Tunnel Protocol (GTP) vport support drivers/net/gtp.c | 375 ++++++++++++++++++++++++++++++++------- include/net/gtp.h | 8 + include/uapi/linux/openvswitch.h | 1 + net/openvswitch/Kconfig | 10 ++ net/openvswitch/Makefile | 1 + net/openvswitch/vport-gtp.c | 144 +++++++++++++++ 6 files changed, 475 insertions(+), 64 deletions(-) create mode 100644 net/openvswitch/vport-gtp.c -- 2.9.3
If flow-based encap/decap is enabled, a separate code path is created for both packet RX and TX. PDP contexts are not used in flow-based mode since all metadata is maintained in metadata_dst: - for RX, pdp lookup and ms check are bypassed, while metadata_dst is constructed and attached to the skb. - for TX, pdp lookup is bypassed. Packets are encapsulated following instructions specified in metadata_dst. Signed-off-by: Jiannan Ouyang <ouyangj@fb.com> --- drivers/net/gtp.c | 162 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 60 deletions(-) diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 1542e83..5a7b504 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -25,6 +25,7 @@ #include <linux/file.h> #include <linux/gtp.h> +#include <net/dst_metadata.h> #include <net/net_namespace.h> #include <net/protocol.h> #include <net/ip.h> @@ -36,6 +37,8 @@ #include <net/netns/generic.h> #include <net/gtp.h> +#define GTP_PDP_HASHSIZE 1024 + /* An active session for the subscriber. */ struct pdp_ctx { struct hlist_node hlist_tid; @@ -59,7 +62,7 @@ struct pdp_ctx { struct in_addr peer_addr_ip4; struct sock *sk; - struct net_device *dev; + struct net_device *dev; atomic_t tx_seq; struct rcu_head rcu_head; @@ -73,11 +76,15 @@ struct gtp_dev { struct sock *sk1u; struct net_device *dev; + struct net *net; unsigned int role; unsigned int hash_size; struct hlist_head *tid_hash; struct hlist_head *addr_hash; + + unsigned int collect_md; + struct ip_tunnel_info info; }; static unsigned int gtp_net_id __read_mostly; @@ -184,22 +191,23 @@ static bool gtp_check_ms(struct sk_buff *skb, struct pdp_ctx *pctx, return false; } -static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, - unsigned int hdrlen, unsigned int role) +static int gtp_rx(struct gtp_dev *gtp, struct sk_buff *skb, + unsigned int hdrlen, struct sock *sk, + struct metadata_dst *tun_dst) { struct pcpu_sw_netstats *stats; - if (!gtp_check_ms(skb, pctx, hdrlen, role)) { - netdev_dbg(pctx->dev, "No PDP ctx for this MS\n"); - return 1; - } - /* Get rid of the GTP + UDP headers. */ if (iptunnel_pull_header(skb, hdrlen, skb->protocol, - !net_eq(sock_net(pctx->sk), dev_net(pctx->dev)))) + !net_eq(sock_net(sk), dev_net(gtp->dev)))) return -1; - netdev_dbg(pctx->dev, "forwarding packet from GGSN to uplink\n"); + netdev_dbg(gtp->dev, "forwarding packet from GGSN to uplink\n"); + + if (tun_dst) { + skb_dst_set(skb, (struct dst_entry *)tun_dst); + netdev_dbg(gtp->dev, "attaching metadata_dst to skb\n"); + } /* Now that the UDP and the GTP header have been removed, set up the * new network header. This is required by the upper layer to @@ -207,15 +215,16 @@ static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, */ skb_reset_network_header(skb); - skb->dev = pctx->dev; + skb->dev = gtp->dev; - stats = this_cpu_ptr(pctx->dev->tstats); + stats = this_cpu_ptr(gtp->dev->tstats); u64_stats_update_begin(&stats->syncp); stats->rx_packets++; stats->rx_bytes += skb->len; u64_stats_update_end(&stats->syncp); netif_rx(skb); + return 0; } @@ -244,7 +253,12 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) return 1; } - return gtp_rx(pctx, skb, hdrlen, gtp->role); + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->role)) { + netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); + return 1; + } + + return gtp_rx(gtp, skb, hdrlen, pctx->sk, NULL); } static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) @@ -253,6 +267,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) sizeof(struct gtp1_header); struct gtp1_header *gtp1; struct pdp_ctx *pctx; + struct metadata_dst *tun_dst = NULL; if (!pskb_may_pull(skb, hdrlen)) return -1; @@ -280,13 +295,24 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr)); - pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid)); - if (!pctx) { - netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb); - return 1; + if (ip_tunnel_collect_metadata() || gtp->collect_md) { + tun_dst = udp_tun_rx_dst(skb, gtp->sk1u->sk_family, TUNNEL_KEY, + key32_to_tunnel_id(gtp1->tid), 0); + } else { + pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid)); + if (!pctx) { + netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", + skb); + return 1; + } + + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->role)) { + netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); + return 1; + } } - return gtp_rx(pctx, skb, hdrlen, gtp->role); + return gtp_rx(gtp, skb, hdrlen, gtp->sk1u, tun_dst); } static void gtp_encap_destroy(struct sock *sk) @@ -410,7 +436,7 @@ static inline void gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) gtp0->tid = cpu_to_be64(pctx->u.v0.tid); } -static inline void gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) +static inline void gtp1_push_header(struct sk_buff *skb, __be32 tid) { int payload_len = skb->len; struct gtp1_header *gtp1; @@ -426,7 +452,7 @@ static inline void gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) gtp1->flags = 0x30; /* v1, GTP-non-prime. */ gtp1->type = GTP_TPDU; gtp1->length = htons(payload_len); - gtp1->tid = htonl(pctx->u.v1.o_tei); + gtp1->tid = tid; /* TODO: Suppport for extension header, sequence number and N-PDU. * Update the length field if any of them is available. @@ -438,34 +464,17 @@ struct gtp_pktinfo { struct iphdr *iph; struct flowi4 fl4; struct rtable *rt; - struct pdp_ctx *pctx; struct net_device *dev; __be16 gtph_port; }; -static void gtp_push_header(struct sk_buff *skb, struct gtp_pktinfo *pktinfo) -{ - switch (pktinfo->pctx->gtp_version) { - case GTP_V0: - pktinfo->gtph_port = htons(GTP0_PORT); - gtp0_push_header(skb, pktinfo->pctx); - break; - case GTP_V1: - pktinfo->gtph_port = htons(GTP1U_PORT); - gtp1_push_header(skb, pktinfo->pctx); - break; - } -} - static inline void gtp_set_pktinfo_ipv4(struct gtp_pktinfo *pktinfo, struct sock *sk, struct iphdr *iph, - struct pdp_ctx *pctx, struct rtable *rt, - struct flowi4 *fl4, + struct rtable *rt, struct flowi4 *fl4, struct net_device *dev) { pktinfo->sk = sk; pktinfo->iph = iph; - pktinfo->pctx = pctx; pktinfo->rt = rt; pktinfo->fl4 = *fl4; pktinfo->dev = dev; @@ -479,36 +488,58 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev, struct rtable *rt; struct flowi4 fl4; struct iphdr *iph; + struct ip_tunnel_info *info = NULL; + struct sock *sk = NULL; __be16 df; + __be32 tun_id; + __be32 daddr; + u8 gtp_version; int mtu; - /* Read the IP destination address and resolve the PDP context. - * Prepend PDP header with TEI/TID from PDP ctx. - */ iph = ip_hdr(skb); - 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); - return -ENOENT; + + // flow-based GTP1U encap + info = skb_tunnel_info(skb); + if (gtp->collect_md && info && ntohs(info->key.tp_dst) == GTP1U_PORT) { + pctx = NULL; + gtp_version = GTP_V1; + tun_id = tunnel_id_to_key32(info->key.tun_id); + daddr = info->key.u.ipv4.dst; + sk = gtp->sk1u; + + netdev_dbg(dev, "flow-based GTP1U encap: tunnel id %d\n", + be32_to_cpu(tun_id)); + } else { + /* Read the IP destination address and resolve the PDP context. + * Prepend PDP header with TEI/TID from PDP ctx. + */ + 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); + return -ENOENT; + } + netdev_dbg(dev, "found PDP context %p\n", pctx); + + gtp_version = pctx->gtp_version; + tun_id = htonl(pctx->u.v1.o_tei); + daddr = pctx->peer_addr_ip4.s_addr; + sk = pctx->sk; } - netdev_dbg(dev, "found PDP context %p\n", pctx); - rt = ip4_route_output_gtp(&fl4, pctx->sk, pctx->peer_addr_ip4.s_addr); + rt = ip4_route_output_gtp(&fl4, sk, daddr); if (IS_ERR(rt)) { - netdev_dbg(dev, "no route to SSGN %pI4\n", - &pctx->peer_addr_ip4.s_addr); + netdev_dbg(dev, "no route to SSGN %pI4\n", &daddr); dev->stats.tx_carrier_errors++; goto err; } if (rt->dst.dev == dev) { - netdev_dbg(dev, "circular route to SSGN %pI4\n", - &pctx->peer_addr_ip4.s_addr); + netdev_dbg(dev, "circular route to SSGN %pI4\n", &daddr); dev->stats.collisions++; goto err_rt; } @@ -520,7 +551,7 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev, if (df) { mtu = dst_mtu(&rt->dst) - dev->hard_header_len - sizeof(struct iphdr) - sizeof(struct udphdr); - switch (pctx->gtp_version) { + switch (gtp_version) { case GTP_V0: mtu -= sizeof(struct gtp0_header); break; @@ -543,8 +574,18 @@ static int gtp_build_skb_ip4(struct sk_buff *skb, struct net_device *dev, goto err_rt; } - gtp_set_pktinfo_ipv4(pktinfo, pctx->sk, iph, pctx, rt, &fl4, dev); - gtp_push_header(skb, pktinfo); + gtp_set_pktinfo_ipv4(pktinfo, sk, iph, rt, &fl4, dev); + + switch (gtp_version) { + case GTP_V0: + pktinfo->gtph_port = htons(GTP0_PORT); + gtp0_push_header(skb, pctx); + break; + case GTP_V1: + pktinfo->gtph_port = htons(GTP1U_PORT); + gtp1_push_header(skb, tun_id); + break; + } return 0; err_rt: @@ -647,13 +688,14 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, return -EINVAL; gtp = netdev_priv(dev); + gtp->net = src_net; err = gtp_encap_enable(gtp, data); if (err < 0) return err; if (!data[IFLA_GTP_PDP_HASHSIZE]) - hashsize = 1024; + hashsize = GTP_PDP_HASHSIZE; else hashsize = nla_get_u32(data[IFLA_GTP_PDP_HASHSIZE]); -- 2.9.3
Hi Jiannan, On Wed, Jul 12, 2017 at 05:44:53PM -0700, Jiannan Ouyang wrote:
+#define GTP_PDP_HASHSIZE 1024
please don't mix cosmetic cleanups with actual functional code changes.
- struct net_device *dev; + struct net_device *dev;
please don't mix cosmetic cleanups with actual functional code changes.
-static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, - unsigned int hdrlen, unsigned int role) +static int gtp_rx(struct gtp_dev *gtp, struct sk_buff *skb, + unsigned int hdrlen, struct sock *sk, + struct metadata_dst *tun_dst) { struct pcpu_sw_netstats *stats;
- if (!gtp_check_ms(skb, pctx, hdrlen, role)) { - netdev_dbg(pctx->dev, "No PDP ctx for this MS\n"); - return 1; - } - /* Get rid of the GTP + UDP headers. */ if (iptunnel_pull_header(skb, hdrlen, skb->protocol, - !net_eq(sock_net(pctx->sk), dev_net(pctx->dev)))) + !net_eq(sock_net(sk), dev_net(gtp->dev)))) return -1;
- netdev_dbg(pctx->dev, "forwarding packet from GGSN to uplink\n"); + netdev_dbg(gtp->dev, "forwarding packet from GGSN to uplink\n"); + + if (tun_dst) { + skb_dst_set(skb, (struct dst_entry *)tun_dst); + netdev_dbg(gtp->dev, "attaching metadata_dst to skb\n"); + }
/* Now that the UDP and the GTP header have been removed, set up the * new network header. This is required by the upper layer to @@ -207,15 +215,16 @@ static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, */ skb_reset_network_header(skb);
- skb->dev = pctx->dev; + skb->dev = gtp->dev;
- stats = this_cpu_ptr(pctx->dev->tstats); + stats = this_cpu_ptr(gtp->dev->tstats); u64_stats_update_begin(&stats->syncp); stats->rx_packets++; stats->rx_bytes += skb->len; u64_stats_update_end(&stats->syncp);
netif_rx(skb); + return 0; }
@@ -244,7 +253,12 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) return 1; }
- return gtp_rx(pctx, skb, hdrlen, gtp->role); + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->role)) { + netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); + return 1; + } + + return gtp_rx(gtp, skb, hdrlen, pctx->sk, NULL); }
static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) @@ -253,6 +267,7 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb) sizeof(struct gtp1_header); struct gtp1_header *gtp1; struct pdp_ctx *pctx; + struct metadata_dst *tun_dst = NULL;
if (!pskb_may_pull(skb, hdrlen)) return -1; @@ -280,13 +295,24 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
- pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid)); - if (!pctx) { - netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb); - return 1; + if (ip_tunnel_collect_metadata() || gtp->collect_md) { + tun_dst = udp_tun_rx_dst(skb, gtp->sk1u->sk_family, TUNNEL_KEY, + key32_to_tunnel_id(gtp1->tid), 0); + } else { + pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid)); + if (!pctx) { + netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", + skb); + return 1; + } + + if (!gtp_check_ms(skb, pctx, hdrlen, gtp->role)) { + netdev_dbg(gtp->dev, "No PDP ctx for this MS\n"); + return 1; + } }
- return gtp_rx(pctx, skb, hdrlen, gtp->role); + return gtp_rx(gtp, skb, hdrlen, gtp->sk1u, tun_dst); }
static void gtp_encap_destroy(struct sock *sk) @@ -410,7 +436,7 @@ static inline void gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) gtp0->tid = cpu_to_be64(pctx->u.v0.tid); }
-static inline void gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) +static inline void gtp1_push_header(struct sk_buff *skb, __be32 tid) { int payload_len = skb->len; struct gtp1_header *gtp1; @@ -426,7 +452,7 @@ static inline void gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) gtp1->flags = 0x30; /* v1, GTP-non-prime. */ gtp1->type = GTP_TPDU; gtp1->length = htons(payload_len); - gtp1->tid = htonl(pctx->u.v1.o_tei); + gtp1->tid = tid;
/* TODO: Suppport for extension header, sequence number and N-PDU. * Update the length field if any of them is available. @@ -438,34 +464,17 @@ struct gtp_pktinfo { struct iphdr *iph; struct flowi4 fl4; struct rtable *rt; - struct pdp_ctx *pctx; struct net_device *dev; __be16 gtph_port; };
-static void gtp_push_header(struct sk_buff *skb, struct gtp_pktinfo *pktinfo) -{ - switch (pktinfo->pctx->gtp_version) { - case GTP_V0: - pktinfo->gtph_port = htons(GTP0_PORT); - gtp0_push_header(skb, pktinfo->pctx); - break; - case GTP_V1: - pktinfo->gtph_port = htons(GTP1U_PORT); - gtp1_push_header(skb, pktinfo->pctx); - break; - } -} - static inline void gtp_set_pktinfo_ipv4(struct gtp_pktinfo *pktinfo, struct sock *sk, struct iphdr *iph, - struct pdp_ctx *pctx, struct rtable *rt, - struct flowi4 *fl4, + struct rtable *rt, struct flowi4 *fl4, struct net_device *dev) { [...] + __be32 tun_id;
you are breaking GTPv0 functionality here. GTPv0 has 64 bit tunnel identifiers, and this function is called both from GTPv1 and GTPv0 context. This makes me wonder how you did verify that your changes do not break the existing operation with both GTPv0 and GTPv1?
+ // flow-based GTP1U encap + info = skb_tunnel_info(skb); + if (gtp->collect_md && info && ntohs(info->key.tp_dst) == GTP1U_PORT) {
I think it's typically safe to assume that GTP is only operated on standard ports, but it is something you chould/should think about, i.e. whether you want that kind of restriction. In the existing use case, we have the v0/v1 information stored in the per-pdp context structure.
+ tun_id = htonl(pctx->u.v1.o_tei);
here is where you're assuming GTPv1 in two ways from code that is called from both v0 and v1. * you're dereferencing a v1 specific element in the pctx union * you're storing the result in a 32bit variable
gtp = netdev_priv(dev); + gtp->net = src_net;
Isn't this a generic change that's independent of your work on OVS GTP? 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)
Hi Harald,
> On 7/13/17, 12:26 AM, "Harald Welte" <laforge@gnumonks.org> wrote:
>·
> > static inline void gtp_set_pktinfo_ipv4(struct gtp_pktinfo *pktinfo,
> > struct sock *sk, struct iphdr *iph,
> > - struct pdp_ctx *pctx, struct rtable *rt,
> > - struct flowi4 *fl4,
> > + struct rtable *rt, struct flowi4 *fl4,
> > struct net_device *dev)
> > {
> > [...]
> > + __be32 tun_id;
>·
> you are breaking GTPv0 functionality here. GTPv0 has 64 bit tunnel
> identifiers, and this function is called both from GTPv1 and GTPv0
> context.
>·
> This makes me wonder how you did verify that your changes do not break
> the existing operation with both GTPv0 and GTPv1?
>·
Good catch. I only fully tested the GTPv1 path against oai-cn. Will fix
this and test the GTPv0 path as well.
I had doubts on how this flow-based GTPv1 code path should fit in,
which is why the GTPv0 and the GTPv1 code pieces are mixed in my changes.
Should I explicitly claim that the flow-based change is for GTPv1 only?
> > + // flow-based GTP1U encap
> > + info = skb_tunnel_info(skb);
> > + if (gtp->collect_md && info && ntohs(info->key.tp_dst) == GTP1U_PORT) {
>·
> I think it's typically safe to assume that GTP is only operated on
> standard ports, but it is something you chould/should think about, i.e.
> whether you want that kind of restriction. In the existing use case, we
> have the v0/v1 information stored in the per-pdp context structure.
>·
The reason I’m checking GTP1U_PORT here is to filter GTP1U traffic.
It possible to pass a port number from ovs into the gtp module. I will
investigate how to support programmable port.
> > + tun_id = htonl(pctx->u.v1.o_tei);
>·
> here is where you're assuming GTPv1 in two ways from code that is called
> from both v0 and v1.
> * you're dereferencing a v1 specific element in the pctx union
> * you're storing the result in a 32bit variable
>·
Right, will fix this for GTPv0.
> > gtp = netdev_priv(dev);
> > + gtp->net = src_net;
>·
> Isn't this a generic change that's independent of your work on OVS GTP?
It is meant to be OVS independent. What makes it not? Should I leave
this field un-initialized?
Thanks
-Jiannan
Hi Jiannan,
gtp = netdev_priv(dev); + gtp->net = src_net; · Isn't this a generic change that's independent of your work on OVS GTP?
It is meant to be OVS independent. What makes it not? Should I leave this field un-initialized?
In general, in all FOSS projects I have worked (and particularly the Linux kernel), it is a strict rule that any given patch adresses only one logical change. So if your change is for flow-based "OVS" support in the GTP code, and the "gtp->net = src_net" is a generic change (and not something specifically required by flow/OVS) then it should be a separate patch. Similarly to the cosmetic changes which should be a separate patch. -- - 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)
Hi Jiannan, ----- On Jul 13, 2017, at 2:44 AM, Jiannan Ouyang ouyangj@fb.com wrote: [...]
-static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, - unsigned int hdrlen, unsigned int role) +static int gtp_rx(struct gtp_dev *gtp, struct sk_buff *skb, + unsigned int hdrlen, struct sock *sk, + struct metadata_dst *tun_dst)
Some time ago, there was an extensive discussion about the relation of PDP context and network devices. You are basically reverting one of the changes that was made in that context. I think it is wrong to couple GTP devices and PDP context the way you do here (there are people that disagree, though). The GTP network device of one of two structures owning the PDP context, the other is the GTP socket. GTP network devices and GTP sockets should be strictly separated. The GTP network device owns the IP given to the MS, handles mapping IP's into GTP tunnels (peer GSN + TEIDs) and hands the resulting GTP packets of to the GTP socket for sending. The GTP socket decaps the GTP packet, find the right context and based on information therein passes it to the GTP network device. By separating is that way, you can have MS with overlapping or colliding IP's on the same GTP socket as long as they belong to different GTP network devices. We had a length discussion about whether the above scenario makes sense. I'm not sure if we reached a final verdict, but the 3GPP specifications clearly permit such a setup. Regards Andreas
On Mon, Jul 31, 2017 at 09:21:36AM +0200, Andreas Schultz wrote:
Hi Jiannan,
----- On Jul 13, 2017, at 2:44 AM, Jiannan Ouyang ouyangj@fb.com wrote:
[...]
-static int gtp_rx(struct pdp_ctx *pctx, struct sk_buff *skb, - unsigned int hdrlen, unsigned int role) +static int gtp_rx(struct gtp_dev *gtp, struct sk_buff *skb, + unsigned int hdrlen, struct sock *sk, + struct metadata_dst *tun_dst)
Some time ago, there was an extensive discussion about the relation of PDP context and network devices. You are basically reverting one of the changes that was made in that context. I think it is wrong to couple GTP devices and PDP context the way you do here (there are people that disagree, though).
The GTP network device of one of two structures owning the PDP context, the other is the GTP socket. GTP network devices and GTP sockets should be strictly separated.
The GTP network device owns the IP given to the MS, handles mapping IP's into GTP tunnels (peer GSN + TEIDs) and hands the resulting GTP packets of to the GTP socket for sending. The GTP socket decaps the GTP packet, find the right context and based on information therein passes it to the GTP network device.
By separating is that way, you can have MS with overlapping or colliding IP's on the same GTP socket as long as they belong to different GTP network devices.
We had a length discussion about whether the above scenario makes sense. I'm not sure if we reached a final verdict, but the 3GPP specifications clearly permit such a setup.
We need a netlink interface to retrieve GTP information accordingly, this includes a top-level APN object to represent what you need. That would allow to accomodate all usecases.
Add the gtp_create_flow_based_dev() interface to create flow-based gtp net_device, which sets gtp->collect_md. Under flow-based mode, UDP sockets are created and maintained in kernel. Signed-off-by: Jiannan Ouyang <ouyangj@fb.com> --- drivers/net/gtp.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++- include/net/gtp.h | 8 ++ 2 files changed, 217 insertions(+), 4 deletions(-) diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 5a7b504..09712c9 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -642,9 +642,94 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev) return NETDEV_TX_OK; } +static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize); +static void gtp_hashtable_free(struct gtp_dev *gtp); +static int gtp_encap_enable(struct gtp_dev *gtp, struct nlattr *data[]); + +static int gtp_change_mtu(struct net_device *dev, int new_mtu, bool strict) +{ + int max_mtu = IP_MAX_MTU - dev->hard_header_len - sizeof(struct iphdr) + - sizeof(struct udphdr) - sizeof(struct gtp1_header); + + if (new_mtu < ETH_MIN_MTU) + return -EINVAL; + + if (new_mtu > max_mtu) { + if (strict) + return -EINVAL; + + new_mtu = max_mtu; + } + + dev->mtu = new_mtu; + return 0; +} + +static int gtp_dev_open(struct net_device *dev) +{ + struct gtp_dev *gtp = netdev_priv(dev); + struct net *net = gtp->net; + struct socket *sock1u; + struct socket *sock0; + struct udp_tunnel_sock_cfg tunnel_cfg; + struct udp_port_cfg udp_conf; + int err; + + memset(&udp_conf, 0, sizeof(udp_conf)); + + udp_conf.family = AF_INET; + udp_conf.local_ip.s_addr = htonl(INADDR_ANY); + udp_conf.local_udp_port = htons(GTP1U_PORT); + + err = udp_sock_create(gtp->net, &udp_conf, &sock1u); + if (err < 0) + return err; + + udp_conf.local_udp_port = htons(GTP0_PORT); + err = udp_sock_create(gtp->net, &udp_conf, &sock0); + if (err < 0) + return err; + + memset(&tunnel_cfg, 0, sizeof(tunnel_cfg)); + tunnel_cfg.sk_user_data = gtp; + tunnel_cfg.encap_rcv = gtp_encap_recv; + tunnel_cfg.encap_destroy = gtp_encap_destroy; + tunnel_cfg.encap_type = UDP_ENCAP_GTP0; + setup_udp_tunnel_sock(net, sock0, &tunnel_cfg); + + tunnel_cfg.encap_type = UDP_ENCAP_GTP1U; + + setup_udp_tunnel_sock(net, sock1u, &tunnel_cfg); + + sock_hold(sock0->sk); + sock_hold(sock1u->sk); + + gtp->sk0 = sock0->sk; + gtp->sk1u = sock1u->sk; + + return 0; +} + +static int gtp_dev_stop(struct net_device *dev) +{ + struct gtp_dev *gtp = netdev_priv(dev); + struct sock *sk = gtp->sk1u; + + udp_tunnel_sock_release(gtp->sk0->sk_socket); + udp_tunnel_sock_release(gtp->sk1u->sk_socket); + + udp_sk(sk)->encap_type = 0; + rcu_assign_sk_user_data(sk, NULL); + sock_put(sk); + + return 0; +} + static const struct net_device_ops gtp_netdev_ops = { .ndo_init = gtp_dev_init, .ndo_uninit = gtp_dev_uninit, + .ndo_open = gtp_dev_open, + .ndo_stop = gtp_dev_stop, .ndo_start_xmit = gtp_dev_xmit, .ndo_get_stats64 = ip_tunnel_get_stats64, }; @@ -672,10 +757,6 @@ static void gtp_link_setup(struct net_device *dev) sizeof(struct gtp0_header); } -static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize); -static void gtp_hashtable_free(struct gtp_dev *gtp); -static int gtp_encap_enable(struct gtp_dev *gtp, struct nlattr *data[]); - static int gtp_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[], struct netlink_ext_ack *extack) @@ -780,6 +861,130 @@ static struct rtnl_link_ops gtp_link_ops __read_mostly = { .fill_info = gtp_fill_info, }; +static void init_tnl_info(struct ip_tunnel_info *info, __u16 dst_port) +{ + memset(info, 0, sizeof(*info)); + info->key.tp_dst = htons(dst_port); +} + +static struct gtp_dev *gtp_find_flow_based_dev( + struct net *net) +{ + struct gtp_net *gn = net_generic(net, gtp_net_id); + struct gtp_dev *gtp, *t = NULL; + + list_for_each_entry(gtp, &gn->gtp_dev_list, list) { + if (gtp->collect_md) + t = gtp; + } + + return t; +} + +static int gtp_configure(struct net *net, struct net_device *dev, + const struct ip_tunnel_info *info) +{ + struct gtp_net *gn = net_generic(net, gtp_net_id); + struct gtp_dev *gtp = netdev_priv(dev); + int err; + + gtp->net = net; + gtp->dev = dev; + + if (gtp_find_flow_based_dev(net)) + return -EBUSY; + + dev->netdev_ops = >p_netdev_ops; + dev->priv_destructor = free_netdev; + + dev->hard_header_len = 0; + dev->addr_len = 0; + + /* Zero header length. */ + dev->type = ARPHRD_NONE; + dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST; + + dev->priv_flags |= IFF_NO_QUEUE; + dev->features |= NETIF_F_LLTX; + netif_keep_dst(dev); + + /* Assume largest header, ie. GTPv0. */ + dev->needed_headroom = LL_MAX_HEADER + + sizeof(struct iphdr) + + sizeof(struct udphdr) + + sizeof(struct gtp0_header); + + dst_cache_reset(>p->info.dst_cache); + gtp->info = *info; + gtp->collect_md = true; + + err = gtp_hashtable_new(gtp, GTP_PDP_HASHSIZE); // JO: when to free?? + if (err < 0) { + pr_err("Error gtp_hashtable_new"); + return err; + } + + err = register_netdevice(dev); + if (err) { + pr_err("Error when registering net device"); + return err; + } + + list_add_rcu(>p->list, &gn->gtp_dev_list); + return 0; +} + +struct net_device *gtp_create_flow_based_dev(struct net *net, + const char *name, + u8 name_assign_type, + u16 dst_port) +{ + struct nlattr *tb[IFLA_MAX + 1]; + struct net_device *dev; + struct ip_tunnel_info info; + LIST_HEAD(list_kill); + int err; + + memset(&tb, 0, sizeof(tb)); + dev = rtnl_create_link(net, name, name_assign_type, + >p_link_ops, tb); + if (IS_ERR(dev)) { + pr_err("error rtnl_create_link"); + return dev; + } + + init_tnl_info(&info, dst_port); + err = gtp_configure(net, dev, &info); + if (err < 0) { + pr_err("error gtp_configure"); + free_netdev(dev); + return ERR_PTR(err); + } + + /* openvswitch users expect packet sizes to be unrestricted, + * so set the largest MTU we can. + */ + err = gtp_change_mtu(dev, IP_MAX_MTU, false); + if (err) { + pr_err("error gtp_change_mtu"); + goto err; + } + + err = rtnl_configure_link(dev, NULL); + if (err < 0) { + pr_err("error rtnl_configure_link"); + goto err; + } + + return dev; + +err: + gtp_dellink(dev, &list_kill); + unregister_netdevice_many(&list_kill); + return ERR_PTR(err); +} +EXPORT_SYMBOL_GPL(gtp_create_flow_based_dev); + static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize) { int i; diff --git a/include/net/gtp.h b/include/net/gtp.h index 6398891..a76b26c 100644 --- a/include/net/gtp.h +++ b/include/net/gtp.h @@ -31,4 +31,12 @@ struct gtp1_header { /* According to 3GPP TS 29.060. */ #define GTP1_F_EXTHDR 0x04 #define GTP1_F_MASK 0x07 +#ifdef CONFIG_INET + +struct net_device *gtp_create_flow_based_dev( + struct net *net, const char *name, + u8 name_assign_type, u16 dst_port); + +#endif /*ifdef CONFIG_INET */ + #endif -- 2.9.3
Hi Jiannan, please keep in mind I have zero clue about OVS, so I cannot really comment on what is customary in that context and what not. However, some general review from the GTP point of view below: On Wed, Jul 12, 2017 at 05:44:54PM -0700, Jiannan Ouyang wrote:
Add the gtp_create_flow_based_dev() interface to create flow-based gtp net_device, which sets gtp->collect_md. Under flow-based mode, UDP sockets are created and maintained in kernel.
Signed-off-by: Jiannan Ouyang <ouyangj@fb.com> --- drivers/net/gtp.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++- include/net/gtp.h | 8 ++ 2 files changed, 217 insertions(+), 4 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c index 5a7b504..09712c9 100644 --- a/drivers/net/gtp.c +++ b/drivers/net/gtp.c @@ -642,9 +642,94 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev) return NETDEV_TX_OK; }
+static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize); +static void gtp_hashtable_free(struct gtp_dev *gtp); +static int gtp_encap_enable(struct gtp_dev *gtp, struct nlattr *data[]); + +static int gtp_change_mtu(struct net_device *dev, int new_mtu, bool strict) +{ + int max_mtu = IP_MAX_MTU - dev->hard_header_len - sizeof(struct iphdr) + - sizeof(struct udphdr) - sizeof(struct gtp1_header); + + if (new_mtu < ETH_MIN_MTU) + return -EINVAL; + + if (new_mtu > max_mtu) { + if (strict) + return -EINVAL; + + new_mtu = max_mtu; + } + + dev->mtu = new_mtu; + return 0; +} + +static int gtp_dev_open(struct net_device *dev) +{ + struct gtp_dev *gtp = netdev_priv(dev); + struct net *net = gtp->net; + struct socket *sock1u; + struct socket *sock0; + struct udp_tunnel_sock_cfg tunnel_cfg; + struct udp_port_cfg udp_conf; + int err; + + memset(&udp_conf, 0, sizeof(udp_conf)); + + udp_conf.family = AF_INET; + udp_conf.local_ip.s_addr = htonl(INADDR_ANY); + udp_conf.local_udp_port = htons(GTP1U_PORT); + + err = udp_sock_create(gtp->net, &udp_conf, &sock1u); + if (err < 0) + return err; + + udp_conf.local_udp_port = htons(GTP0_PORT); + err = udp_sock_create(gtp->net, &udp_conf, &sock0); + if (err < 0) + return err;
you're unconditionally binding to both GTP0 and GTP1 UDP ports. This is done selectively based on netlink attributes in the existing "normal" non-OVS kernel code, i.e. the control is left to the user. Is this function is only called/used in the context of OVS? If so, since you explicitly implement only GTPv1, why bind to GTPv0 port?
+ setup_udp_tunnel_sock(net, sock1u, &tunnel_cfg);
even here, you're only setting up the v1 and not v0.
+ /* Assume largest header, ie. GTPv0. */ + dev->needed_headroom = LL_MAX_HEADER + + sizeof(struct iphdr) + + sizeof(struct udphdr) + + sizeof(struct gtp0_header);
... and here you're using headroom for a GTPv0 header, despite (I think) only supporting GTPv1 from this configuration?
+ err = gtp_hashtable_new(gtp, GTP_PDP_HASHSIZE); // JO: when to free??
I think that question about when to free needs to be resolved before any merge. Did you check that it persists even after the device is closed/removed? -- - 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)
Hi Harald,
On 7/13/17, 12:35 AM, "Harald Welte" <laforge@gnumonks.org> wrote:
+static int gtp_dev_open(struct net_device *dev) +{ + struct gtp_dev *gtp = netdev_priv(dev); + struct net *net = gtp->net; + struct socket *sock1u; + struct socket *sock0; + struct udp_tunnel_sock_cfg tunnel_cfg; + struct udp_port_cfg udp_conf; + int err; + + memset(&udp_conf, 0, sizeof(udp_conf)); + + udp_conf.family = AF_INET; + udp_conf.local_ip.s_addr = htonl(INADDR_ANY); + udp_conf.local_udp_port = htons(GTP1U_PORT); + + err = udp_sock_create(gtp->net, &udp_conf, &sock1u); + if (err < 0) + return err; + + udp_conf.local_udp_port = htons(GTP0_PORT); + err = udp_sock_create(gtp->net, &udp_conf, &sock0); + if (err < 0) + return err;
you're unconditionally binding to both GTP0 and GTP1 UDP ports. This is done selectively based on netlink attributes in the existing "normal" non-OVS kernel code, i.e. the control is left to the user.
Is this function is only called/used in the context of OVS? If so, since you explicitly implement only GTPv1, why bind to GTPv0 port?
I had doubts on how this flow-based GTPv1 code path should fit in, which is why the GTPv0 and the GTPv1 code pieces are mixed in my changes. Should I explicitly claim that the flow-based change is for GTPv1 only?
+ setup_udp_tunnel_sock(net, sock1u, &tunnel_cfg);
even here, you're only setting up the v1 and not v0.
same reason as above.
+ /* Assume largest header, ie. GTPv0. */ + dev->needed_headroom = LL_MAX_HEADER + + sizeof(struct iphdr) + + sizeof(struct udphdr) + + sizeof(struct gtp0_header);
... and here you're using headroom for a GTPv0 header, despite (I think) only supporting GTPv1 from this configuration?
Yes, only GTPv1 is supported.
+ err = gtp_hashtable_new(gtp, GTP_PDP_HASHSIZE); // JO: when to free??
I think that question about when to free needs to be resolved before any merge. Did you check that it persists even after the device is closed/removed?
I didn't investigate it. What do you mean by persist? Thanks -Jiannan
Hi Jiannan, On Fri, Jul 14, 2017 at 01:01:34AM +0000, Jiannan Ouyang wrote:
you're unconditionally binding to both GTP0 and GTP1 UDP ports. This is done selectively based on netlink attributes in the existing "normal" non-OVS kernel code, i.e. the control is left to the user.
Is this function is only called/used in the context of OVS? If so, since you explicitly implement only GTPv1, why bind to GTPv0 port?
I had doubts on how this flow-based GTPv1 code path should fit in, which is why the GTPv0 and the GTPv1 code pieces are mixed in my changes.
Well, I know nothing about flow-based paths and OVS, so if you are the one proposing related changes to me as the maintainer, you need to make sure that your changes are consistend and useful within your use case/scenario while making sure that the existing features don't break. If you refactor generic code (used by "classic" GTP tunneling + your new flow based tunneling), and that old code worked with GTPv0 and GTPv1, then your modifications must make sure that they continue to support GTPv0 and v1 in the "classic tunnel" use case. If your new code for flow-based tunneling simply only implements v0, it is fine to me - but then those restrictions must be in the flow-based part only, and things must be consistent. I.e. in this case, for the flow-based tunnel approach you must not bind the v0 port, if you don't handle related packets.
Should I explicitly claim that the flow-based change is for GTPv1 only?
That's definitely important, too - but is not the point I raised (see above).
+ setup_udp_tunnel_sock(net, sock1u, &tunnel_cfg);
even here, you're only setting up the v1 and not v0.
same reason as above.
yes, but if I read your code correctly, this is generic/shared code that will break the existing GTPv0 support in the "classic tunnel" case!
+ /* Assume largest header, ie. GTPv0. */ + dev->needed_headroom = LL_MAX_HEADER + + sizeof(struct iphdr) + + sizeof(struct udphdr) + + sizeof(struct gtp0_header);
... and here you're using headroom for a GTPv0 header, despite (I think) only supporting GTPv1 from this configuration?
Yes, only GTPv1 is supported.
well, then I suggest you don't generate headroom for a v0 header (which is larger) in a v1-only code path :)
+ err = gtp_hashtable_new(gtp, GTP_PDP_HASHSIZE); // JO: when to free??
I think that question about when to free needs to be resolved before any merge. Did you check that it persists even after the device is closed/removed?
I didn't investigate it. What do you mean by persist?
"persist" means "remains allocated after the release of the network device". Whatever you allocate during device creation you must de-allocate on device release. I cannot tell you when exactly (as I'm not familiar with OVS or flow-based tunneling, as indicateD). However, I know for sure we cannot introduce code that looks like it introduces memory leaks to the kernel :) 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)
On 12 July 2017 at 17:44, Jiannan Ouyang <ouyangj@fb.com> wrote:
Add the gtp_create_flow_based_dev() interface to create flow-based gtp net_device, which sets gtp->collect_md. Under flow-based mode, UDP sockets are created and maintained in kernel.
Signed-off-by: Jiannan Ouyang <ouyangj@fb.com> ---
<snip>
+static int gtp_configure(struct net *net, struct net_device *dev, + const struct ip_tunnel_info *info) +{ + struct gtp_net *gn = net_generic(net, gtp_net_id); + struct gtp_dev *gtp = netdev_priv(dev); + int err; + + gtp->net = net; + gtp->dev = dev; + + if (gtp_find_flow_based_dev(net)) + return -EBUSY; + + dev->netdev_ops = >p_netdev_ops; + dev->priv_destructor = free_netdev; + + dev->hard_header_len = 0; + dev->addr_len = 0; + + /* Zero header length. */ + dev->type = ARPHRD_NONE; + dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST; + + dev->priv_flags |= IFF_NO_QUEUE; + dev->features |= NETIF_F_LLTX; + netif_keep_dst(dev); + + /* Assume largest header, ie. GTPv0. */ + dev->needed_headroom = LL_MAX_HEADER + + sizeof(struct iphdr) + + sizeof(struct udphdr) + + sizeof(struct gtp0_header); + + dst_cache_reset(>p->info.dst_cache); + gtp->info = *info; + gtp->collect_md = true; + + err = gtp_hashtable_new(gtp, GTP_PDP_HASHSIZE); // JO: when to free?? + if (err < 0) { + pr_err("Error gtp_hashtable_new"); + return err; + }
Firstly, if this succeeds then the subsequent register_netdevice() call below could still fail, in which case the error handling there free this hashtable. Don't forget that even if this whole function succeeds, the caller may still fail to set up the device and in those error cases everything needs to be freed/cleaned up as well. Then in terms of the overall lifecycle, I guess it's a matter of 1) If there is a hashtable allocated then it must be freed on device close; 2) If the device can be reconfigured, and there is already one of these allocated then it would need to be freed upon reconfiguration. Cheers, Joe
Add OVS_VPORT_TYPE_GTP type and vport-gtp support. Signed-off-by: Jiannan Ouyang <ouyangj@fb.com> --- include/uapi/linux/openvswitch.h | 1 + net/openvswitch/Kconfig | 10 +++ net/openvswitch/Makefile | 1 + net/openvswitch/vport-gtp.c | 144 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+) create mode 100644 net/openvswitch/vport-gtp.c diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h index 156ee4c..82b87b2 100644 --- a/include/uapi/linux/openvswitch.h +++ b/include/uapi/linux/openvswitch.h @@ -213,6 +213,7 @@ enum ovs_vport_type { OVS_VPORT_TYPE_GRE, /* GRE tunnel. */ OVS_VPORT_TYPE_VXLAN, /* VXLAN tunnel. */ OVS_VPORT_TYPE_GENEVE, /* Geneve tunnel. */ + OVS_VPORT_TYPE_GTP, /* GTP tunnel. */ __OVS_VPORT_TYPE_MAX }; diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig index ce94729..d30d0ff 100644 --- a/net/openvswitch/Kconfig +++ b/net/openvswitch/Kconfig @@ -71,3 +71,13 @@ config OPENVSWITCH_GENEVE If you say Y here, then the Open vSwitch will be able create geneve vport. Say N to exclude this support and reduce the binary size. + +config OPENVSWITCH_GTP + tristate "Open vSwitch GTP tunneling support" + depends on OPENVSWITCH + depends on GTP + default OPENVSWITCH + ---help--- + If you say Y here, then the Open vSwitch will be able create gtp vport. + + Say N to exclude this support and reduce the binary size. diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile index 60f8090..d77fcc0 100644 --- a/net/openvswitch/Makefile +++ b/net/openvswitch/Makefile @@ -22,3 +22,4 @@ endif obj-$(CONFIG_OPENVSWITCH_VXLAN)+= vport-vxlan.o obj-$(CONFIG_OPENVSWITCH_GENEVE)+= vport-geneve.o obj-$(CONFIG_OPENVSWITCH_GRE) += vport-gre.o +obj-$(CONFIG_OPENVSWITCH_GTP) += vport-gtp.o diff --git a/net/openvswitch/vport-gtp.c b/net/openvswitch/vport-gtp.c new file mode 100644 index 0000000..ed736ef --- /dev/null +++ b/net/openvswitch/vport-gtp.c @@ -0,0 +1,144 @@ +/* + * Copyright (c) 2017 Facebook, Inc. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/in.h> +#include <linux/ip.h> +#include <linux/net.h> +#include <linux/rculist.h> +#include <linux/udp.h> +#include <linux/if_vlan.h> +#include <linux/module.h> + +#include <net/gtp.h> +#include <net/icmp.h> +#include <net/ip.h> +#include <net/route.h> +#include <net/udp.h> +#include <net/xfrm.h> + +#include "datapath.h" +#include "vport.h" +#include "vport-netdev.h" + +static struct vport_ops ovs_gtp_vport_ops; +/** + * struct gtp_port - Keeps track of open UDP ports + * @dst_port: destination port. + */ +struct gtp_port { + u16 port_no; +}; + +static inline struct gtp_port *gtp_vport(const struct vport *vport) +{ + return vport_priv(vport); +} + +static int gtp_get_options(const struct vport *vport, + struct sk_buff *skb) +{ + struct gtp_port *gtp_port = gtp_vport(vport); + + if (nla_put_u16(skb, OVS_TUNNEL_ATTR_DST_PORT, gtp_port->port_no)) + return -EMSGSIZE; + return 0; +} + +static struct vport *gtp_tnl_create(const struct vport_parms *parms) +{ + struct net *net = ovs_dp_get_net(parms->dp); + struct nlattr *options = parms->options; + struct gtp_port *gtp_port; + struct net_device *dev; + struct vport *vport; + struct nlattr *a; + u16 dst_port; + int err; + + if (!options) { + err = -EINVAL; + goto error; + } + + a = nla_find_nested(options, OVS_TUNNEL_ATTR_DST_PORT); + if (a && nla_len(a) == sizeof(u16)) { + dst_port = nla_get_u16(a); + } else { + /* Require destination port from userspace. */ + err = -EINVAL; + goto error; + } + + vport = ovs_vport_alloc(sizeof(struct gtp_port), + &ovs_gtp_vport_ops, parms); + if (IS_ERR(vport)) + return vport; + + gtp_port = gtp_vport(vport); + gtp_port->port_no = dst_port; + + rtnl_lock(); + dev = gtp_create_flow_based_dev(net, parms->name, + NET_NAME_USER, dst_port); + if (IS_ERR(dev)) { + rtnl_unlock(); + ovs_vport_free(vport); + return ERR_CAST(dev); + } + + err = dev_change_flags(dev, dev->flags | IFF_UP); + if (err < 0) { + rtnl_delete_link(dev); + rtnl_unlock(); + ovs_vport_free(vport); + goto error; + } + + rtnl_unlock(); + return vport; +error: + return ERR_PTR(err); +} + +static struct vport *gtp_create(const struct vport_parms *parms) +{ + struct vport *vport; + + vport = gtp_tnl_create(parms); + if (IS_ERR(vport)) + return vport; + + return ovs_netdev_link(vport, parms->name); +} + +static struct vport_ops ovs_gtp_vport_ops = { + .type = OVS_VPORT_TYPE_GTP, + .create = gtp_create, + .destroy = ovs_netdev_tunnel_destroy, + .get_options = gtp_get_options, + .send = dev_queue_xmit, +}; + +static int __init ovs_gtp_tnl_init(void) +{ + return ovs_vport_ops_register(&ovs_gtp_vport_ops); +} +late_initcall(ovs_gtp_tnl_init); + +static void __exit ovs_gtp_tnl_exit(void) +{ + ovs_vport_ops_unregister(&ovs_gtp_vport_ops); +} +module_exit(ovs_gtp_tnl_exit); + +MODULE_DESCRIPTION("OVS: GTP switching port"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("vport-type-6"); -- 2.9.3
On 12 July 2017 at 17:44, Jiannan Ouyang <ouyangj@fb.com> wrote:
This patch series augmented the existing GTP module to support flow based GTP tunneling and modified the openvswitch datapath to support the GTP vport type.
A flow based GTP net device enables that, 1) on the RX path, the outer (IP/UDP/GTP) header information could to be stored in the metadata_dst struct, and embedded into the skb. 2) on the TX path, packets are encapsulated following instructions in the metadata_dst field of the skb.
A flow based GTP net device can be integrated with Open vSwitch, which allows SDN controllers to program GTP tunnels via Open vSwitch.
Open vSwitch changes are based on patch set [PATCH] Add GTP vport based on upstream datapath
Example usage with OVS:
ovs-vsctl add-port br0 gtp-vport -- set interface gtp-vport \ ofport_request=2 type=gtp option:remote_ip=flow options:key=flow
ovs-ofctl add-flow br0 "in_port=2,tun_src=192.168.60.141,tun_id=123, \ actions=set_field:02:00:00:00:00:00->eth_src, \ set_field:ff:ff:ff:ff:ff:ff->eth_dst,LOCAL"
ovs-ofctl add-flow br0 \ "in_port=LOCAL,actions=set_tunnel:888, \ set_field:192.168.60.141->tun_dst,2"
arp -s 10.1.1.122 02:00:00:00:00:00
Jiannan Ouyang (3): gtp: refactor to support flow-based gtp encap and decap gtp: Support creating flow-based gtp net_device openvswitch: Add GPRS Tunnel Protocol (GTP) vport support
Hi Jiannan, net-next is closed, Dave won't accept patches at this time. Some brief feedback in regards to patch #3, the preference these days is for OVS userspace to use rtnetlink to configure devices in COLLECT_METADATA mode, then attach those devices as regular vport-netdev device type to OVS kernel datapath. I think that should mean that no kernel changes to openvswitch are required for providing GTP vports. Instead of this patch it would require something similar to the IFLA_GRE_COLLECT_METADATA flag which GRE has, but for the GTP devices. The latest OVS master now supports configuring devices in this way, perhaps you could take a look at OVS tree's lib/dpif-netlink-rtnl.c to see how other tunnel devices are configured and see if that makes sense for GTP as well? Cheers, Joe
hi Jiannan, net-next si closed, as it has been pointed out already by Joe. On Wed, Jul 12, 2017 at 05:44:52PM -0700, Jiannan Ouyang wrote:
ovs-ofctl add-flow br0 "in_port=2,tun_src=192.168.60.141,tun_id=123, \ actions=set_field:02:00:00:00:00:00->eth_src, \ set_field:ff:ff:ff:ff:ff:ff->eth_dst,LOCAL"
I'm not familiar with the details here, but does this imply that you are matching on the outer (transport layer) source IP address? If so, please note this is in violation of the 3GPP specifications for GTP, which require explicitly that the TEID is the *only* criteria for matching an encapsulated packet to the tunnel. Basically anyone can send you an encapsulated packet from any random source IP, just as long as the TEID matches a tunnel, it will be decapsulated. This is [presumably] in order to take care of mobility, as the subscribers' phone moves around different MME/S-GW/SGSN, each having different source IP addresses. -- - 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)
On 13 July 2017 at 00:12, Harald Welte <laforge@gnumonks.org> wrote:
hi Jiannan,
net-next si closed, as it has been pointed out already by Joe.
On Wed, Jul 12, 2017 at 05:44:52PM -0700, Jiannan Ouyang wrote:
ovs-ofctl add-flow br0 "in_port=2,tun_src=192.168.60.141,tun_id=123, \ actions=set_field:02:00:00:00:00:00->eth_src, \ set_field:ff:ff:ff:ff:ff:ff->eth_dst,LOCAL"
I'm not familiar with the details here, but does this imply that you are matching on the outer (transport layer) source IP address? If so, please note this is in violation of the 3GPP specifications for GTP, which require explicitly that the TEID is the *only* criteria for matching an encapsulated packet to the tunnel. Basically anyone can send you an encapsulated packet from any random source IP, just as long as the TEID matches a tunnel, it will be decapsulated.
This is [presumably] in order to take care of mobility, as the subscribers' phone moves around different MME/S-GW/SGSN, each having different source IP addresses.
I think that this will be hard to avoid; the several existing tunnel implementations that OVS plugs into all allow matching on the outer addresses/ports, I don't see a good way to restrict it without introducing completely new metadata_dst paths for GTP. I'd prefer not to introduce something like this if we can avoid it; several tunnels currently share all of the same metadata_dst code and that's proven sufficient for all cases so far. If someone wishes to implement the 3GPP standard correctly then they should not create matches like this. In quite a few cases, OVS tends to take the approach that we give the user the tools to do what they need to do, but if they wish to shoot themselves in the foot then that's up to them. We can of course work towards ensuring the OVS userspace guides users in the right direction though.
Hi Harald and Jeo, Thank you for the code review. They are really helpful!
On 7/13/17, 11:14 AM, "Joe Stringer" <joe@ovn.org> wrote:
> On 13 July 2017 at 00:12, Harald Welte <laforge@gnumonks.org> wrote: >> I'm not familiar with the details here, but does this imply that you >> are matching on the outer (transport layer) source IP address? If so, >> please note this is in violation of the 3GPP specifications for GTP, >> which require explicitly that the TEID is the *only* criteria for >> matching an encapsulated packet to the tunnel. Basically anyone can >> send you an encapsulated packet from any random source IP, just as long >> as the TEID matches a tunnel, it will be decapsulated. >> >> This is [presumably] in order to take care of mobility, as the >> subscribers' phone moves around different MME/S-GW/SGSN, each having >> different source IP addresses. > > I think that this will be hard to avoid; the several existing tunnel > implementations that OVS plugs into all allow matching on the outer > addresses/ports, I don't see a good way to restrict it without > introducing completely new metadata_dst paths for GTP. I'd prefer not > to introduce something like this if we can avoid it; several tunnels > currently share all of the same metadata_dst code and that's proven > sufficient for all cases so far. If someone wishes to implement the > 3GPP standard correctly then they should not create matches like this. > In quite a few cases, OVS tends to take the approach that we give the > user the tools to do what they need to do, but if they wish to shoot > themselves in the foot then that's up to them. We can of course work > towards ensuring the OVS userspace guides users in the right direction > though. The flow listed out here is an example for the nature of the match that can be performed, but the actual match rule that is programmed by the control plane should wildcard the tun_src as noted by Harald. It is the control plane’s responsibility to enforce the 3GPP Specifications, e.g. creating flow rules to make TEID the only criteria for matching an encapsulated packet to the tunnel. I will provide a 3GPP compliant example in the next version, thank Harald for pointing it out. -Jiannan
participants (6)
-
Andreas Schultz -
Harald Welte -
Harald Welte -
Jiannan Ouyang -
Joe Stringer -
Pablo Neira Ayuso