diff mbox

[14/16] gtp-rtnl: Split tid handling for GTPv0 and GTPv1

Message ID 1447686417-3979-15-git-send-email-aschultz@tpip.net
State Changes Requested
Delegated to: Pablo Neira
Headers show

Commit Message

Andreas Schultz Nov. 16, 2015, 3:06 p.m. UTC 
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

Comments

Neels Hofmeyr Nov. 19, 2015, 2 p.m. UTC | #1 
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
diff mbox

Patch

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);
-}