Hi, Sorry for smsching this all together, but I couldn't find a logical point to split the patches. Also the order can't really be changed as most patches depend on each other. After this series, the GTP kernel module is functional for IPv4 payload in IPv4 GTP tunnels (for GTPv0 and v1). The libgtnl API was changed without increasing the SO version of the lib. I think this is ok since nothing really uses it (yet) nor has the library been offcially released. Andreas Schultz (16): gtp: remove unused local variable gtp: remove genl_register_family_with_ops for Linux < 3.13 gtp: convert the global gtp_instance_list to a per netns list gtp: select netns based on NL attribute gtp-rtnl: and netns support gtp: add error handling to gtp_newlink gtp: fix the order of error cases in gtp_encap_enable gtp: add socket destroy handler gtp: replace udp encap setup with setup_udp_tunnel_sock gtp: switch to iptunnel framework and no longer depend on real_dev gtp: Split TID handling got GTPv0 and GTPv1 gtp-rtnl: sync GTPA_FLOW nl attribute name from kernel to userspace gtp-rtnl: real_ifname is not long needed, remove it gtp-rtnl: Split tid handling for GTPv0 and GTPv1 list Andreas Schultz as author gtp: add support for replacing PDP contexts gtp.c | 657 ++++++++++++++++++++++++--------------- gtp_nl.h | 3 + libgtnl/AUTHORS | 1 + libgtnl/include/libgtpnl/gtp.h | 6 + libgtnl/include/libgtpnl/gtpnl.h | 3 +- libgtnl/include/linux/gtp_nl.h | 5 +- libgtnl/src/gtp-genl.c | 34 +- libgtnl/src/gtp-rtnl.c | 10 +- libgtnl/src/gtp.c | 44 ++- libgtnl/src/internal.h | 13 +- libgtnl/src/libgtpnl.map | 6 + libgtnl/tools/Makefile.am | 4 +- libgtnl/tools/gtp-link-add.c | 5 +- libgtnl/tools/gtp-tunnel.c | 132 +++++--- libgtnl/tools/gtpnl.c | 35 --- 15 files changed, 613 insertions(+), 345 deletions(-) delete mode 100644 libgtnl/tools/gtpnl.c -- 2.5.0
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/gtp.c b/gtp.c index 7c72eb0..11bda38 100644 --- a/gtp.c +++ b/gtp.c @@ -957,7 +957,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) struct gtp_instance *gti = netdev_priv(dev); struct pdp_ctx *pctx; u16 flow = 0; - u32 gtp_version, link, sgsn_addr, ms_addr, hash_ms, hash_tid; + u32 gtp_version, sgsn_addr, ms_addr, hash_ms, hash_tid; u64 tid; bool found = false; @@ -986,7 +986,6 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) flow = nla_get_u16(info->attrs[GTPA_FLOW]); } - link = nla_get_u32(info->attrs[GTPA_LINK]); sgsn_addr = nla_get_u32(info->attrs[GTPA_SGSN_ADDRESS]); ms_addr = nla_get_u32(info->attrs[GTPA_MS_ADDRESS]); -- 2.5.0
Remove the support for genl_register_family_with_ops for Linux < 3.13. Also reorder the initialization to be more in line with similar modules. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/gtp.c b/gtp.c index 11bda38..7e615f3 100644 --- a/gtp.c +++ b/gtp.c @@ -1321,35 +1321,31 @@ static int __init gtp_init(void) get_random_bytes(>p_h_initval, sizeof(gtp_h_initval)); -#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 13, 0) - err = genl_register_family_with_ops(>p_genl_family, gtp_genl_ops); - if (err < 0) - return err; -#else - err = genl_register_family_with_ops(>p_genl_family, - gtp_genl_ops, - ARRAY_SIZE(gtp_genl_ops)); - if (err < 0) - return err; -#endif err = rtnl_link_register(>p_link_ops); if (err < 0) - goto err1; + goto error_out; + + err = genl_register_family_with_ops(>p_genl_family, gtp_genl_ops); + if (err < 0) + goto unreg_rtnl_link; pr_info("GTP module loaded (pdp ctx size %Zd bytes)\n", sizeof(struct pdp_ctx)); return 0; -err1: + +unreg_rtnl_link: + rtnl_link_unregister(>p_link_ops); + +error_out: pr_err("error loading GTP module loaded\n"); - genl_unregister_family(>p_genl_family); return err; } late_initcall(gtp_init); static void __exit gtp_fini(void) { - rtnl_link_unregister(>p_link_ops); genl_unregister_family(>p_genl_family); + rtnl_link_unregister(>p_link_ops); pr_info("GTP module unloaded\n"); } -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:43PM +0100, Andreas Schultz wrote:
Remove the support for genl_register_family_with_ops for Linux < 3.13. Also reorder the initialization to be more in line with similar modules.
Also applied with minor nitpick, see below.
-err1: + +unreg_rtnl_link: + rtnl_link_unregister(>p_link_ops); +
No need for extra space here, I have mangled the patch here, no problem.
+error_out: pr_err("error loading GTP module loaded\n"); - genl_unregister_family(>p_genl_family); return err; } late_initcall(gtp_init);
static void __exit gtp_fini(void) { - rtnl_link_unregister(>p_link_ops); genl_unregister_family(>p_genl_family); + rtnl_link_unregister(>p_link_ops);
pr_info("GTP module unloaded\n"); } -- 2.5.0
This add basic network namespace support by changing to global gtp_instance_list into a pre namespace list. Before this change all pdp context would be visible from all network namespaces, now only the namespace that they belong too, can see them. Also selectively destroy all gtp devices when a namespace is destroyed. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/gtp.c b/gtp.c index 7e615f3..7c61e61 100644 --- a/gtp.c +++ b/gtp.c @@ -20,13 +20,15 @@ #include <linux/net.h> #include <linux/file.h> +#include <net/net_namespace.h> #include <net/protocol.h> #include <net/ip.h> #include <net/udp.h> #include <net/icmp.h> #include <net/xfrm.h> #include <net/genetlink.h> - +#include <net/netns/generic.h> +# #include "gtp.h" #include "gtp_nl.h" @@ -72,7 +74,11 @@ struct gtp_instance { struct hlist_head *addr_hash; }; -static LIST_HEAD(gtp_instance_list); /* XXX netns */ +static int gtp_net_id __read_mostly; + +struct gtp_net { + struct list_head gtp_instance_list; +}; static inline u32 gtp0_hashfn(u64 tid) { @@ -732,6 +738,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, static int gtp_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[]) { + struct gtp_net *gn = net_generic(src_net, gtp_net_id); struct net_device *real_dev; struct gtp_instance *gti; int hashsize, err, fd0, fd1; @@ -771,7 +778,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, if (err < 0) goto err1; - list_add_rcu(>i->list, >p_instance_list); + list_add_rcu(>i->list, &gn->gtp_instance_list); netdev_dbg(dev, "registered new interface\n"); @@ -941,11 +948,12 @@ static void gtp_encap_disable(struct gtp_instance *gti) sockfd_put(gti->sock0); } -static struct net_device *gtp_find_dev(int ifindex) +static struct net_device *gtp_find_dev(struct net *net, int ifindex) { + struct gtp_net *gn = net_generic(net, gtp_net_id); struct gtp_instance *gti; - list_for_each_entry_rcu(gti, >p_instance_list, list) { + list_for_each_entry_rcu(gti, &gn->gtp_instance_list, list) { if (ifindex == gti->dev->ifindex) return gti->dev; } @@ -1051,6 +1059,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) { + struct net *net = sock_net(skb->sk); struct net_device *dev; if (!info->attrs[GTPA_VERSION] || @@ -1061,7 +1070,7 @@ static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) return -EINVAL; /* Check if there's an existing gtpX device to configure */ - dev = gtp_find_dev(nla_get_u32(info->attrs[GTPA_LINK])); + dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) return -ENODEV; @@ -1070,6 +1079,7 @@ static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) { + struct net *net = sock_net(skb->sk); struct gtp_instance *gti; struct net_device *dev; struct pdp_ctx *pctx; @@ -1098,7 +1108,7 @@ static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) return -EINVAL; /* Check if there's an existing gtpX device to configure */ - dev = gtp_find_dev(nla_get_u32(info->attrs[GTPA_LINK])); + dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) return -ENODEV; @@ -1165,6 +1175,7 @@ nla_put_failure: static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) { + struct net *net = sock_net(skb->sk); struct net_device *dev; struct gtp_instance *gti; struct pdp_ctx *pctx = NULL; @@ -1186,7 +1197,7 @@ static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) } /* Check if there's an existing gtpX device to configure */ - dev = gtp_find_dev(nla_get_u32(info->attrs[GTPA_LINK])); + dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) return -ENODEV; @@ -1249,11 +1260,13 @@ gtp_genl_tunnel_dump(struct sk_buff *skb, struct netlink_callback *cb) unsigned long tid = cb->args[1]; struct gtp_instance *last_gti = (struct gtp_instance *)cb->args[2], *gti; struct pdp_ctx *pctx; + struct net *net = sock_net(skb->sk); + struct gtp_net *gn = net_generic(net, gtp_net_id); if (cb->args[4]) return 0; - list_for_each_entry_rcu(gti, >p_instance_list, list) { + list_for_each_entry_rcu(gti, &gn->gtp_instance_list, list) { if (last_gti && last_gti != gti) continue; else @@ -1315,6 +1328,37 @@ static const struct genl_ops gtp_genl_ops[] = { }, }; +static int __net_init gtp_net_init(struct net *net) +{ + struct gtp_net *gn = net_generic(net, gtp_net_id); + + INIT_LIST_HEAD(&gn->gtp_instance_list); + + return 0; +} + +static void __net_exit gtp_net_exit(struct net *net) +{ + struct gtp_net *gn = net_generic(net, gtp_net_id); + struct gtp_instance *gti; + LIST_HEAD(list); + + rtnl_lock(); + list_for_each_entry_rcu(gti, &gn->gtp_instance_list, list) { + gtp_dellink(gti->dev, &list); + } + + unregister_netdevice_many(&list); + rtnl_unlock(); +} + +static struct pernet_operations gtp_net_ops = { + .init = gtp_net_init, + .exit = gtp_net_exit, + .id = >p_net_id, + .size = sizeof(struct gtp_net), +}; + static int __init gtp_init(void) { int err; @@ -1329,10 +1373,17 @@ static int __init gtp_init(void) if (err < 0) goto unreg_rtnl_link; + err = register_pernet_subsys(>p_net_ops); + if (err < 0) + goto unreg_genl_family; + pr_info("GTP module loaded (pdp ctx size %Zd bytes)\n", sizeof(struct pdp_ctx)); return 0; +unreg_genl_family: + genl_unregister_family(>p_genl_family); + unreg_rtnl_link: rtnl_link_unregister(>p_link_ops); @@ -1344,6 +1395,7 @@ late_initcall(gtp_init); static void __exit gtp_fini(void) { + unregister_pernet_subsys(>p_net_ops); genl_unregister_family(>p_genl_family); rtnl_link_unregister(>p_link_ops); -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:44PM +0100, Andreas Schultz wrote:
This add basic network namespace support by changing to global gtp_instance_list into a pre namespace list. Before this change all pdp context would be visible from all network namespaces, now only the namespace that they belong too, can see them.
Also selectively destroy all gtp devices when a namespace is destroyed.
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 9 deletions(-)
diff --git a/gtp.c b/gtp.c index 7e615f3..7c61e61 100644 --- a/gtp.c +++ b/gtp.c @@ -20,13 +20,15 @@ #include <linux/net.h> #include <linux/file.h>
+#include <net/net_namespace.h> #include <net/protocol.h> #include <net/ip.h> #include <net/udp.h> #include <net/icmp.h> #include <net/xfrm.h> #include <net/genetlink.h> - +#include <net/netns/generic.h> +#
This line above has slipped through, no problem I have fixed this here. Applied.
On Mon, Nov 16, 2015 at 04:06:44PM +0100, Andreas Schultz wrote:
+static void __net_exit gtp_net_exit(struct net *net) +{ + struct gtp_net *gn = net_generic(net, gtp_net_id); + struct gtp_instance *gti; + LIST_HEAD(list); + + rtnl_lock(); + list_for_each_entry_rcu(gti, &gn->gtp_instance_list, list) {
BTW, I have also removed the _rcu here. The instance list is protected by rtnl_lock, so no need for safe read-side interation here.
+ gtp_dellink(gti->dev, &list); + } + + unregister_netdevice_many(&list); + rtnl_unlock(); +}
On Mon, Nov 16, 2015 at 04:06:44PM +0100, Andreas Schultz wrote:
This add basic network namespace support by changing to global gtp_instance_list into a pre namespace list.
Just a hair splitting style remark ... from my ASF days I've adopted the nice, short and simple way of writing comments and log messages, which is to basically use imperatives without naming subjects, all the time: "Add foo. Return bar. Implement baz." (not "This adds foo" or "This function returns bar", etc.) It always makes things shorter and easier to edit, because it loses all the verb suffixes ("add", not "adds"). I'm writing this mainly because following this style helped me a lot, not trying to impose it on others -- carry on, no problems here ;) That said, above log message has English errors:
This add basic network namespace support
"adds"
by changing to global gtp_instance_list into a pre namespace list.
A "to" too many? Is "pre namespace list" something I don't know or could it be described more clearly?
all pdp context
"contexts"
now only the namespace that they belong too, can see them.
"...belong to can see them" (lose an o and the comma) I'm sorry to review the log message and omit the code, but I'm juust out of time for now. More will follow. ~Neels
This permits a split namespace setup where the GTP transport sockets are in one namespace the gtp tunnel interface is in another namespace. The target namespece is selected by the new GTPA_NET_NS_FD NL attributes. It fall back to the netns of the GTP-U sockets if the NL attr is not present. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 41 +++++++++++++++++++++++++++++++++++++---- gtp_nl.h | 1 + 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/gtp.c b/gtp.c index 7c61e61..11f8fad 100644 --- a/gtp.c +++ b/gtp.c @@ -330,6 +330,8 @@ static int gtp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) if (!gti) goto user; + netdev_dbg(gti->dev, "encap_recv %p\n", sk); + switch (udp_sk(sk)->encap_type) { case UDP_ENCAP_GTP0: netdev_dbg(gti->dev, "received GTP0 packet\n"); @@ -738,7 +740,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, static int gtp_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[]) { - struct gtp_net *gn = net_generic(src_net, gtp_net_id); + struct gtp_net *gn; struct net_device *real_dev; struct gtp_instance *gti; int hashsize, err, fd0, fd1; @@ -778,6 +780,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, if (err < 0) goto err1; + gn = net_generic(dev_net(dev), gtp_net_id); list_add_rcu(>i->list, &gn->gtp_instance_list); netdev_dbg(dev, "registered new interface\n"); @@ -845,6 +848,19 @@ static struct rtnl_link_ops gtp_link_ops __read_mostly = { .fill_info = gtp_fill_info, }; +static struct net *gtp_genl_get_net(struct net *src_net, struct nlattr *tb[]) +{ + struct net *net; + /* Examine the link attributes and figure out which + * network namespace we are talking about. + */ + if (tb[GTPA_NET_NS_FD]) + net = get_net_ns_by_fd(nla_get_u32(tb[GTPA_NET_NS_FD])); + else + net = get_net(src_net); + return net; +} + static int gtp_hashtable_new(struct gtp_instance *gti, int hsize) { int i; @@ -893,6 +909,8 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, struct socket *sock0, *sock1u; struct sock *sk; + netdev_dbg(dev, "enable gtp on %d, %d\n", fd_gtp0, fd_gtp1); + sock0 = sockfd_lookup(fd_gtp0, &err); if (sock0 == NULL) { netdev_dbg(dev, "socket fd=%d not found (gtp0)\n", fd_gtp0); @@ -918,6 +936,8 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, goto err2; } + netdev_dbg(dev, "enable gtp on %p, %p\n", sock0, sock1u); + gti->sock0 = sock0; gti->sock1u = sock1u; @@ -1059,7 +1079,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) { - struct net *net = sock_net(skb->sk); + struct net *net; struct net_device *dev; if (!info->attrs[GTPA_VERSION] || @@ -1069,6 +1089,10 @@ static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) !info->attrs[GTPA_TID]) return -EINVAL; + net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); + if (net == NULL) + return -EINVAL; + /* Check if there's an existing gtpX device to configure */ dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) @@ -1079,7 +1103,7 @@ static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) { - struct net *net = sock_net(skb->sk); + struct net *net; struct gtp_instance *gti; struct net_device *dev; struct pdp_ctx *pctx; @@ -1107,6 +1131,10 @@ static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) if (gtp_version == GTP_V1 && tid > UINT_MAX) return -EINVAL; + net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); + if (net == NULL) + return -EINVAL; + /* Check if there's an existing gtpX device to configure */ dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) @@ -1175,7 +1203,7 @@ nla_put_failure: static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) { - struct net *net = sock_net(skb->sk); + struct net *net; struct net_device *dev; struct gtp_instance *gti; struct pdp_ctx *pctx = NULL; @@ -1196,6 +1224,10 @@ static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) return -EINVAL; } + net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); + if (net == NULL) + return -EINVAL; + /* Check if there's an existing gtpX device to configure */ dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) @@ -1304,6 +1336,7 @@ static struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = { [GTPA_SGSN_ADDRESS] = { .type = NLA_NESTED, }, [GTPA_MS_ADDRESS] = { .type = NLA_NESTED, }, [GTPA_FLOW] = { .type = NLA_U16, }, + [GTPA_NET_NS_FD] = { .type = NLA_U32, }, }; static const struct genl_ops gtp_genl_ops[] = { diff --git a/gtp_nl.h b/gtp_nl.h index 7bdd2f5..c20666d 100644 --- a/gtp_nl.h +++ b/gtp_nl.h @@ -40,6 +40,7 @@ enum gtp_attrs { GTPA_SGSN_ADDRESS, GTPA_MS_ADDRESS, GTPA_FLOW, + GTPA_NET_NS_FD, __GTPA_MAX, }; #define GTPA_MAX (__GTPA_MAX + 1) -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:45PM +0100, Andreas Schultz wrote:
This permits a split namespace setup where the GTP transport sockets are in one namespace the gtp tunnel interface is in another namespace.
The target namespece is selected by the new GTPA_NET_NS_FD NL attributes. It fall back to the netns of the GTP-U sockets if the NL attr is not present.
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 41 +++++++++++++++++++++++++++++++++++++---- gtp_nl.h | 1 + 2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/gtp.c b/gtp.c index 7c61e61..11f8fad 100644 --- a/gtp.c +++ b/gtp.c @@ -330,6 +330,8 @@ static int gtp_udp_encap_recv(struct sock *sk, struct sk_buff *skb) if (!gti) goto user;
+ netdev_dbg(gti->dev, "encap_recv %p\n", sk); + switch (udp_sk(sk)->encap_type) { case UDP_ENCAP_GTP0: netdev_dbg(gti->dev, "received GTP0 packet\n"); @@ -738,7 +740,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, static int gtp_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[]) { - struct gtp_net *gn = net_generic(src_net, gtp_net_id); + struct gtp_net *gn; struct net_device *real_dev; struct gtp_instance *gti; int hashsize, err, fd0, fd1; @@ -778,6 +780,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, if (err < 0) goto err1;
+ gn = net_generic(dev_net(dev), gtp_net_id); list_add_rcu(>i->list, &gn->gtp_instance_list);
netdev_dbg(dev, "registered new interface\n"); @@ -845,6 +848,19 @@ static struct rtnl_link_ops gtp_link_ops __read_mostly = { .fill_info = gtp_fill_info, };
+static struct net *gtp_genl_get_net(struct net *src_net, struct nlattr *tb[]) +{ + struct net *net; + /* Examine the link attributes and figure out which + * network namespace we are talking about. + */ + if (tb[GTPA_NET_NS_FD]) + net = get_net_ns_by_fd(nla_get_u32(tb[GTPA_NET_NS_FD]));
I can see this returns ERR_PTR() so...
+ else + net = get_net(src_net); + return net; +} + static int gtp_hashtable_new(struct gtp_instance *gti, int hsize) { int i; @@ -893,6 +909,8 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, struct socket *sock0, *sock1u; struct sock *sk;
+ netdev_dbg(dev, "enable gtp on %d, %d\n", fd_gtp0, fd_gtp1); + sock0 = sockfd_lookup(fd_gtp0, &err); if (sock0 == NULL) { netdev_dbg(dev, "socket fd=%d not found (gtp0)\n", fd_gtp0); @@ -918,6 +936,8 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, goto err2; }
+ netdev_dbg(dev, "enable gtp on %p, %p\n", sock0, sock1u); + gti->sock0 = sock0; gti->sock1u = sock1u;
@@ -1059,7 +1079,7 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info)
static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) { - struct net *net = sock_net(skb->sk); + struct net *net; struct net_device *dev;
if (!info->attrs[GTPA_VERSION] || @@ -1069,6 +1089,10 @@ static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info) !info->attrs[GTPA_TID]) return -EINVAL;
+ net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); + if (net == NULL)
This has to be IS_ERR().
+ return -EINVAL; + /* Check if there's an existing gtpX device to configure */ dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) @@ -1079,7 +1103,7 @@ static int gtp_genl_tunnel_new(struct sk_buff *skb, struct genl_info *info)
static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) { - struct net *net = sock_net(skb->sk); + struct net *net; struct gtp_instance *gti; struct net_device *dev; struct pdp_ctx *pctx; @@ -1107,6 +1131,10 @@ static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) if (gtp_version == GTP_V1 && tid > UINT_MAX) return -EINVAL;
+ net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); + if (net == NULL)
This one too.
+ return -EINVAL; + /* Check if there's an existing gtpX device to configure */ dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK])); if (dev == NULL) @@ -1175,7 +1203,7 @@ nla_put_failure:
static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) { - struct net *net = sock_net(skb->sk); + struct net *net; struct net_device *dev; struct gtp_instance *gti; struct pdp_ctx *pctx = NULL; @@ -1196,6 +1224,10 @@ static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) return -EINVAL; }
+ net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); + if (net == NULL)
And here. BTW, I'd appreciate if you can add netdev_dbg stuff in a separated patch.
----- Original Message -----
From: "Pablo Neira Ayuso" <pablo@soleta.eu> To: "Andreas Schultz" <aschultz@tpip.net> Cc: openbsc@lists.osmocom.org, "Harald Welte" <laforge@gnumonks.org> Sent: Monday, November 16, 2015 6:41:34 PM Subject: Re: [PATCH 04/16] gtp: select netns based on NL attribute
On Mon, Nov 16, 2015 at 04:06:45PM +0100, Andreas Schultz wrote:
[...]
This has to be IS_ERR().
Didn't know that, thanks for the explanation. I'll send a new patch soon.
BTW, I'd appreciate if you can add netdev_dbg stuff in a separated patch.
Will do. Andreas
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- libgtnl/include/libgtpnl/gtp.h | 2 ++ libgtnl/include/libgtpnl/gtpnl.h | 2 +- libgtnl/include/linux/gtp_nl.h | 1 + libgtnl/src/gtp-genl.c | 2 ++ libgtnl/src/gtp-rtnl.c | 8 +++++++- libgtnl/src/gtp.c | 12 ++++++++++++ libgtnl/src/internal.h | 1 + libgtnl/src/libgtpnl.map | 2 ++ 8 files changed, 28 insertions(+), 2 deletions(-) diff --git a/libgtnl/include/libgtpnl/gtp.h b/libgtnl/include/libgtpnl/gtp.h index 10b34d5..fa09d2a 100644 --- a/libgtnl/include/libgtpnl/gtp.h +++ b/libgtnl/include/libgtpnl/gtp.h @@ -8,6 +8,7 @@ struct gtp_tunnel; struct gtp_tunnel *gtp_tunnel_alloc(void); void gtp_tunnel_free(struct gtp_tunnel *t); +void gtp_tunnel_set_ifns(struct gtp_tunnel *t, int ifns); void gtp_tunnel_set_ifidx(struct gtp_tunnel *t, uint32_t ifidx); void gtp_tunnel_set_ms_ip4(struct gtp_tunnel *t, struct in_addr *ms_addr); void gtp_tunnel_set_sgsn_ip4(struct gtp_tunnel *t, struct in_addr *sgsn_addr); @@ -15,6 +16,7 @@ void gtp_tunnel_set_version(struct gtp_tunnel *t, uint32_t version); void gtp_tunnel_set_tid(struct gtp_tunnel *t, uint64_t tid); void gtp_tunnel_set_flowid(struct gtp_tunnel *t, uint16_t flowid); +const int gtp_tunnel_get_ifns(struct gtp_tunnel *t); const uint32_t gtp_tunnel_get_ifidx(struct gtp_tunnel *t); const struct in_addr *gtp_tunnel_get_ms_ip4(struct gtp_tunnel *t); const struct in_addr *gtp_tunnel_get_sgsn_ip4(struct gtp_tunnel *t); diff --git a/libgtnl/include/libgtpnl/gtpnl.h b/libgtnl/include/libgtpnl/gtpnl.h index c4faf6c..3d3fd73 100644 --- a/libgtnl/include/libgtpnl/gtpnl.h +++ b/libgtnl/include/libgtpnl/gtpnl.h @@ -16,7 +16,7 @@ int genl_lookup_family(struct mnl_socket *nl, const char *family); struct in_addr; -int gtp_dev_create(const char *gtp_ifname, const char *real_ifname, +int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname, int fd0, int fd1); int gtp_dev_config(const char *iface, struct in_addr *net, uint32_t prefix); int gtp_dev_destroy(const char *gtp_ifname); diff --git a/libgtnl/include/linux/gtp_nl.h b/libgtnl/include/linux/gtp_nl.h index 0a28046..a8fdf3a 100644 --- a/libgtnl/include/linux/gtp_nl.h +++ b/libgtnl/include/linux/gtp_nl.h @@ -40,6 +40,7 @@ enum gtp_attrs { GTPA_SGSN_ADDRESS, GTPA_MS_ADDRESS, GTPA_FLOWID, /* only for GTPv0 */ + GTPA_NET_NS_FD, __GTPA_MAX, }; #define GTPA_MAX (__GTPA_MAX + 1) diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c index c1f60ab..9e68a30 100644 --- a/libgtnl/src/gtp-genl.c +++ b/libgtnl/src/gtp-genl.c @@ -44,6 +44,8 @@ static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t) { mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version); + if (t->ifns > 0) + mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns); mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx); mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr); mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr); diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c index 22b9430..db54653 100644 --- a/libgtnl/src/gtp-rtnl.c +++ b/libgtnl/src/gtp-rtnl.c @@ -38,6 +38,10 @@ #include "internal.h" +#if !defined(IFLA_LINK_NETNSID) +#define IFLA_LINK_NETNSID (IFLA_PHYS_SWITCH_ID + 1) +#endif + static struct nlmsghdr * gtp_put_nlmsg(char *buf, uint16_t type, uint16_t nl_flags, uint32_t seq) { @@ -104,7 +108,7 @@ static int gtp_dev_talk(struct nlmsghdr *nlh, uint32_t seq) return ret; } -int gtp_dev_create(const char *gtp_ifname, const char *real_ifname, +int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname, int fd0, int fd1) { char buf[MNL_SOCKET_BUFFER_SIZE]; @@ -120,6 +124,8 @@ int gtp_dev_create(const char *gtp_ifname, const char *real_ifname, ifm->ifi_change |= IFF_UP; ifm->ifi_flags |= IFF_UP; + if (dest_ns > 0) + mnl_attr_put_u32(nlh, IFLA_NET_NS_FD, dest_ns); mnl_attr_put_u32(nlh, IFLA_LINK, if_nametoindex(real_ifname)); mnl_attr_put_str(nlh, IFLA_IFNAME, gtp_ifname); nest = mnl_attr_nest_start(nlh, IFLA_LINKINFO); diff --git a/libgtnl/src/gtp.c b/libgtnl/src/gtp.c index 4534091..6e3d473 100644 --- a/libgtnl/src/gtp.c +++ b/libgtnl/src/gtp.c @@ -39,6 +39,12 @@ void gtp_tunnel_free(struct gtp_tunnel *t) } EXPORT_SYMBOL(gtp_tunnel_free); +void gtp_tunnel_set_ifns(struct gtp_tunnel *t, int ifns) +{ + t->ifns = ifns; +} +EXPORT_SYMBOL(gtp_tunnel_set_ifns); + void gtp_tunnel_set_ifidx(struct gtp_tunnel *t, uint32_t ifidx) { t->ifidx = ifidx; @@ -75,6 +81,12 @@ void gtp_tunnel_set_flowid(struct gtp_tunnel *t, uint16_t flowid) } EXPORT_SYMBOL(gtp_tunnel_set_flowid); +const int gtp_tunnel_get_ifns(struct gtp_tunnel *t) +{ + return t->ifns; +} +EXPORT_SYMBOL(gtp_tunnel_get_ifns); + const uint32_t gtp_tunnel_get_ifidx(struct gtp_tunnel *t) { return t->ifidx; diff --git a/libgtnl/src/internal.h b/libgtnl/src/internal.h index 75b3954..68f0135 100644 --- a/libgtnl/src/internal.h +++ b/libgtnl/src/internal.h @@ -13,6 +13,7 @@ #include <netinet/in.h> struct gtp_tunnel { + int ifns; uint32_t ifidx; struct in_addr ms_addr; struct in_addr sgsn_addr; diff --git a/libgtnl/src/libgtpnl.map b/libgtnl/src/libgtpnl.map index 6e69ef8..2368467 100644 --- a/libgtnl/src/libgtpnl.map +++ b/libgtnl/src/libgtpnl.map @@ -15,12 +15,14 @@ global: gtp_tunnel_alloc; gtp_tunnel_free; + gtp_tunnel_set_ifns; gtp_tunnel_set_ifidx; gtp_tunnel_set_ms_ip4; gtp_tunnel_set_sgsn_ip4; gtp_tunnel_set_version; gtp_tunnel_set_tid; gtp_tunnel_set_flowid; + gtp_tunnel_get_ifns; gtp_tunnel_get_ifidx; gtp_tunnel_get_ms_ip4; gtp_tunnel_get_sgsn_ip4; -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:46PM +0100, Andreas Schultz wrote:
diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c index c1f60ab..9e68a30 100644 --- a/libgtnl/src/gtp-genl.c +++ b/libgtnl/src/gtp-genl.c @@ -44,6 +44,8 @@ static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t) { mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version); + if (t->ifns > 0) + mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);
Any reason not to consider descriptor zero as valid? I guess we'll need some flags for gtp_tunnel so we know what we have set here.
mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx); mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr); mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr); diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c index 22b9430..db54653 100644 --- a/libgtnl/src/gtp-rtnl.c +++ b/libgtnl/src/gtp-rtnl.c @@ -38,6 +38,10 @@
#include "internal.h"
+#if !defined(IFLA_LINK_NETNSID) +#define IFLA_LINK_NETNSID (IFLA_PHYS_SWITCH_ID + 1) +#endif
Why do you need this?
----- Original Message -----
From: "Pablo Neira Ayuso" <pablo@soleta.eu> To: "Andreas Schultz" <aschultz@tpip.net> Cc: openbsc@lists.osmocom.org, "Harald Welte" <laforge@gnumonks.org> Sent: Monday, November 16, 2015 6:43:55 PM Subject: Re: [PATCH 05/16] gtp-rtnl: and netns support
On Mon, Nov 16, 2015 at 04:06:46PM +0100, Andreas Schultz wrote:
diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c index c1f60ab..9e68a30 100644 --- a/libgtnl/src/gtp-genl.c +++ b/libgtnl/src/gtp-genl.c @@ -44,6 +44,8 @@ static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t) { mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version); + if (t->ifns > 0) + mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);
Any reason not to consider descriptor zero as valid?
ifns is a file descriptor and fd's are as far as I know always greater zero. So, I didn't see a reason to permit zero there when simply not setting the attribute would serve the same purpose.
I guess we'll need some flags for gtp_tunnel so we know what we have set here.
mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx); mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr); mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr); diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c index 22b9430..db54653 100644 --- a/libgtnl/src/gtp-rtnl.c +++ b/libgtnl/src/gtp-rtnl.c @@ -38,6 +38,10 @@
#include "internal.h"
+#if !defined(IFLA_LINK_NETNSID) +#define IFLA_LINK_NETNSID (IFLA_PHYS_SWITCH_ID + 1) +#endif
Why do you need this?
I'm test compiling the library on an older kernel. I'll remove that.
On Tue, Nov 17, 2015 at 11:09:46AM +0100, Andreas Schultz wrote:
----- Original Message -----
From: "Pablo Neira Ayuso" <pablo@soleta.eu> To: "Andreas Schultz" <aschultz@tpip.net> Cc: openbsc@lists.osmocom.org, "Harald Welte" <laforge@gnumonks.org> Sent: Monday, November 16, 2015 6:43:55 PM Subject: Re: [PATCH 05/16] gtp-rtnl: and netns support
On Mon, Nov 16, 2015 at 04:06:46PM +0100, Andreas Schultz wrote:
diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c index c1f60ab..9e68a30 100644 --- a/libgtnl/src/gtp-genl.c +++ b/libgtnl/src/gtp-genl.c @@ -44,6 +44,8 @@ static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t) { mnl_attr_put_u32(nlh, GTPA_VERSION, t->gtp_version); + if (t->ifns > 0) + mnl_attr_put_u32(nlh, GTPA_NET_NS_FD, t->ifns);
Any reason not to consider descriptor zero as valid?
ifns is a file descriptor and fd's are as far as I know always greater zero. So, I didn't see a reason to permit zero there when simply not setting the attribute would serve the same purpose.
You close(0) and then allocate a descriptor via open(...) and see what it happens. #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> int main(void) { int fd; close(0); fd = open("/etc/passwd", O_RDONLY); printf("fd = %d\n", fd); }
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/gtp.c b/gtp.c index 11f8fad..4f5729e 100644 --- a/gtp.c +++ b/gtp.c @@ -756,8 +756,12 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, if (!tb[IFLA_MTU]) dev->mtu = real_dev->mtu; - else if (dev->mtu > real_dev->mtu) - return -EINVAL; + else if (dev->mtu > real_dev->mtu) { + netdev_dbg(dev, "GTP mtu greater that transport MTU (%d > %d)\n", + dev->mtu, real_dev->mtu); + err = -EINVAL; + goto out_err; + } gti = netdev_priv(dev); gti->real_dev = real_dev; @@ -765,7 +769,9 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, fd0 = nla_get_u32(data[IFLA_GTP_FD0]); fd1 = nla_get_u32(data[IFLA_GTP_FD1]); - gtp_encap_enable(dev, gti, fd0, fd1); + err = gtp_encap_enable(dev, gti, fd0, fd1); + if (err < 0) + goto out_err; if (!data[IFLA_GTP_HASHSIZE]) hashsize = 1024; @@ -774,21 +780,29 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, err = gtp_hashtable_new(gti, hashsize); if (err < 0) - return err; + goto out_encap; err = register_netdevice(dev); - if (err < 0) - goto err1; + if (err < 0) { + netdev_dbg(dev, "failed to register new netdev %d\n", err); + goto out_hashtable; + } gn = net_generic(dev_net(dev), gtp_net_id); list_add_rcu(>i->list, &gn->gtp_instance_list); - netdev_dbg(dev, "registered new interface\n"); + netdev_dbg(dev, "registered new GTP interface\n"); return 0; -err1: - netdev_dbg(dev, "failed to register new netdev %d\n", err); + +out_hashtable: gtp_hashtable_free(gti); + +out_encap: + gtp_encap_disable(gti); + +out_err: + dev_put(real_dev); return err; } -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:47PM +0100, Andreas Schultz wrote:
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/gtp.c b/gtp.c index 11f8fad..4f5729e 100644 --- a/gtp.c +++ b/gtp.c @@ -756,8 +756,12 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
if (!tb[IFLA_MTU]) dev->mtu = real_dev->mtu; - else if (dev->mtu > real_dev->mtu) - return -EINVAL; + else if (dev->mtu > real_dev->mtu) { + netdev_dbg(dev, "GTP mtu greater that transport MTU (%d > %d)\n", + dev->mtu, real_dev->mtu); + err = -EINVAL; + goto out_err;
This is function is using __dev_get_by_index(), so we're not holding a reference on the netdevice here.
+ }
gti = netdev_priv(dev); gti->real_dev = real_dev; [...] +out_err: + dev_put(real_dev); return err; }
----- Original Message -----
From: "Pablo Neira Ayuso" <pablo@soleta.eu> To: "Andreas Schultz" <aschultz@tpip.net> Cc: openbsc@lists.osmocom.org, "Harald Welte" <laforge@gnumonks.org> Sent: Monday, November 16, 2015 6:46:29 PM Subject: Re: [PATCH 06/16] gtp: add error handling to gtp_newlink
On Mon, Nov 16, 2015 at 04:06:47PM +0100, Andreas Schultz wrote:
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/gtp.c b/gtp.c index 11f8fad..4f5729e 100644 --- a/gtp.c +++ b/gtp.c @@ -756,8 +756,12 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
if (!tb[IFLA_MTU]) dev->mtu = real_dev->mtu; - else if (dev->mtu > real_dev->mtu) - return -EINVAL; + else if (dev->mtu > real_dev->mtu) { + netdev_dbg(dev, "GTP mtu greater that transport MTU (%d > %d)\n", + dev->mtu, real_dev->mtu); + err = -EINVAL; + goto out_err;
This is function is using __dev_get_by_index(), so we're not holding a reference on the netdevice here.
But there is a 'dev_hold(real_dev);' right before that if condition. Doesn't that take a reference to the netdevice? Anyway, the conversion to the iptunnel framework makes this code largely obsolete. So I'm going to drop this change. Andreas
+ }
gti = netdev_priv(dev); gti->real_dev = real_dev; [...] +out_err: + dev_put(real_dev); return err; }
On Mon, Nov 16, 2015 at 07:16:58PM +0100, Andreas Schultz wrote:
diff --git a/gtp.c b/gtp.c index 11f8fad..4f5729e 100644 --- a/gtp.c +++ b/gtp.c @@ -756,8 +756,12 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
if (!tb[IFLA_MTU]) dev->mtu = real_dev->mtu; - else if (dev->mtu > real_dev->mtu) - return -EINVAL; + else if (dev->mtu > real_dev->mtu) { + netdev_dbg(dev, "GTP mtu greater that transport MTU (%d > %d)\n", + dev->mtu, real_dev->mtu); + err = -EINVAL; + goto out_err;
This is function is using __dev_get_by_index(), so we're not holding a reference on the netdevice here.
But there is a 'dev_hold(real_dev);' right before that if condition. Doesn't that take a reference to the netdevice?
Ah I see. Right, there is a leak there.
Anyway, the conversion to the iptunnel framework makes this code largely obsolete. So I'm going to drop this change.
OK.
On Mon, Nov 16, 2015 at 04:06:47PM +0100, Andreas Schultz wrote:
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/gtp.c b/gtp.c index 11f8fad..4f5729e 100644 --- a/gtp.c +++ b/gtp.c @@ -756,8 +756,12 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
if (!tb[IFLA_MTU]) dev->mtu = real_dev->mtu; - else if (dev->mtu > real_dev->mtu) - return -EINVAL; + else if (dev->mtu > real_dev->mtu) { + netdev_dbg(dev, "GTP mtu greater that transport MTU (%d > %d)\n",
"than" with n ~Neels
The ordering of the error case exit was wrong and would attempt to release the wrong socket. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtp.c b/gtp.c index 4f5729e..61add82 100644 --- a/gtp.c +++ b/gtp.c @@ -967,10 +967,10 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, sk->sk_user_data = gti; return 0; -err1: - sockfd_put(sock0); err2: sockfd_put(sock1u); +err1: + sockfd_put(sock0); return err; } -- 2.5.0
Add a socket destroy handler and use it to detach and release all resources from an UDP socket. Do not longer pin the socket with a reference. This together with the release handler allows user space to close the socket from underneath us. The destroy handler will handle the rest. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/gtp.c b/gtp.c index 61add82..40a5d1c 100644 --- a/gtp.c +++ b/gtp.c @@ -80,6 +80,8 @@ struct gtp_net { struct list_head gtp_instance_list; }; +static void gtp_encap_disable(struct gtp_instance *gti); + static inline u32 gtp0_hashfn(u64 tid) { u32 *tid32 = (u32 *) &tid; @@ -317,6 +319,16 @@ out_rcu: return ret; } +static void gtp_udp_encap_destroy(struct sock *sk) +{ + struct gtp_instance *gti = sk_to_gti(sk); + + if (gti) { + gtp_encap_disable(gti); + sock_put(sk); + } +} + /* UDP encapsulation receive handler. See net/ipv4/udp.c. * Return codes: 0: success, <0: error, >0: passed up to userspace UDP. */ @@ -401,8 +413,6 @@ static int gtp_dev_init(struct net_device *dev) return 0; } -static void gtp_encap_disable(struct gtp_instance *gti); - static void gtp_dev_uninit(struct net_device *dev) { struct gtp_instance *gti = netdev_priv(dev); @@ -958,15 +968,18 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, sk = gti->sock0->sk; udp_sk(sk)->encap_type = UDP_ENCAP_GTP0; udp_sk(sk)->encap_rcv = gtp_udp_encap_recv; + udp_sk(sk)->encap_destroy = gtp_udp_encap_destroy; sk->sk_user_data = gti; udp_encap_enable(); sk = gti->sock1u->sk; udp_sk(sk)->encap_type = UDP_ENCAP_GTP1U; udp_sk(sk)->encap_rcv = gtp_udp_encap_recv; + udp_sk(sk)->encap_destroy = gtp_udp_encap_destroy; sk->sk_user_data = gti; - return 0; + err = 0; + err2: sockfd_put(sock1u); err1: @@ -976,10 +989,17 @@ err1: static void gtp_encap_disable(struct gtp_instance *gti) { - if (gti->sock1u) - sockfd_put(gti->sock1u); - if (gti->sock0) - sockfd_put(gti->sock0); + if (gti->sock0 && gti->sock0->sk) { + udp_sk(gti->sock0->sk)->encap_type = 0; + rcu_assign_sk_user_data(gti->sock0->sk, NULL); + } + if (gti->sock1u && gti->sock1u->sk) { + udp_sk(gti->sock1u->sk)->encap_type = 0; + rcu_assign_sk_user_data(gti->sock1u->sk, NULL); + } + + gti->sock0 = NULL; + gti->sock1u = NULL; } static struct net_device *gtp_find_dev(struct net *net, int ifindex) -- 2.5.0
setup_udp_tunnel_sock() is doing exactly the same as we did before. So instead of replicating all that, simply use it instead. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/gtp.c b/gtp.c index 40a5d1c..794b459 100644 --- a/gtp.c +++ b/gtp.c @@ -24,6 +24,7 @@ #include <net/protocol.h> #include <net/ip.h> #include <net/udp.h> +#include <net/udp_tunnel.h> #include <net/icmp.h> #include <net/xfrm.h> #include <net/genetlink.h> @@ -931,7 +932,7 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, { int err; struct socket *sock0, *sock1u; - struct sock *sk; + struct udp_tunnel_sock_cfg tuncfg = {NULL}; netdev_dbg(dev, "enable gtp on %d, %d\n", fd_gtp0, fd_gtp1); @@ -965,18 +966,15 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_instance *gti, gti->sock0 = sock0; gti->sock1u = sock1u; - sk = gti->sock0->sk; - udp_sk(sk)->encap_type = UDP_ENCAP_GTP0; - udp_sk(sk)->encap_rcv = gtp_udp_encap_recv; - udp_sk(sk)->encap_destroy = gtp_udp_encap_destroy; - sk->sk_user_data = gti; - udp_encap_enable(); - - sk = gti->sock1u->sk; - udp_sk(sk)->encap_type = UDP_ENCAP_GTP1U; - udp_sk(sk)->encap_rcv = gtp_udp_encap_recv; - udp_sk(sk)->encap_destroy = gtp_udp_encap_destroy; - sk->sk_user_data = gti; + tuncfg.sk_user_data = gti; + tuncfg.encap_rcv = gtp_udp_encap_recv; + tuncfg.encap_destroy = gtp_udp_encap_destroy; + + tuncfg.encap_type = UDP_ENCAP_GTP0; + setup_udp_tunnel_sock(sock_net(gti->sock0->sk), gti->sock0, &tuncfg); + + tuncfg.encap_type = UDP_ENCAP_GTP1U; + setup_udp_tunnel_sock(sock_net(gti->sock1u->sk), gti->sock1u, &tuncfg); err = 0; -- 2.5.0
Send tunnel data on GTP-U socket insead of raw interface and use existing iptunnel helpers for that. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 223 ++++++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 116 insertions(+), 107 deletions(-) diff --git a/gtp.c b/gtp.c index 794b459..2ac6431 100644 --- a/gtp.c +++ b/gtp.c @@ -68,7 +68,6 @@ struct gtp_instance { struct socket *sock1u; struct net_device *dev; - struct net_device *real_dev; unsigned int hash_size; struct hlist_head *tid_hash; @@ -404,7 +403,6 @@ static int gtp_dev_init(struct net_device *dev) { struct gtp_instance *gti = netdev_priv(dev); - dev->flags = IFF_NOARP; gti->dev = dev; dev->tstats = alloc_percpu(struct pcpu_sw_netstats); @@ -424,26 +422,34 @@ static void gtp_dev_uninit(struct net_device *dev) #define IP_UDP_LEN (sizeof(struct iphdr) + sizeof(struct udphdr)) -static struct rtable * -ip4_route_output_gtp(struct net *net, struct flowi4 *fl4, - __be32 daddr, __be32 saddr, __u8 tos, int oif) +static inline void init_gtp_flow(struct flowi4 *fl4, + const struct sock *sk, + __be32 daddr) { memset(fl4, 0, sizeof(*fl4)); - fl4->flowi4_oif = oif; + fl4->flowi4_oif = sk->sk_bound_dev_if; fl4->daddr = daddr; - fl4->saddr = saddr; - fl4->flowi4_tos = tos; - fl4->flowi4_proto = IPPROTO_UDP; + fl4->saddr = inet_sk(sk)->inet_saddr; + fl4->flowi4_tos = RT_CONN_FLAGS(sk); + fl4->flowi4_proto = sk->sk_protocol; +} + +static struct rtable * +ip4_route_output_gtp(struct net *net, struct flowi4 *fl4, + const struct sock *sk, + __be32 daddr) +{ + init_gtp_flow(fl4, sk, daddr); return ip_route_output_key(net, fl4); } static inline void -gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx, int payload_len) +gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) { struct gtp0_header *gtp0; + int payload_len = skb->len; /* ensure there is sufficient headroom */ - skb_cow(skb, sizeof(*gtp0) + IP_UDP_LEN); gtp0 = (struct gtp0_header *) skb_push(skb, sizeof(*gtp0)); gtp0->flags = 0x1e; /* V0, GTP-non-prime */ @@ -457,12 +463,12 @@ gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx, int payload_len) } static inline void -gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx, int payload_len) +gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) { struct gtp1_header *gtp1; + int payload_len = skb->len; /* ensure there is sufficient headroom */ - skb_cow(skb, sizeof(*gtp1) + IP_UDP_LEN); gtp1 = (struct gtp1_header *) skb_push(skb, sizeof(*gtp1)); /* Bits 8 7 6 5 4 3 2 1 @@ -502,6 +508,7 @@ gtp_iptunnel_xmit_stats(int err, struct net_device_stats *err_stats, } struct gtp_pktinfo { + struct sock *sk; union { struct iphdr *iph; struct ipv6hdr *ip6h; @@ -515,10 +522,12 @@ struct gtp_pktinfo { }; static inline void -gtp_set_pktinfo_ipv4(struct gtp_pktinfo *pktinfo, struct iphdr *iph, +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 net_device *dev) { + pktinfo->sk = sk; pktinfo->iph = iph; pktinfo->pctx = pctx; pktinfo->rt = rt; @@ -530,7 +539,7 @@ static int gtp_ip4_prepare_xmit(struct sk_buff *skb, struct net_device *dev, struct gtp_pktinfo *pktinfo) { struct gtp_instance *gti = netdev_priv(dev); - struct inet_sock *inet = inet_sk(gti->sock0->sk); + struct sock *sk; struct iphdr *iph; struct pdp_ctx *pctx; struct rtable *rt; @@ -549,14 +558,23 @@ static int gtp_ip4_prepare_xmit(struct sk_buff *skb, struct net_device *dev, netdev_dbg(dev, "found PDP context %p\n", pctx); /* Obtain route for the new encapsulated GTP packet */ - rt = ip4_route_output_gtp(dev_net(dev), &fl4, - pctx->sgsn_addr.ip4.s_addr, - inet->inet_saddr, 0, - gti->real_dev->ifindex); + switch (pctx->gtp_version) { + case GTP_V0: + sk = gti->sock0->sk; + break; + case GTP_V1: + sk = gti->sock1u->sk; + break; + default: + return -ENOENT; + } + + rt = ip4_route_output_gtp(sock_net(sk), &fl4, + gti->sock0->sk, + pctx->sgsn_addr.ip4.s_addr); if (IS_ERR(rt)) { - netdev_dbg(dev, "no route to SSGN %pI4 from ifidx=%d\n", - &pctx->sgsn_addr.ip4.s_addr, - gti->real_dev->ifindex); + netdev_dbg(dev, "no route to SSGN %pI4\n", + &pctx->sgsn_addr.ip4.s_addr); dev->stats.tx_carrier_errors++; goto err; } @@ -570,12 +588,11 @@ static int gtp_ip4_prepare_xmit(struct sk_buff *skb, struct net_device *dev, } skb_dst_drop(skb); - skb_dst_set(skb, &rt->dst); /* This is similar to tnl_update_pmtu() */ df = iph->frag_off; if (df) { - mtu = dst_mtu(&rt->dst) - gti->real_dev->hard_header_len - + mtu = dst_mtu(&rt->dst) - dev->hard_header_len - sizeof(struct iphdr) - sizeof(struct udphdr); switch (pctx->gtp_version) { case GTP_V0: @@ -586,10 +603,9 @@ static int gtp_ip4_prepare_xmit(struct sk_buff *skb, struct net_device *dev, break; } } else - mtu = skb_dst(skb) ? dst_mtu(skb_dst(skb)) : dev->mtu; + mtu = dst_mtu(&rt->dst); - if (skb_dst(skb)) - skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); + rt->dst.ops->update_pmtu(&rt->dst, NULL, skb, mtu); if (!skb_is_gso(skb) && (iph->frag_off & htons(IP_DF)) && mtu < ntohs(iph->tot_len)) { @@ -600,7 +616,7 @@ static int gtp_ip4_prepare_xmit(struct sk_buff *skb, struct net_device *dev, goto err_rt; } - gtp_set_pktinfo_ipv4(pktinfo, iph, pctx, rt, &fl4, dev); + gtp_set_pktinfo_ipv4(pktinfo, sk, iph, pctx, rt, &fl4, dev); return 0; err_rt: @@ -616,34 +632,23 @@ static int gtp_ip6_prepare_xmit(struct sk_buff *skb, struct net_device *dev, return 0; } -static inline void -gtp_push_ip4hdr(struct sk_buff *skb, struct gtp_pktinfo *pktinfo) +static inline int +gtp_udp_tunnel_xmit(struct sk_buff *skb, __be16 port, + struct gtp_pktinfo *pktinfo) { - struct iphdr *iph; - - /* Push down and install the IP header. Similar to iptunnel_xmit() */ - skb_push(skb, sizeof(struct iphdr)); - skb_reset_network_header(skb); - - iph = ip_hdr(skb); - - iph->version = 4; - iph->ihl = sizeof(struct iphdr) >> 2; - iph->frag_off = htons(IP_DF); - iph->protocol = IPPROTO_UDP; - iph->tos = pktinfo->iph->tos; - iph->daddr = pktinfo->fl4.daddr; - iph->saddr = pktinfo->fl4.saddr; - iph->ttl = ip4_dst_hoplimit(&pktinfo->rt->dst); - __ip_select_ident(dev_net(pktinfo->rt->dst.dev), iph, - (skb_shinfo(skb)->gso_segs ?: 1) - 1); - netdev_dbg(pktinfo->dev, "gtp -> IP src: %pI4 dst: %pI4\n", - &iph->saddr, &iph->daddr); + &pktinfo->iph->saddr, &pktinfo->iph->daddr); + + return udp_tunnel_xmit_skb(pktinfo->rt, pktinfo->sk, skb, + pktinfo->fl4.saddr, + pktinfo->fl4.daddr, + pktinfo->iph->tos, + ip4_dst_hoplimit(&pktinfo->rt->dst), + htons(IP_DF), port, port, true, false); } -static inline void -gtp_push_ip6hdr(struct sk_buff *skb, struct gtp_pktinfo *pktinfo) +static inline int +gtp_ip6tunnel_xmit(struct sk_buff *skb, struct gtp_pktinfo *pktinfo) { /* TODO IPV6 support */ } @@ -654,9 +659,17 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev) unsigned int payload_len; struct gtp_pktinfo pktinfo; unsigned int proto = ntohs(skb->protocol); - int gtph_len, err; + int gtph_len, err = -EINVAL; + __be16 gtph_port; rcu_read_lock(); + + /* ensure there is sufficient headroom */ + if (skb_cow_head(skb, dev->needed_headroom)) + goto tx_error; + + skb_reset_inner_headers(skb); + switch (proto) { case ETH_P_IP: err = gtp_ip4_prepare_xmit(skb, dev, &pktinfo); @@ -669,56 +682,55 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev) if (err < 0) goto tx_error; - /* Annotate length of the encapsulated packet */ - payload_len = skb->len; - /* Push down GTP header */ switch (pktinfo.pctx->gtp_version) { case GTP_V0: - gtp0_push_header(skb, pktinfo.pctx, payload_len); - break; - case GTP_V1: - gtp1_push_header(skb, pktinfo.pctx, payload_len); - break; - } - - /* Push down and install the UDP header. */ - skb_push(skb, sizeof(struct udphdr)); - skb_reset_transport_header(skb); - - uh = udp_hdr(skb); - switch (pktinfo.pctx->gtp_version) { - case GTP_V0: - uh->source = uh->dest = htons(GTP0_PORT); + gtph_port = htons(GTP0_PORT); gtph_len = sizeof(struct gtp0_header); + + gtp0_push_header(skb, pktinfo.pctx); break; case GTP_V1: - uh->source = uh->dest = htons(GTP1U_PORT); + gtph_port = htons(GTP1U_PORT); gtph_len = sizeof(struct gtp1_header); + + gtp1_push_header(skb, pktinfo.pctx); break; + default: + goto tx_error; } - uh->len = htons(sizeof(struct udphdr) + payload_len + gtph_len); - uh->check = 0; - - netdev_dbg(dev, "gtp -> UDP src: %u dst: %u (len %u)\n", - ntohs(uh->source), ntohs(uh->dest), ntohs(uh->len)); - switch (proto) { case ETH_P_IP: - gtp_push_ip4hdr(skb, &pktinfo); + err = gtp_udp_tunnel_xmit(skb, gtph_port, &pktinfo); break; case ETH_P_IPV6: - gtp_push_ip6hdr(skb, &pktinfo); + /* Annotate length of the encapsulated packet */ + payload_len = skb->len; + + /* Push down and install the UDP header. */ + skb_push(skb, sizeof(struct udphdr)); + skb_reset_transport_header(skb); + + uh = udp_hdr(skb); + + uh->source = uh->dest = gtph_port; + uh->len = htons(sizeof(struct udphdr) + payload_len + gtph_len); + uh->check = 0; + + netdev_dbg(dev, "gtp -> UDP src: %u dst: %u (len %u)\n", + ntohs(uh->source), ntohs(uh->dest), ntohs(uh->len)); + + nf_reset(skb); + + netdev_dbg(dev, "Good, now packet leaving from GGSN to SGSN\n"); + + err = gtp_ip6tunnel_xmit(skb, &pktinfo); break; } - rcu_read_unlock(); - nf_reset(skb); - - netdev_dbg(dev, "Good, now packet leaving from GGSN to SGSN\n"); + rcu_read_unlock(); - err = ip_local_out(skb); gtp_iptunnel_xmit_stats(err, &dev->stats, dev->tstats); return NETDEV_TX_OK; @@ -737,10 +749,26 @@ static const struct net_device_ops gtp_netdev_ops = { static void gtp_link_setup(struct net_device *dev) { - dev->priv_flags |= IFF_NO_QUEUE; - dev->netdev_ops = >p_netdev_ops; dev->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->tx_queue_len = 1000; + + dev->priv_flags |= IFF_NO_QUEUE; + dev->features |= NETIF_F_LLTX; + netif_keep_dst(dev); + + dev->needed_headroom = LL_MAX_HEADER + + sizeof(struct iphdr) + + sizeof(struct udphdr) + + sizeof(struct gtp0_header); + } static int gtp_hashtable_new(struct gtp_instance *gti, int hsize); @@ -752,30 +780,13 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[]) { struct gtp_net *gn; - struct net_device *real_dev; struct gtp_instance *gti; int hashsize, err, fd0, fd1; - if (!tb[IFLA_LINK]) - return -EINVAL; - - real_dev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK])); - if (!real_dev) - return -ENODEV; - - dev_hold(real_dev); - if (!tb[IFLA_MTU]) - dev->mtu = real_dev->mtu; - else if (dev->mtu > real_dev->mtu) { - netdev_dbg(dev, "GTP mtu greater that transport MTU (%d > %d)\n", - dev->mtu, real_dev->mtu); - err = -EINVAL; - goto out_err; - } + dev->mtu = 1500; gti = netdev_priv(dev); - gti->real_dev = real_dev; fd0 = nla_get_u32(data[IFLA_GTP_FD0]); fd1 = nla_get_u32(data[IFLA_GTP_FD1]); @@ -813,7 +824,6 @@ out_encap: gtp_encap_disable(gti); out_err: - dev_put(real_dev); return err; } @@ -823,7 +833,6 @@ static void gtp_dellink(struct net_device *dev, struct list_head *head) gtp_encap_disable(gti); gtp_hashtable_free(gti); - dev_put(gti->real_dev); list_del_rcu(>i->list); unregister_netdevice_queue(dev, head); } -- 2.5.0
GTPv1 tunnel use a separate TEI for each direction. Add a union to hold TID for v0 and v1 separately. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 211 +++++++++++++++++++++++++++++++++++---------------------------- gtp_nl.h | 2 + 2 files changed, 118 insertions(+), 95 deletions(-) diff --git a/gtp.c b/gtp.c index 2ac6431..a4be31d 100644 --- a/gtp.c +++ b/gtp.c @@ -39,7 +39,17 @@ struct pdp_ctx { struct hlist_node hlist_tid; struct hlist_node hlist_addr; - u64 tid; + union { + uint64_t tid; + struct { + uint64_t tid; + u16 flow; + } v0 __attribute__((__packed__)); + struct { + uint32_t i_tid; + uint32_t o_tid; + } v1 __attribute__((__packed__)); + } u; u8 gtp_version; u16 af; @@ -53,7 +63,6 @@ struct pdp_ctx { struct in_addr ip4; } sgsn_addr; - u16 flow; atomic_t tx_seq; struct rcu_head rcu_head; @@ -113,7 +122,7 @@ static struct pdp_ctx *gtp0_pdp_find(struct gtp_instance *gti, u64 tid) head = >i->tid_hash[gtp0_hashfn(tid) % gti->hash_size]; hlist_for_each_entry_rcu(pdp, head, hlist_tid) { - if (pdp->gtp_version == GTP_V0 && pdp->tid == tid) + if (pdp->gtp_version == GTP_V0 && pdp->u.v0.tid == tid) return pdp; } @@ -129,7 +138,7 @@ static struct pdp_ctx *gtp1_pdp_find(struct gtp_instance *gti, u32 tid) head = >i->tid_hash[gtp1u_hashfn(tid) % gti->hash_size]; hlist_for_each_entry_rcu(pdp, head, hlist_tid) { - if (pdp->gtp_version == GTP_V1 && pdp->tid == tid) + if (pdp->gtp_version == GTP_V1 && pdp->u.v1.i_tid == tid) return pdp; } @@ -456,10 +465,10 @@ gtp0_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) gtp0->type = GTP_TPDU; gtp0->length = htons(payload_len); gtp0->seq = htons((atomic_inc_return(&pctx->tx_seq)-1) % 0xffff); - gtp0->flow = htons(pctx->flow); + gtp0->flow = htons(pctx->u.v0.flow); gtp0->number = 0xFF; gtp0->spare[0] = gtp0->spare[1] = gtp0->spare[2] = 0xFF; - gtp0->tid = cpu_to_be64(pctx->tid); + gtp0->tid = cpu_to_be64(pctx->u.v0.tid); } static inline void @@ -480,7 +489,7 @@ gtp1_push_header(struct sk_buff *skb, struct pdp_ctx *pctx) gtp1->flags = 0x38; /* V1, GTP-non-prime */ gtp1->type = GTP_TPDU; gtp1->length = htons(payload_len); - gtp1->tid = htonl((u32)pctx->tid); + gtp1->tid = htonl(pctx->u.v1.o_tid); /* TODO: Suppport for extension header, sequence number and N-PDU. * Update the length field if any of them is available. @@ -1021,43 +1030,45 @@ static struct net_device *gtp_find_dev(struct net *net, int ifindex) return NULL; } -static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) +static void ipv4_pdp_fill(struct pdp_ctx *pctx, struct genl_info *info) { - struct gtp_instance *gti = netdev_priv(dev); - struct pdp_ctx *pctx; - u16 flow = 0; - u32 gtp_version, sgsn_addr, ms_addr, hash_ms, hash_tid; - u64 tid; - bool found = false; + pctx->gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]); + pctx->af = AF_INET; + pctx->sgsn_addr.ip4.s_addr = + nla_get_u32(info->attrs[GTPA_SGSN_ADDRESS]); + pctx->ms_addr.ip4.s_addr = + nla_get_u32(info->attrs[GTPA_MS_ADDRESS]); - gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]); - switch (gtp_version) { + switch (pctx->gtp_version) { case GTP_V0: - case GTP_V1: + + /* According to TS 09.60, sections 7.5.1 and 7.5.2, the flow + * label needs to be the same for uplink and downlink packets, + * so let's annotate this. + */ + pctx->u.v0.tid = nla_get_u64(info->attrs[GTPA_TID]); + pctx->u.v0.flow = nla_get_u16(info->attrs[GTPA_FLOW]); break; - default: - return -EINVAL; - } - tid = nla_get_u64(info->attrs[GTPA_TID]); - /* GTPv1 allows 32-bits tunnel IDs */ - if (gtp_version == GTP_V1 && tid > UINT_MAX) - return -EINVAL; + case GTP_V1: + pctx->u.v1.i_tid = nla_get_u32(info->attrs[GTPA_I_TID]); + pctx->u.v1.o_tid = nla_get_u32(info->attrs[GTPA_O_TID]); - /* According to TS 09.60, sections 7.5.1 and 7.5.2, the flow label - * needs to be the same for uplink and downlink packets, so let's - * annotate this. - */ - if (gtp_version == GTP_V0) { - if (!info->attrs[GTPA_FLOW]) - return -EINVAL; + break; - flow = nla_get_u16(info->attrs[GTPA_FLOW]); + default: + break; } +} - sgsn_addr = nla_get_u32(info->attrs[GTPA_SGSN_ADDRESS]); - ms_addr = nla_get_u32(info->attrs[GTPA_MS_ADDRESS]); +static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) +{ + struct gtp_instance *gti = netdev_priv(dev); + struct pdp_ctx *pctx; + u32 ms_addr, hash_ms, hash_tid = 0; + bool found = false; + ms_addr = nla_get_u32(info->attrs[GTPA_MS_ADDRESS]); hash_ms = ipv4_hashfn(ms_addr) % gti->hash_size; hlist_for_each_entry_rcu(pctx, >i->addr_hash[hash_ms], hlist_addr) { @@ -1073,47 +1084,50 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE) return -EOPNOTSUPP; - pctx->af = AF_INET; - pctx->gtp_version = gtp_version; - pctx->tid = tid; - pctx->sgsn_addr.ip4.s_addr = sgsn_addr; - pctx->ms_addr.ip4.s_addr = ms_addr; + ipv4_pdp_fill(pctx, info); - netdev_dbg(dev, "update tunnel id = %llx (pdp %p)\n", - tid, pctx); + if (pctx->gtp_version == GTP_V0) + netdev_dbg(dev, "GTPv0-U: update tunnel id = %llx (pdp %p)\n", + pctx->u.v0.tid, pctx); + else if (pctx->gtp_version == GTP_V1) + netdev_dbg(dev, "GTPv1-U: update tunnel id = %x/%x (pdp %p)\n", + pctx->u.v1.i_tid, pctx->u.v1.o_tid, pctx); return 0; + } pctx = kmalloc(sizeof(struct pdp_ctx), GFP_KERNEL); if (pctx == NULL) return -ENOMEM; - pctx->af = AF_INET; - pctx->gtp_version = gtp_version; - pctx->tid = tid; - pctx->sgsn_addr.ip4.s_addr = sgsn_addr; - pctx->ms_addr.ip4.s_addr = ms_addr; - pctx->flow = flow; + ipv4_pdp_fill(pctx, info); atomic_set(&pctx->tx_seq, 0); - switch (gtp_version) { + switch (pctx->gtp_version) { case GTP_V0: /* TS 09.60: "The flow label identifies unambiguously a GTP * flow.". We use the tid for this instead, I cannot find a * situation in which this doesn't unambiguosly identify the * PDP context. */ - hash_tid = gtp0_hashfn(tid) % gti->hash_size; + hash_tid = gtp0_hashfn(pctx->u.v0.tid) % gti->hash_size; break; + case GTP_V1: - hash_tid = gtp1u_hashfn(tid) % gti->hash_size; + hash_tid = gtp1u_hashfn(pctx->u.v1.i_tid) % gti->hash_size; break; } + hlist_add_head_rcu(&pctx->hlist_addr, >i->addr_hash[hash_ms]); hlist_add_head_rcu(&pctx->hlist_tid, >i->tid_hash[hash_tid]); - netdev_dbg(dev, "adding tunnel id = %llx (pdp %p)\n", tid, pctx); + if (pctx->gtp_version == GTP_V0) + netdev_dbg(dev, "GTPv0-U: adding tunnel id = %llx (pdp %p)\n", + pctx->u.v0.tid, pctx); + else if (pctx->gtp_version == GTP_V1) + netdev_dbg(dev, "GTPv1-U: adding tunnel id = %x/%x (pdp %p)\n", + pctx->u.v1.i_tid, pctx->u.v1.o_tid, pctx); return 0; } @@ -1126,9 +1140,25 @@ static int gtp_genl_tunnel_new(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_MS_ADDRESS] || - !info->attrs[GTPA_TID]) + !info->attrs[GTPA_MS_ADDRESS]) + return -EINVAL; + + switch (nla_get_u32(info->attrs[GTPA_VERSION])) { + case GTP_V0: + if (!info->attrs[GTPA_TID] || + !info->attrs[GTPA_FLOW]) + return -EINVAL; + break; + + case GTP_V1: + if (!info->attrs[GTPA_I_TID] || + !info->attrs[GTPA_O_TID]) + return -EINVAL; + break; + + default: return -EINVAL; + } net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); if (net == NULL) @@ -1148,28 +1178,9 @@ static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) struct gtp_instance *gti; struct net_device *dev; struct pdp_ctx *pctx; - u32 gtp_version; - u64 tid; if (!info->attrs[GTPA_VERSION] || - !info->attrs[GTPA_LINK] || - !info->attrs[GTPA_SGSN_ADDRESS] || - !info->attrs[GTPA_MS_ADDRESS] || - !info->attrs[GTPA_TID]) - return -EINVAL; - - gtp_version = nla_get_u32(info->attrs[GTPA_VERSION]); - switch (gtp_version) { - case GTP_V0: - case GTP_V1: - break; - default: - return -EINVAL; - } - - tid = nla_get_u64(info->attrs[GTPA_TID]); - /* GTPv1 allows 32-bits tunnel IDs */ - if (gtp_version == GTP_V1 && tid > UINT_MAX) + !info->attrs[GTPA_LINK]) return -EINVAL; net = gtp_genl_get_net(sock_net(skb->sk), info->attrs); @@ -1183,20 +1194,32 @@ static int gtp_genl_tunnel_delete(struct sk_buff *skb, struct genl_info *info) gti = netdev_priv(dev); - switch (gtp_version) { + switch (nla_get_u32(info->attrs[GTPA_VERSION])) { case GTP_V0: + if (!info->attrs[GTPA_TID]) + return -EINVAL; pctx = gtp0_pdp_find(gti, nla_get_u64(info->attrs[GTPA_TID])); break; + case GTP_V1: - pctx = gtp1_pdp_find(gti, nla_get_u64(info->attrs[GTPA_TID])); + if (!info->attrs[GTPA_I_TID]) + return -EINVAL; + pctx = gtp1_pdp_find(gti, nla_get_u64(info->attrs[GTPA_I_TID])); break; + + default: + return -EINVAL; } if (pctx == NULL) return -ENOENT; - netdev_dbg(dev, "deleting tunnel with ID %lld\n", - (unsigned long long) nla_get_u64(info->attrs[GTPA_TID])); + if (pctx->gtp_version == GTP_V0) + netdev_dbg(dev, "GTPv0-U: deleting tunnel id = %llx (pdp %p)\n", + pctx->u.v0.tid, pctx); + else if (pctx->gtp_version == GTP_V1) + netdev_dbg(dev, "GTPv1-U: deleting tunnel id = %x/%x (pdp %p)\n", + pctx->u.v1.i_tid, pctx->u.v1.o_tid, pctx); hlist_del_rcu(&pctx->hlist_tid); hlist_del_rcu(&pctx->hlist_addr); @@ -1228,9 +1251,12 @@ gtp_genl_fill_info(struct sk_buff *skb, u32 snd_portid, u32 snd_seq, if (nla_put_u32(skb, GTPA_VERSION, pctx->gtp_version) || nla_put_u32(skb, GTPA_SGSN_ADDRESS, pctx->sgsn_addr.ip4.s_addr) || nla_put_u32(skb, GTPA_MS_ADDRESS, pctx->ms_addr.ip4.s_addr) || - nla_put_u64(skb, GTPA_TID, pctx->tid) || (pctx->gtp_version == GTP_V0 && - nla_put_u16(skb, GTPA_FLOW, pctx->flow))) + nla_put_u64(skb, GTPA_TID, pctx->u.v0.tid) && + nla_put_u16(skb, GTPA_FLOW, pctx->u.v0.flow)) || + (pctx->gtp_version == GTP_V1 && + nla_put_u32(skb, GTPA_I_TID, pctx->u.v1.i_tid) && + nla_put_u32(skb, GTPA_O_TID, pctx->u.v1.o_tid))) goto nla_put_failure; genlmsg_end(skb, genlh); @@ -1277,23 +1303,16 @@ static int gtp_genl_tunnel_get(struct sk_buff *skb, struct genl_info *info) gti = netdev_priv(dev); rcu_read_lock(); - if (info->attrs[GTPA_TID]) { + if (gtp_version == GTP_V0 && + info->attrs[GTPA_TID]) { u64 tid = nla_get_u64(info->attrs[GTPA_TID]); - /* GTPv1 allows 32-bits tunnel IDs */ - if (gtp_version == GTP_V1 && tid > UINT_MAX) { - err = -EINVAL; - goto err_unlock; - } + pctx = gtp0_pdp_find(gti, tid); + } else if (gtp_version == GTP_V1 && + info->attrs[GTPA_I_TID]) { + u32 tid = nla_get_u32(info->attrs[GTPA_I_TID]); - switch (gtp_version) { - case GTP_V0: - pctx = gtp0_pdp_find(gti, tid); - break; - case GTP_V1: - pctx = gtp1_pdp_find(gti, tid); - break; - } + pctx = gtp1_pdp_find(gti, tid); } else if (info->attrs[GTPA_MS_ADDRESS]) { u32 ip = nla_get_u32(info->attrs[GTPA_MS_ADDRESS]); @@ -1347,7 +1366,7 @@ gtp_genl_tunnel_dump(struct sk_buff *skb, struct netlink_callback *cb) for (i = k; i < gti->hash_size; i++) { hlist_for_each_entry_rcu(pctx, >i->tid_hash[i], hlist_tid) { - if (tid && tid != pctx->tid) + if (tid && tid != pctx->u.tid) continue; else tid = 0; @@ -1358,7 +1377,7 @@ gtp_genl_tunnel_dump(struct sk_buff *skb, struct netlink_callback *cb) cb->nlh->nlmsg_type, pctx); if (ret < 0) { cb->args[0] = i; - cb->args[1] = pctx->tid; + cb->args[1] = pctx->u.tid; cb->args[2] = (unsigned long)gti; goto out; } @@ -1378,6 +1397,8 @@ static struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = { [GTPA_MS_ADDRESS] = { .type = NLA_NESTED, }, [GTPA_FLOW] = { .type = NLA_U16, }, [GTPA_NET_NS_FD] = { .type = NLA_U32, }, + [GTPA_I_TID] = { .type = NLA_U32, }, + [GTPA_O_TID] = { .type = NLA_U32, }, }; static const struct genl_ops gtp_genl_ops[] = { diff --git a/gtp_nl.h b/gtp_nl.h index c20666d..370576b 100644 --- a/gtp_nl.h +++ b/gtp_nl.h @@ -41,6 +41,8 @@ enum gtp_attrs { GTPA_MS_ADDRESS, GTPA_FLOW, GTPA_NET_NS_FD, + GTPA_I_TID, + GTPA_O_TID, __GTPA_MAX, }; #define GTPA_MAX (__GTPA_MAX + 1) -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:52PM +0100, Andreas Schultz wrote:
GTPv1 tunnel use a separate TEI for each direction. Add a union to hold TID for v0 and v1 separately.
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 211 +++++++++++++++++++++++++++++++++++---------------------------- gtp_nl.h | 2 + 2 files changed, 118 insertions(+), 95 deletions(-)
diff --git a/gtp.c b/gtp.c index 2ac6431..a4be31d 100644 --- a/gtp.c +++ b/gtp.c @@ -39,7 +39,17 @@ struct pdp_ctx { struct hlist_node hlist_tid; struct hlist_node hlist_addr;
- u64 tid; + union { + uint64_t tid; + struct { + uint64_t tid; + u16 flow; + } v0 __attribute__((__packed__));
We're not sending this through network, so this packed attribute makes no sense to me.
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- libgtnl/include/linux/gtp_nl.h | 2 +- libgtnl/src/gtp-genl.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libgtnl/include/linux/gtp_nl.h b/libgtnl/include/linux/gtp_nl.h index a8fdf3a..a1e8ce1 100644 --- a/libgtnl/include/linux/gtp_nl.h +++ b/libgtnl/include/linux/gtp_nl.h @@ -39,7 +39,7 @@ enum gtp_attrs { GTPA_TID, /* 64 bits for GTPv1 */ GTPA_SGSN_ADDRESS, GTPA_MS_ADDRESS, - GTPA_FLOWID, /* only for GTPv0 */ + GTPA_FLOW, /* only for GTPv0 */ GTPA_NET_NS_FD, __GTPA_MAX, }; diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c index 9e68a30..2717821 100644 --- a/libgtnl/src/gtp-genl.c +++ b/libgtnl/src/gtp-genl.c @@ -50,7 +50,7 @@ static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t) mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr); mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr); mnl_attr_put_u64(nlh, GTPA_TID, t->tid); - mnl_attr_put_u16(nlh, GTPA_FLOWID, t->flowid); + mnl_attr_put_u16(nlh, GTPA_FLOW, t->flowid); } int gtp_add_tunnel(int genl_id, struct mnl_socket *nl, struct gtp_tunnel *t) -- 2.5.0
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- libgtnl/include/libgtpnl/gtpnl.h | 3 +-- libgtnl/src/gtp-rtnl.c | 4 +--- libgtnl/tools/gtp-link-add.c | 5 ++--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/libgtnl/include/libgtpnl/gtpnl.h b/libgtnl/include/libgtpnl/gtpnl.h index 3d3fd73..49ba03d 100644 --- a/libgtnl/include/libgtpnl/gtpnl.h +++ b/libgtnl/include/libgtpnl/gtpnl.h @@ -16,8 +16,7 @@ int genl_lookup_family(struct mnl_socket *nl, const char *family); struct in_addr; -int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname, - int fd0, int fd1); +int gtp_dev_create(int dest_ns, const char *gtp_ifname, int fd0, int fd1); int gtp_dev_config(const char *iface, struct in_addr *net, uint32_t prefix); int gtp_dev_destroy(const char *gtp_ifname); diff --git a/libgtnl/src/gtp-rtnl.c b/libgtnl/src/gtp-rtnl.c index db54653..0999aa3 100644 --- a/libgtnl/src/gtp-rtnl.c +++ b/libgtnl/src/gtp-rtnl.c @@ -108,8 +108,7 @@ static int gtp_dev_talk(struct nlmsghdr *nlh, uint32_t seq) return ret; } -int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname, - int fd0, int fd1) +int gtp_dev_create(int dest_ns, const char *gtp_ifname, int fd0, int fd1) { char buf[MNL_SOCKET_BUFFER_SIZE]; struct nlmsghdr *nlh; @@ -126,7 +125,6 @@ int gtp_dev_create(int dest_ns, const char *gtp_ifname, const char *real_ifname, if (dest_ns > 0) mnl_attr_put_u32(nlh, IFLA_NET_NS_FD, dest_ns); - mnl_attr_put_u32(nlh, IFLA_LINK, if_nametoindex(real_ifname)); mnl_attr_put_str(nlh, IFLA_IFNAME, gtp_ifname); nest = mnl_attr_nest_start(nlh, IFLA_LINKINFO); mnl_attr_put_str(nlh, IFLA_INFO_KIND, "gtp"); diff --git a/libgtnl/tools/gtp-link-add.c b/libgtnl/tools/gtp-link-add.c index 3d893eb..4ef025e 100644 --- a/libgtnl/tools/gtp-link-add.c +++ b/libgtnl/tools/gtp-link-add.c @@ -43,8 +43,8 @@ int main(int argc, char *argv[]) unsigned int seq, portid, change = 0, flags = 0; struct nlattr *nest, *nest2; - if (argc != 2) { - printf("Usage: %s [ifname]\n", argv[0]); + if (argc != 1) { + printf("Usage: %s\n", argv[0]); exit(EXIT_FAILURE); } @@ -62,7 +62,6 @@ int main(int argc, char *argv[]) int fd1 = socket(AF_INET, SOCK_DGRAM, 0); int fd2 = socket(AF_INET, SOCK_DGRAM, 0); - mnl_attr_put_u32(nlh, IFLA_LINK, if_nametoindex(argv[1])); mnl_attr_put_str(nlh, IFLA_IFNAME, "gtp0"); nest = mnl_attr_nest_start(nlh, IFLA_LINKINFO); mnl_attr_put_str(nlh, IFLA_INFO_KIND, "gtp"); -- 2.5.0
log message: "no longer needed" ? ~Neels
On Thu, Nov 19, 2015 at 01:41:05PM +0100, Neels Hofmeyr wrote:
log message: "no longer needed" ?
Please, no more English description corrections. This is good enough to understand the intention. Thank you.
On Thu, Nov 19, 2015 at 05:38:16PM +0100, Pablo Neira Ayuso wrote:
On Thu, Nov 19, 2015 at 01:41:05PM +0100, Neels Hofmeyr wrote:
log message: "no longer needed" ?
Please, no more English description corrections. This is good enough to understand the intention. Thank you.
Acknowledged. Please do correct my own comments/log typos, if you find any. Thanks! ~Neels
GTPv1 tunnel use a separate 32bit TIDs for each direction while GTPv0 uses only one 64bit TID. Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- libgtnl/include/libgtpnl/gtp.h | 4 ++ libgtnl/include/linux/gtp_nl.h | 2 + libgtnl/src/gtp-genl.c | 32 ++++++++-- libgtnl/src/gtp.c | 32 ++++++++-- libgtnl/src/internal.h | 12 +++- libgtnl/src/libgtpnl.map | 4 ++ libgtnl/tools/Makefile.am | 4 +- libgtnl/tools/gtp-tunnel.c | 132 ++++++++++++++++++++++++++++------------- libgtnl/tools/gtpnl.c | 35 ----------- 9 files changed, 168 insertions(+), 89 deletions(-) delete mode 100644 libgtnl/tools/gtpnl.c diff --git a/libgtnl/include/libgtpnl/gtp.h b/libgtnl/include/libgtpnl/gtp.h index fa09d2a..52fa4d9 100644 --- a/libgtnl/include/libgtpnl/gtp.h +++ b/libgtnl/include/libgtpnl/gtp.h @@ -14,6 +14,8 @@ void gtp_tunnel_set_ms_ip4(struct gtp_tunnel *t, struct in_addr *ms_addr); void gtp_tunnel_set_sgsn_ip4(struct gtp_tunnel *t, struct in_addr *sgsn_addr); void gtp_tunnel_set_version(struct gtp_tunnel *t, uint32_t version); void gtp_tunnel_set_tid(struct gtp_tunnel *t, uint64_t tid); +void gtp_tunnel_set_i_tid(struct gtp_tunnel *t, uint32_t i_tid); +void gtp_tunnel_set_o_tid(struct gtp_tunnel *t, uint32_t o_tid); void gtp_tunnel_set_flowid(struct gtp_tunnel *t, uint16_t flowid); const int gtp_tunnel_get_ifns(struct gtp_tunnel *t); @@ -22,6 +24,8 @@ const struct in_addr *gtp_tunnel_get_ms_ip4(struct gtp_tunnel *t); const struct in_addr *gtp_tunnel_get_sgsn_ip4(struct gtp_tunnel *t); int gtp_tunnel_get_version(struct gtp_tunnel *t); uint64_t gtp_tunnel_get_tid(struct gtp_tunnel *t); +uint32_t gtp_tunnel_get_i_tid(struct gtp_tunnel *t); +uint32_t gtp_tunnel_get_o_tid(struct gtp_tunnel *t); uint16_t gtp_tunnel_get_flowid(struct gtp_tunnel *t); #endif diff --git a/libgtnl/include/linux/gtp_nl.h b/libgtnl/include/linux/gtp_nl.h index a1e8ce1..5859b4e 100644 --- a/libgtnl/include/linux/gtp_nl.h +++ b/libgtnl/include/linux/gtp_nl.h @@ -41,6 +41,8 @@ enum gtp_attrs { GTPA_MS_ADDRESS, GTPA_FLOW, /* only for GTPv0 */ GTPA_NET_NS_FD, + GTPA_I_TID, + GTPA_O_TID, __GTPA_MAX, }; #define GTPA_MAX (__GTPA_MAX + 1) diff --git a/libgtnl/src/gtp-genl.c b/libgtnl/src/gtp-genl.c index 2717821..2b72600 100644 --- a/libgtnl/src/gtp-genl.c +++ b/libgtnl/src/gtp-genl.c @@ -49,8 +49,13 @@ static void gtp_build_payload(struct nlmsghdr *nlh, struct gtp_tunnel *t) mnl_attr_put_u32(nlh, GTPA_LINK, t->ifidx); mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, t->sgsn_addr.s_addr); mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, t->ms_addr.s_addr); - mnl_attr_put_u64(nlh, GTPA_TID, t->tid); - mnl_attr_put_u16(nlh, GTPA_FLOW, t->flowid); + if (t->gtp_version == GTP_V0) { + mnl_attr_put_u64(nlh, GTPA_TID, t->u.v0.tid); + mnl_attr_put_u16(nlh, GTPA_FLOW, t->u.v0.flowid); + } else if (t->gtp_version == GTP_V1) { + mnl_attr_put_u32(nlh, GTPA_I_TID, t->u.v1.i_tid); + mnl_attr_put_u32(nlh, GTPA_O_TID, t->u.v1.o_tid); + } } int gtp_add_tunnel(int genl_id, struct mnl_socket *nl, struct gtp_tunnel *t) @@ -95,7 +100,15 @@ EXPORT_SYMBOL(gtp_del_tunnel); struct gtp_pdp { uint32_t version; - uint64_t tid; + union { + struct { + uint64_t tid; + } v0; + struct { + uint32_t i_tid; + uint32_t o_tid; + } v1; + } u; struct in_addr sgsn_addr; struct in_addr ms_addr; }; @@ -115,6 +128,8 @@ static int genl_gtp_validate_cb(const struct nlattr *attr, void *data) return MNL_CB_ERROR; } break; + case GTPA_O_TID: + case GTPA_I_TID: case GTPA_SGSN_ADDRESS: case GTPA_MS_ADDRESS: case GTPA_VERSION: @@ -138,7 +153,11 @@ static int genl_gtp_attr_cb(const struct nlmsghdr *nlh, void *data) mnl_attr_parse(nlh, sizeof(*genl), genl_gtp_validate_cb, tb); if (tb[GTPA_TID]) - pdp.tid = mnl_attr_get_u64(tb[GTPA_TID]); + pdp.u.v0.tid = mnl_attr_get_u64(tb[GTPA_TID]); + if (tb[GTPA_I_TID]) + pdp.u.v1.i_tid = mnl_attr_get_u32(tb[GTPA_I_TID]); + if (tb[GTPA_O_TID]) + pdp.u.v1.o_tid = mnl_attr_get_u32(tb[GTPA_O_TID]); if (tb[GTPA_SGSN_ADDRESS]) { pdp.sgsn_addr.s_addr = mnl_attr_get_u32(tb[GTPA_SGSN_ADDRESS]); @@ -151,7 +170,10 @@ static int genl_gtp_attr_cb(const struct nlmsghdr *nlh, void *data) } printf("version %u ", pdp.version); - printf("tid %"PRIu64" ms_addr %s ", pdp.tid, inet_ntoa(pdp.sgsn_addr)); + if (pdp.version == GTP_V0) + printf("tid %"PRIu64" ms_addr %s ", pdp.u.v0.tid, inet_ntoa(pdp.sgsn_addr)); + else if (pdp.version == GTP_V1) + printf("tid %u/%u ms_addr %s ", pdp.u.v1.i_tid, pdp.u.v1.o_tid, inet_ntoa(pdp.sgsn_addr)); printf("sgsn_addr %s\n", inet_ntoa(pdp.ms_addr)); return MNL_CB_OK; diff --git a/libgtnl/src/gtp.c b/libgtnl/src/gtp.c index 6e3d473..4fedf81 100644 --- a/libgtnl/src/gtp.c +++ b/libgtnl/src/gtp.c @@ -71,16 +71,28 @@ EXPORT_SYMBOL(gtp_tunnel_set_version); void gtp_tunnel_set_tid(struct gtp_tunnel *t, uint64_t tid) { - t->tid = tid; + t->u.v0.tid = tid; } EXPORT_SYMBOL(gtp_tunnel_set_tid); void gtp_tunnel_set_flowid(struct gtp_tunnel *t, uint16_t flowid) { - t->flowid = flowid; + t->u.v0.flowid = flowid; } EXPORT_SYMBOL(gtp_tunnel_set_flowid); +void gtp_tunnel_set_i_tid(struct gtp_tunnel *t, uint32_t i_tid) +{ + t->u.v1.i_tid = i_tid; +} +EXPORT_SYMBOL(gtp_tunnel_set_i_tid); + +void gtp_tunnel_set_o_tid(struct gtp_tunnel *t, uint32_t o_tid) +{ + t->u.v1.o_tid = o_tid; +} +EXPORT_SYMBOL(gtp_tunnel_set_o_tid); + const int gtp_tunnel_get_ifns(struct gtp_tunnel *t) { return t->ifns; @@ -113,12 +125,24 @@ EXPORT_SYMBOL(gtp_tunnel_get_version); uint64_t gtp_tunnel_get_tid(struct gtp_tunnel *t) { - return t->tid; + return t->u.v0.tid; } EXPORT_SYMBOL(gtp_tunnel_get_tid); uint16_t gtp_tunnel_get_flowid(struct gtp_tunnel *t) { - return t->flowid; + return t->u.v0.flowid; } EXPORT_SYMBOL(gtp_tunnel_get_flowid); + +uint32_t gtp_tunnel_get_i_tid(struct gtp_tunnel *t) +{ + return t->u.v1.i_tid; +} +EXPORT_SYMBOL(gtp_tunnel_get_i_tid); + +uint32_t gtp_tunnel_get_o_tid(struct gtp_tunnel *t) +{ + return t->u.v1.o_tid; +} +EXPORT_SYMBOL(gtp_tunnel_get_o_tid); diff --git a/libgtnl/src/internal.h b/libgtnl/src/internal.h index 68f0135..2496454 100644 --- a/libgtnl/src/internal.h +++ b/libgtnl/src/internal.h @@ -17,9 +17,17 @@ struct gtp_tunnel { uint32_t ifidx; struct in_addr ms_addr; struct in_addr sgsn_addr; - uint64_t tid; - uint16_t flowid; int gtp_version; + union { + struct { + uint64_t tid; + uint16_t flowid; + } v0; + struct { + uint32_t i_tid; + uint32_t o_tid; + } v1; + } u; }; #endif diff --git a/libgtnl/src/libgtpnl.map b/libgtnl/src/libgtpnl.map index 2368467..09bdde5 100644 --- a/libgtnl/src/libgtpnl.map +++ b/libgtnl/src/libgtpnl.map @@ -22,6 +22,8 @@ global: gtp_tunnel_set_version; gtp_tunnel_set_tid; gtp_tunnel_set_flowid; + gtp_tunnel_set_i_tid; + gtp_tunnel_set_o_tid; gtp_tunnel_get_ifns; gtp_tunnel_get_ifidx; gtp_tunnel_get_ms_ip4; @@ -29,6 +31,8 @@ global: gtp_tunnel_get_version; gtp_tunnel_get_tid; gtp_tunnel_get_flowid; + gtp_tunnel_get_i_tid; + gtp_tunnel_get_o_tid; local: *; }; diff --git a/libgtnl/tools/Makefile.am b/libgtnl/tools/Makefile.am index 7880f3c..89dca32 100644 --- a/libgtnl/tools/Makefile.am +++ b/libgtnl/tools/Makefile.am @@ -3,8 +3,8 @@ include $(top_srcdir)/Make_global.am check_PROGRAMS = gtp-link-add \ gtp-tunnel -gtp_link_add_SOURCES = gtp-link-add.c gtpnl.c +gtp_link_add_SOURCES = gtp-link-add.c gtp_link_add_LDADD = ../src/libgtpnl.la ${LIBMNL_LIBS} -gtp_tunnel_SOURCES = gtp-tunnel.c gtpnl.c +gtp_tunnel_SOURCES = gtp-tunnel.c gtp_tunnel_LDADD = ../src/libgtpnl.la ${LIBMNL_LIBS} diff --git a/libgtnl/tools/gtp-tunnel.c b/libgtnl/tools/gtp-tunnel.c index 9c52a27..409126e 100644 --- a/libgtnl/tools/gtp-tunnel.c +++ b/libgtnl/tools/gtp-tunnel.c @@ -28,71 +28,91 @@ #include <arpa/inet.h> #include <sys/socket.h> #include <netinet/in.h> +#include <net/if.h> +#include <inttypes.h> #include <libmnl/libmnl.h> #include <linux/genetlink.h> #include <linux/gtp_nl.h> +#include <libgtpnl/gtp.h> #include <libgtpnl/gtpnl.h> +static void add_usage(const char *name) +{ + printf("%s add <gtp device> <v0> <tid> <ms-addr> <sgsn-addr>\n", + name); + printf("%s add <gtp device> <v1> <i_tid> <o_tid> <ms-addr> <sgsn-addr>\n", + name); +} + static int add_tunnel(int argc, char *argv[], int genl_id, struct mnl_socket *nl) { + struct gtp_tunnel *t; uint32_t gtp_ifidx; struct in_addr ms, sgsn; - struct nlmsghdr *nlh; - char buf[MNL_SOCKET_BUFFER_SIZE]; - uint32_t seq = time(NULL), gtp_version; + uint32_t gtp_version; + int optidx; - if (argc != 7) { - printf("%s add <gtp device> <v0|v1> <tid> <ms-addr> <sgsn-addr>\n", - argv[0]); + if (argc < 7 || argc > 8) { + add_usage(argv[0]); return EXIT_FAILURE; } - gtp_ifidx = if_nametoindex(argv[2]); + + t = gtp_tunnel_alloc(); + optidx = 2; + + gtp_ifidx = if_nametoindex(argv[optidx]); if (gtp_ifidx == 0) { - fprintf(stderr, "wrong GTP interface %s\n", argv[2]); + fprintf(stderr, "wrong GTP interface %s\n", argv[optidx]); return EXIT_FAILURE; } + gtp_tunnel_set_ifidx(t, gtp_ifidx); - if (inet_aton(argv[5], &ms) < 0) { - perror("bad address for ms"); - exit(EXIT_FAILURE); - } - - if (inet_aton(argv[6], &sgsn) < 0) { - perror("bad address for sgsn"); - exit(EXIT_FAILURE); - } + optidx++; - if (strcmp(argv[3], "v0") == 0) + if (strcmp(argv[optidx], "v0") == 0) gtp_version = GTP_V0; - else if (strcmp(argv[3], "v1") == 0) + else if (strcmp(argv[optidx], "v1") == 0) gtp_version = GTP_V1; else { fprintf(stderr, "wrong GTP version %s, use v0 or v1\n", - argv[3]); + argv[optidx]); return EXIT_FAILURE; } + gtp_tunnel_set_version(t, gtp_version); - nlh = genl_nlmsg_build_hdr(buf, genl_id, NLM_F_EXCL | NLM_F_ACK, ++seq, - GTP_CMD_TUNNEL_NEW); - gtp_build_payload(nlh, atoi(argv[4]), gtp_ifidx, sgsn.s_addr, - ms.s_addr, gtp_version); + if (gtp_version == GTP_V0) + gtp_tunnel_set_tid(t, atoi(argv[optidx++])); + else if (gtp_version == GTP_V1) { + gtp_tunnel_set_i_tid(t, atoi(argv[optidx++])); + gtp_tunnel_set_o_tid(t, atoi(argv[optidx++])); + } - if (genl_socket_talk(nl, nlh, seq, NULL, NULL) < 0) - perror("genl_socket_talk"); + if (inet_aton(argv[optidx++], &ms) < 0) { + perror("bad address for ms"); + exit(EXIT_FAILURE); + } + gtp_tunnel_set_ms_ip4(t, &ms); + + if (inet_aton(argv[optidx++], &sgsn) < 0) { + perror("bad address for sgsn"); + exit(EXIT_FAILURE); + } + gtp_tunnel_set_sgsn_ip4(t, &sgsn); + + gtp_add_tunnel(genl_id, nl, t); + gtp_tunnel_free(t); return 0; } static int del_tunnel(int argc, char *argv[], int genl_id, struct mnl_socket *nl) { + struct gtp_tunnel *t; uint32_t gtp_ifidx; - char buf[MNL_SOCKET_BUFFER_SIZE]; - struct nlmsghdr *nlh; - uint32_t seq = time(NULL); if (argc != 5) { printf("%s add <gtp device> <version> <tid>\n", @@ -100,21 +120,44 @@ del_tunnel(int argc, char *argv[], int genl_id, struct mnl_socket *nl) return EXIT_FAILURE; } - gtp_ifidx = if_nametoindex(argv[2]); + t = gtp_tunnel_alloc(); - nlh = genl_nlmsg_build_hdr(buf, genl_id, NLM_F_ACK, ++seq, - GTP_CMD_TUNNEL_DELETE); - gtp_build_payload(nlh, atoi(argv[4]), gtp_ifidx, 0, 0, atoi(argv[3])); + gtp_ifidx = if_nametoindex(argv[2]); + if (gtp_ifidx == 0) { + fprintf(stderr, "wrong GTP interface %s\n", argv[2]); + return EXIT_FAILURE; + } + gtp_tunnel_set_ifidx(t, gtp_ifidx); + + if (strcmp(argv[3], "v0") == 0) { + gtp_tunnel_set_version(t, GTP_V0); + gtp_tunnel_set_tid(t, atoi(argv[4])); + } else if (strcmp(argv[3], "v1") == 0) { + gtp_tunnel_set_version(t, GTP_V1); + gtp_tunnel_set_i_tid(t, atoi(argv[4])); + } else { + fprintf(stderr, "wrong GTP version %s, use v0 or v1\n", + argv[3]); + return EXIT_FAILURE; + } - if (genl_socket_talk(nl, nlh, seq, NULL, NULL) < 0) - perror("genl_socket_talk"); + gtp_del_tunnel(genl_id, nl, t); + gtp_tunnel_free(t); return 0; } struct gtp_pdp { uint32_t version; - uint64_t tid; + union { + struct { + uint64_t tid; + } v0; + struct { + uint32_t i_tid; + uint32_t o_tid; + } v1; + } u; struct in_addr sgsn_addr; struct in_addr ms_addr; }; @@ -134,6 +177,8 @@ static int genl_gtp_validate_cb(const struct nlattr *attr, void *data) return MNL_CB_ERROR; } break; + case GTPA_I_TID: + case GTPA_O_TID: case GTPA_SGSN_ADDRESS: case GTPA_MS_ADDRESS: case GTPA_VERSION: @@ -152,12 +197,16 @@ static int genl_gtp_validate_cb(const struct nlattr *attr, void *data) static int genl_gtp_attr_cb(const struct nlmsghdr *nlh, void *data) { struct nlattr *tb[GTPA_MAX + 1] = {}; - struct gtp_pdp pdp; + struct gtp_pdp pdp = {}; struct genlmsghdr *genl; mnl_attr_parse(nlh, sizeof(*genl), genl_gtp_validate_cb, tb); if (tb[GTPA_TID]) - pdp.tid = mnl_attr_get_u64(tb[GTPA_TID]); + pdp.u.v0.tid = mnl_attr_get_u64(tb[GTPA_TID]); + if (tb[GTPA_I_TID]) + pdp.u.v1.i_tid = mnl_attr_get_u32(tb[GTPA_I_TID]); + if (tb[GTPA_O_TID]) + pdp.u.v1.o_tid = mnl_attr_get_u32(tb[GTPA_O_TID]); if (tb[GTPA_SGSN_ADDRESS]) { pdp.sgsn_addr.s_addr = mnl_attr_get_u32(tb[GTPA_SGSN_ADDRESS]); @@ -170,7 +219,10 @@ static int genl_gtp_attr_cb(const struct nlmsghdr *nlh, void *data) } printf("version %u ", pdp.version); - printf("tid %llx ms_addr %s ", pdp.tid, inet_ntoa(pdp.ms_addr)); + if (pdp.version == GTP_V0) + printf("tid %"PRIu64" ms_addr %s ", pdp.u.v0.tid, inet_ntoa(pdp.sgsn_addr)); + else if (pdp.version == GTP_V1) + printf("tid %u/%u ms_addr %s ", pdp.u.v1.i_tid, pdp.u.v1.o_tid, inet_ntoa(pdp.sgsn_addr)); printf("sgsn_addr %s\n", inet_ntoa(pdp.sgsn_addr)); return MNL_CB_OK; @@ -197,8 +249,6 @@ list_tunnel(int argc, char *argv[], int genl_id, struct mnl_socket *nl) int main(int argc, char *argv[]) { struct mnl_socket *nl; - char buf[MNL_SOCKET_BUFFER_SIZE]; - unsigned int portid; int32_t genl_id; int ret; diff --git a/libgtnl/tools/gtpnl.c b/libgtnl/tools/gtpnl.c deleted file mode 100644 index 252f0b1..0000000 --- a/libgtnl/tools/gtpnl.c +++ /dev/null @@ -1,35 +0,0 @@ -/* Helper functions */ - -/* (C) 2014 by sysmocom - s.f.m.c. GmbH - * Author: Pablo Neira Ayuso <pablo@gnumonks.org> - * - * All Rights Reserved - * - * 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. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - * - */ - -#include <stdint.h> -#include <libmnl/libmnl.h> -#include <linux/gtp_nl.h> - -void gtp_build_payload(struct nlmsghdr *nlh, uint64_t tid, uint32_t ifidx, - uint32_t sgsn_addr, uint32_t ms_addr, uint32_t version) -{ - mnl_attr_put_u32(nlh, GTPA_VERSION, version); - mnl_attr_put_u32(nlh, GTPA_LINK, ifidx); - mnl_attr_put_u32(nlh, GTPA_SGSN_ADDRESS, sgsn_addr); - mnl_attr_put_u32(nlh, GTPA_MS_ADDRESS, ms_addr); - mnl_attr_put_u64(nlh, GTPA_TID, tid); -} -- 2.5.0
On Mon, Nov 16, 2015 at 04:06:55PM +0100, Andreas Schultz wrote:
GTPv1 tunnel use a separate 32bit TIDs for each direction while GTPv0 uses only one 64bit TID.
English nitpicking: "GTPv1 tunnels use separate..." ...and the names differ. In v0, it's called "Tunnel IDentifier"; in v1 they're two "Tunnel Endpoint Identifier"s ("TID" vs. "TEI"). This naming is used consistently in the specs.
--- a/libgtnl/include/libgtpnl/gtp.h +++ b/libgtnl/include/libgtpnl/gtp.h @@ -14,6 +14,8 @@ void gtp_tunnel_set_ms_ip4(struct gtp_tunnel *t, struct in_addr *ms_addr); void gtp_tunnel_set_sgsn_ip4(struct gtp_tunnel *t, struct in_addr *sgsn_addr); void gtp_tunnel_set_version(struct gtp_tunnel *t, uint32_t version); void gtp_tunnel_set_tid(struct gtp_tunnel *t, uint64_t tid); +void gtp_tunnel_set_i_tid(struct gtp_tunnel *t, uint32_t i_tid); +void gtp_tunnel_set_o_tid(struct gtp_tunnel *t, uint32_t o_tid); [...] +uint32_t gtp_tunnel_get_i_tid(struct gtp_tunnel *t); +uint32_t gtp_tunnel_get_o_tid(struct gtp_tunnel *t); [...] + GTPA_I_TID, + GTPA_O_TID, [...] + union { [...] + struct { + uint32_t i_tid; + uint32_t o_tid; + } v1; + } u; [...] + else if (pdp.version == GTP_V1) + printf("tid %u/%u ms_addr %s ", pdp.u.v1.i_tid, pdp.u.v1.o_tid, inet_ntoa(pdp.sgsn_addr));
So I'd call all of these "tei" and "TEI" (leaving v0 ones called "tid").
+static void add_usage(const char *name)
"add" to stdout? :) Ah I see, print the usage for the "add" command....
+{ + printf("%s add <gtp device> <v0> <tid> <ms-addr> <sgsn-addr>\n", + name); + printf("%s add <gtp device> <v1> <i_tid> <o_tid> <ms-addr> <sgsn-addr>\n", + name); +}
"v0" and "v1" are the exact literal arguments to pass, in which case I would omit the braces, like printf("%s add <gtp device> v1 <i_tid> <o_tid> <ms-addr> <sgsn-addr>\n",
+ uint32_t gtp_version; In the GTP headers, there are just three bits available for indicating the GTP version. Any particular reason to pick a uint32_t?
+ t = gtp_tunnel_alloc(); [...] + gtp_tunnel_free(t);
It seems that you calloc and free a struct gtp_tunnel within the same function (I think twice). You could use a local struct instead? struct gtp_tunnel = {0}; And a possible mem leak because of that:
static int add_tunnel(int argc, char *argv[], int genl_id, struct mnl_socket *nl) { + struct gtp_tunnel *t; [...] + t = gtp_tunnel_alloc(); + optidx = 2; + + gtp_ifidx = if_nametoindex(argv[optidx]); if (gtp_ifidx == 0) { - fprintf(stderr, "wrong GTP interface %s\n", argv[2]); + fprintf(stderr, "wrong GTP interface %s\n", argv[optidx]); return EXIT_FAILURE;
t is still allocated upon EXIT_FAILURE.
} + gtp_tunnel_set_ifidx(t, gtp_ifidx);
- if (inet_aton(argv[5], &ms) < 0) { - perror("bad address for ms"); - exit(EXIT_FAILURE); - } - - if (inet_aton(argv[6], &sgsn) < 0) { - perror("bad address for sgsn"); - exit(EXIT_FAILURE); - } + optidx++;
- if (strcmp(argv[3], "v0") == 0) + if (strcmp(argv[optidx], "v0") == 0) gtp_version = GTP_V0; - else if (strcmp(argv[3], "v1") == 0) + else if (strcmp(argv[optidx], "v1") == 0) gtp_version = GTP_V1; else { fprintf(stderr, "wrong GTP version %s, use v0 or v1\n", - argv[3]); + argv[optidx]); return EXIT_FAILURE;
t is still allocated upon EXIT_FAILURE.
} + gtp_tunnel_set_version(t, gtp_version);
- nlh = genl_nlmsg_build_hdr(buf, genl_id, NLM_F_EXCL | NLM_F_ACK, ++seq, - GTP_CMD_TUNNEL_NEW); - gtp_build_payload(nlh, atoi(argv[4]), gtp_ifidx, sgsn.s_addr, - ms.s_addr, gtp_version); + if (gtp_version == GTP_V0) + gtp_tunnel_set_tid(t, atoi(argv[optidx++])); + else if (gtp_version == GTP_V1) { + gtp_tunnel_set_i_tid(t, atoi(argv[optidx++])); + gtp_tunnel_set_o_tid(t, atoi(argv[optidx++])); + }
- if (genl_socket_talk(nl, nlh, seq, NULL, NULL) < 0) - perror("genl_socket_talk"); + if (inet_aton(argv[optidx++], &ms) < 0) { + perror("bad address for ms"); + exit(EXIT_FAILURE);
Some failures return EXIT_FAILURE, some exit() directly? Is that intended?
+ } + gtp_tunnel_set_ms_ip4(t, &ms); + + if (inet_aton(argv[optidx++], &sgsn) < 0) { + perror("bad address for sgsn"); + exit(EXIT_FAILURE); + } + gtp_tunnel_set_sgsn_ip4(t, &sgsn); + + gtp_add_tunnel(genl_id, nl, t);
+ gtp_tunnel_free(t);
Just to make sure ... t's data is copied by gtp_add_tunnel(), right? All in all, seems to be a case for a local struct. Same thing in del_tunnel():
return 0; }
static int del_tunnel(int argc, char *argv[], int genl_id, struct mnl_socket *nl) { + struct gtp_tunnel *t; [...] + t = gtp_tunnel_alloc();
- nlh = genl_nlmsg_build_hdr(buf, genl_id, NLM_F_ACK, ++seq, - GTP_CMD_TUNNEL_DELETE); - gtp_build_payload(nlh, atoi(argv[4]), gtp_ifidx, 0, 0, atoi(argv[3])); + gtp_ifidx = if_nametoindex(argv[2]); + if (gtp_ifidx == 0) { + fprintf(stderr, "wrong GTP interface %s\n", argv[2]); + return EXIT_FAILURE;
possible missing free
+ } + gtp_tunnel_set_ifidx(t, gtp_ifidx); + + if (strcmp(argv[3], "v0") == 0) { + gtp_tunnel_set_version(t, GTP_V0); + gtp_tunnel_set_tid(t, atoi(argv[4])); + } else if (strcmp(argv[3], "v1") == 0) { + gtp_tunnel_set_version(t, GTP_V1); + gtp_tunnel_set_i_tid(t, atoi(argv[4])); + } else { + fprintf(stderr, "wrong GTP version %s, use v0 or v1\n", + argv[3]); + return EXIT_FAILURE;
possible missing free
+ }
- if (genl_socket_talk(nl, nlh, seq, NULL, NULL) < 0) - perror("genl_socket_talk"); + gtp_del_tunnel(genl_id, nl, t);
+ gtp_tunnel_free(t);
...same as above.
struct gtp_pdp { uint32_t version; - uint64_t tid; + union { + struct { + uint64_t tid; + } v0; + struct { + uint32_t i_tid; + uint32_t o_tid; + } v1; + } u;
The same union again? Actually, it seems the entire struct gtp_pdp definition is duplicated. Wouldn't it be better to define it once and reuse?
@@ -152,12 +197,16 @@ static int genl_gtp_validate_cb(const struct nlattr *attr, void *data) static int genl_gtp_attr_cb(const struct nlmsghdr *nlh, void *data) { struct nlattr *tb[GTPA_MAX + 1] = {}; - struct gtp_pdp pdp; + struct gtp_pdp pdp = {};
Heh, exactly :) Actually, we've recently had a discussion in the sysmocom office about this way of zero initialization. There's the "traditional" method of naming one element as zero, after which the remaining elements will also be initialized to nul. If the first member is a primitive, {0} suffices. If not, one has to explicitly name a member, which can be cumbersome. Then I came across the possibility to just omit all members and obviously preferred that: {}. However, that doesn't seem to be part of C99, so now I'm back to naming a member again. Any further insights on the subject would be appreciated... ~Neels
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 1 + libgtnl/AUTHORS | 1 + 2 files changed, 2 insertions(+) diff --git a/gtp.c b/gtp.c index a4be31d..4b233f7 100644 --- a/gtp.c +++ b/gtp.c @@ -3,6 +3,7 @@ /* (C) 2012-2014 by sysmocom - s.f.m.c. GmbH * Author: Harald Welte <hwelte@sysmocom.de> * Pablo Neira Ayuso <pablo@gnumonks.org> + * Andreas Schultz <aschultz@travelping.com> * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License diff --git a/libgtnl/AUTHORS b/libgtnl/AUTHORS index 7deed5b..649d40b 100644 --- a/libgtnl/AUTHORS +++ b/libgtnl/AUTHORS @@ -1 +1,2 @@ Pablo Neira Ayuso <pablo@gnumonks.org> +Andreas Schultz <aschultz@travelping.com> -- 2.5.0
Signed-off-by: Andreas Schultz <aschultz@tpip.net> --- gtp.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/gtp.c b/gtp.c index 4b233f7..2b8738d 100644 --- a/gtp.c +++ b/gtp.c @@ -1080,24 +1080,44 @@ static int ipv4_pdp_add(struct net_device *dev, struct genl_info *info) } if (found) { + struct pdp_ctx *repl_pctx; + if (info->nlhdr->nlmsg_flags & NLM_F_EXCL) return -EEXIST; - if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE) - return -EOPNOTSUPP; - ipv4_pdp_fill(pctx, info); + repl_pctx = kmemdup(pctx, sizeof(struct pdp_ctx), GFP_KERNEL); + if (repl_pctx == NULL) + return -ENOMEM; + + ipv4_pdp_fill(repl_pctx, info); + + /* only the SGSN can be changed */ + if (pctx->af != repl_pctx->af || + pctx->gtp_version != repl_pctx->gtp_version || + memcpy(&pctx->u, &repl_pctx->u, sizeof(pctx->u) != 0)) { + kfree(repl_pctx); + return -EINVAL; + } + + hlist_replace_rcu(&pctx->hlist_addr, &repl_pctx->hlist_addr); + hlist_replace_rcu(&pctx->hlist_tid, &repl_pctx->hlist_tid); - if (pctx->gtp_version == GTP_V0) + kfree_rcu(pctx, rcu_head); + + if (repl_pctx->gtp_version == GTP_V0) netdev_dbg(dev, "GTPv0-U: update tunnel id = %llx (pdp %p)\n", - pctx->u.v0.tid, pctx); - else if (pctx->gtp_version == GTP_V1) + repl_pctx->u.v0.tid, repl_pctx); + else if (repl_pctx->gtp_version == GTP_V1) netdev_dbg(dev, "GTPv1-U: update tunnel id = %x/%x (pdp %p)\n", - pctx->u.v1.i_tid, pctx->u.v1.o_tid, pctx); + repl_pctx->u.v1.i_tid, repl_pctx->u.v1.o_tid, repl_pctx); return 0; } + if (info->nlhdr->nlmsg_flags & NLM_F_REPLACE) + return -ENXIO; + pctx = kmalloc(sizeof(struct pdp_ctx), GFP_KERNEL); if (pctx == NULL) return -ENOMEM; -- 2.5.0
Hi Andreas, On Mon, Nov 16, 2015 at 04:06:41PM +0100, Andreas Schultz wrote:
After this series, the GTP kernel module is functional for IPv4 payload in IPv4 GTP tunnels (for GTPv0 and v1).
Thanks a lot. I did a brief review of the patches and they look fine to me. Pablo has more (recent) experience in Linux kernel networking code these days, so I'd like to wait for his feedback on this.
The libgtnl API was changed without increasing the SO version of the lib. I think this is ok since nothing really uses it (yet) nor has the library been offcially released.
agreed. 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 Mon, Nov 16, 2015 at 04:06:41PM +0100, Andreas Schultz wrote:
Hi,
Sorry for smsching this all together, but I couldn't find a logical point to split the patches. Also the order can't really be changed as most patches depend on each other.
I have reviewed and applied as much as I could, please follow up with another incremental update.
After this series, the GTP kernel module is functional for IPv4 payload in IPv4 GTP tunnels (for GTPv0 and v1).
The libgtnl API was changed without increasing the SO version of the lib. I think this is ok since nothing really uses it (yet) nor has the library been offcially released.
Then, why update it? Thanks.
participants (4)
-
Andreas Schultz -
Harald Welte -
Neels Hofmeyr -
Pablo Neira Ayuso