All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2-next v3 0/2] GTP support for ip link and tc flower
@ 2022-02-11 18:29 Wojciech Drewek
  2022-02-11 18:29 ` [PATCH iproute2-next v3 1/2] ip: GTP support in ip link Wojciech Drewek
  2022-02-11 18:29 ` [PATCH iproute2-next v3 2/2] f_flower: Implement gtp options support Wojciech Drewek
  0 siblings, 2 replies; 7+ messages in thread
From: Wojciech Drewek @ 2022-02-11 18:29 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, stephen, laforge

This patch series introduces GTP support to iproute2. Since this patch
series it is possible to create net devices of GTP type. Then, those
devices can be used in tc in order to offload GTP packets. New field
in tc flower (gtp_opts) can be used to match on QFI and PDU type.

Kernel changes:
https://lore.kernel.org/netdev/20220211175405.7651-1-marcin.szycik@linux.intel.com/T/#t

Wojciech Drewek (2):
  ip: GTP support in ip link
  f_flower: Implement gtp options support

 include/uapi/linux/if_link.h |   2 +
 include/uapi/linux/pkt_cls.h |  16 +++++
 ip/Makefile                  |   2 +-
 ip/iplink.c                  |   2 +-
 ip/iplink_gtp.c              | 128 +++++++++++++++++++++++++++++++++++
 man/man8/ip-link.8.in        |  29 +++++++-
 man/man8/tc-flower.8         |  10 +++
 tc/f_flower.c                | 122 ++++++++++++++++++++++++++++++++-
 8 files changed, 306 insertions(+), 5 deletions(-)
 create mode 100644 ip/iplink_gtp.c

-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next v3 1/2] ip: GTP support in ip link
  2022-02-11 18:29 [PATCH iproute2-next v3 0/2] GTP support for ip link and tc flower Wojciech Drewek
@ 2022-02-11 18:29 ` Wojciech Drewek
  2022-02-12  8:27   ` Harald Welte
  2022-02-27  6:57   ` Jonas Bonn
  2022-02-11 18:29 ` [PATCH iproute2-next v3 2/2] f_flower: Implement gtp options support Wojciech Drewek
  1 sibling, 2 replies; 7+ messages in thread
From: Wojciech Drewek @ 2022-02-11 18:29 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, stephen, laforge

Support for creating GTP devices through ip link. Two arguments
can be specified by the user when adding device of the GTP type.
 - role (sgsn or ggsn) - indicates whether we are on the GGSN or SGSN
 - hsize - indicates the size of the hash table where PDP sessions
   are stored

IFLA_GTP_FD0 and IFLA_GTP_FD1 arguments would not be provided. Those
are file descriptores to the sockets created in the userspace. Since
we are not going to create sockets in ip link, we don't have to
provide them.

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v2: use SPDX tag, use strcmp() instead of matches(), parse
    IFLA_GTP_RESTART_COUNT arg
v3: IFLA_GTP_CREATE_SOCKETS attribute introduced, fix options
    alpha order
---
 include/uapi/linux/if_link.h |   2 +
 ip/Makefile                  |   2 +-
 ip/iplink.c                  |   2 +-
 ip/iplink_gtp.c              | 128 +++++++++++++++++++++++++++++++++++
 man/man8/ip-link.8.in        |  29 +++++++-
 5 files changed, 160 insertions(+), 3 deletions(-)
 create mode 100644 ip/iplink_gtp.c

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 41708e26a3c9..c8ed41ee4efd 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -820,6 +820,8 @@ enum {
 	IFLA_GTP_FD1,
 	IFLA_GTP_PDP_HASHSIZE,
 	IFLA_GTP_ROLE,
+	IFLA_GTP_CREATE_SOCKETS,
+	IFLA_GTP_RESTART_COUNT,
 	__IFLA_GTP_MAX,
 };
 #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1)
diff --git a/ip/Makefile b/ip/Makefile
index 2a7a51c313c6..06ba60b341af 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -12,7 +12,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
     iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
     ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
     ipnexthop.o ipmptcp.o iplink_bareudp.o iplink_wwan.o ipioam6.o \
-    iplink_amt.o
+    iplink_amt.o iplink_gtp.o
 
 RTMONOBJ=rtmon.o
 
diff --git a/ip/iplink.c b/ip/iplink.c
index c0a3a9ad3e62..1fe163794d35 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -51,7 +51,7 @@ void iplink_types_usage(void)
 	/* Remember to add new entry here if new type is added. */
 	fprintf(stderr,
 		"TYPE := { amt | bareudp | bond | bond_slave | bridge | bridge_slave |\n"
-		"          dummy | erspan | geneve | gre | gretap | ifb |\n"
+		"          dummy | erspan | geneve | gre | gretap | gtp | ifb |\n"
 		"          ip6erspan | ip6gre | ip6gretap | ip6tnl |\n"
 		"          ipip | ipoib | ipvlan | ipvtap |\n"
 		"          macsec | macvlan | macvtap |\n"
diff --git a/ip/iplink_gtp.c b/ip/iplink_gtp.c
new file mode 100644
index 000000000000..6ba684876a66
--- /dev/null
+++ b/ip/iplink_gtp.c
@@ -0,0 +1,128 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <stdio.h>
+
+#include "rt_names.h"
+#include "utils.h"
+#include "ip_common.h"
+
+#define GTP_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
+
+static void print_explain(FILE *f)
+{
+	fprintf(f,
+		"Usage: ... gtp role ROLE\n"
+		"		[ hsize HSIZE ]\n"
+		"		[ restart_count RESTART_COUNT ]\n"
+		"\n"
+		"Where:	ROLE		:= { sgsn | ggsn }\n"
+		"	HSIZE		:= 1-131071\n"
+		"	RESTART_COUNT	:= 0-255\n"
+	);
+}
+
+static void check_duparg(__u32 *attrs, int type, const char *key,
+			 const char *argv)
+{
+	if (!GTP_ATTRSET(*attrs, type)) {
+		*attrs |= (1L << type);
+		return;
+	}
+	duparg2(key, argv);
+}
+
+static int gtp_parse_opt(struct link_util *lu, int argc, char **argv,
+			 struct nlmsghdr *n)
+{
+	__u32 attrs = 0;
+
+	/* When creating GTP device through ip link,
+	 * this flag has to be set.
+	 */
+	addattr8(n, 1024, IFLA_GTP_CREATE_SOCKETS, true);
+
+	while (argc > 0) {
+		if (!strcmp(*argv, "role")) {
+			NEXT_ARG();
+			check_duparg(&attrs, IFLA_GTP_ROLE, "role", *argv);
+			if (!strcmp(*argv, "sgsn"))
+				addattr32(n, 1024, IFLA_GTP_ROLE, GTP_ROLE_SGSN);
+			else if (!strcmp(*argv, "ggsn"))
+				addattr32(n, 1024, IFLA_GTP_ROLE, GTP_ROLE_GGSN);
+			else
+				invarg("invalid role, use sgsn or ggsn", *argv);
+		} else if (!strcmp(*argv, "hsize")) {
+			__u32 hsize;
+
+			NEXT_ARG();
+			check_duparg(&attrs, IFLA_GTP_PDP_HASHSIZE, "hsize", *argv);
+
+			if (get_u32(&hsize, *argv, 0))
+				invarg("invalid PDP hash size", *argv);
+			if (hsize >= 1u << 17)
+				invarg("PDP hash size too big", *argv);
+			addattr32(n, 1024, IFLA_GTP_PDP_HASHSIZE, hsize);
+		} else if (!strcmp(*argv, "restart_count")) {
+			__u8 restart_count;
+
+			NEXT_ARG();
+			check_duparg(&attrs, IFLA_GTP_RESTART_COUNT, "restart_count", *argv);
+
+			if (get_u8(&restart_count, *argv, 10))
+				invarg("invalid restart_count", *argv);
+			addattr8(n, 1024, IFLA_GTP_RESTART_COUNT, restart_count);
+		} else if (!strcmp(*argv, "help")) {
+			print_explain(stderr);
+			return -1;
+		}
+		argc--, argv++;
+	}
+
+	if (!GTP_ATTRSET(attrs, IFLA_GTP_ROLE)) {
+		fprintf(stderr, "gtp: role of the gtp device was not specified\n");
+		return -1;
+	}
+
+	if (!GTP_ATTRSET(attrs, IFLA_GTP_PDP_HASHSIZE))
+		addattr32(n, 1024, IFLA_GTP_PDP_HASHSIZE, 1024);
+
+	return 0;
+}
+
+static void gtp_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
+{
+
+	if (tb[IFLA_GTP_ROLE]) {
+		__u32 role = rta_getattr_u32(tb[IFLA_GTP_ROLE]);
+
+		print_string(PRINT_ANY, "role", "role %s ",
+			     role == GTP_ROLE_SGSN ? "sgsn" : "ggsn");
+	}
+
+	if (tb[IFLA_GTP_PDP_HASHSIZE]) {
+		__u32 hsize = rta_getattr_u32(tb[IFLA_GTP_PDP_HASHSIZE]);
+
+		print_uint(PRINT_ANY, "hsize", "hsize %u ", hsize);
+	}
+
+	if (tb[IFLA_GTP_RESTART_COUNT]) {
+		__u8 restart_count = rta_getattr_u8(tb[IFLA_GTP_RESTART_COUNT]);
+
+		print_uint(PRINT_ANY, "restart_count",
+			   "restart_count %u ", restart_count);
+	}
+}
+
+static void gtp_print_help(struct link_util *lu, int argc, char **argv,
+			   FILE *f)
+{
+	print_explain(f);
+}
+
+struct link_util gtp_link_util = {
+	.id		= "gtp",
+	.maxattr	= IFLA_GTP_MAX,
+	.parse_opt	= gtp_parse_opt,
+	.print_opt	= gtp_print_opt,
+	.print_help	= gtp_print_help,
+};
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 5f5b835cb2e3..56b8c7b2917e 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -243,7 +243,8 @@ ip-link \- network device configuration
 .BR macsec " |"
 .BR netdevsim " |"
 .BR rmnet " |"
-.BR xfrm " ]"
+.BR xfrm " |"
+.BR gtp " ]"
 
 .ti -8
 .IR ETYPE " := [ " TYPE " |"
@@ -392,6 +393,9 @@ Link types:
 .sp
 .BR xfrm
 - Virtual xfrm interface
+.sp
+.BR gtp
+- GPRS Tunneling Protocol
 .in -8
 
 .TP
@@ -1941,6 +1945,29 @@ policies. Policies must be configured with the same key. If not set, the key def
 
 .in -8
 
+.TP
+GTP Type Support
+For a link of type
+.I GTP
+the following additional arguments are supported:
+
+.BI "ip link add " DEVICE " type gtp role " ROLE " hsize " HSIZE
+
+.in +8
+.sp
+.BI role " ROLE "
+- specifies the role of the GTP device, either sgsn or ggsn
+
+.sp
+.BI hsize " HSIZE "
+- specifies size of the hashtable which stores PDP contexts
+
+.sp
+.BI restart_count " RESTART_COUNT "
+- GTP instance restart counter
+
+.in -8
+
 .SS ip link delete - delete virtual link
 
 .TP
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH iproute2-next v3 2/2] f_flower: Implement gtp options support
  2022-02-11 18:29 [PATCH iproute2-next v3 0/2] GTP support for ip link and tc flower Wojciech Drewek
  2022-02-11 18:29 ` [PATCH iproute2-next v3 1/2] ip: GTP support in ip link Wojciech Drewek
@ 2022-02-11 18:29 ` Wojciech Drewek
  1 sibling, 0 replies; 7+ messages in thread
From: Wojciech Drewek @ 2022-02-11 18:29 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, stephen, laforge

Add support for parsing TCA_FLOWER_KEY_ENC_OPTS_GTP.
Options are as follows: PDU_TYPE:QFI where each
option is represented as 8-bit hexadecimal value.

e.g.
  # ip link add gtp_dev type gtp role sgsn
  # tc qdisc add dev gtp_dev ingress
  # tc filter add dev gtp_dev protocol ip parent ffff: \
      flower \
        enc_key_id 11 \
        gtp_opts 1:8/ff:ff \
      action mirred egress redirect dev eth0

Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
v2: get rid of JSON specific code
v3: use snprintf in flower_print_gtp_opts
---
 include/uapi/linux/pkt_cls.h |  16 +++++
 man/man8/tc-flower.8         |  10 +++
 tc/f_flower.c                | 122 ++++++++++++++++++++++++++++++++++-
 3 files changed, 146 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index ee38b35c3f57..30ff8da0631b 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -616,6 +616,10 @@ enum {
 					 * TCA_FLOWER_KEY_ENC_OPT_ERSPAN_
 					 * attributes
 					 */
+	TCA_FLOWER_KEY_ENC_OPTS_GTP,	/* Nested
+					 * TCA_FLOWER_KEY_ENC_OPT_GTP_
+					 * attributes
+					 */
 	__TCA_FLOWER_KEY_ENC_OPTS_MAX,
 };
 
@@ -654,6 +658,18 @@ enum {
 #define TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX \
 		(__TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX - 1)
 
+enum {
+	TCA_FLOWER_KEY_ENC_OPT_GTP_UNSPEC,
+	TCA_FLOWER_KEY_ENC_OPT_GTP_PDU_TYPE,		/* u8 */
+	TCA_FLOWER_KEY_ENC_OPT_GTP_QFI,			/* u8 */
+
+	__TCA_FLOWER_KEY_ENC_OPT_GTP_MAX,
+};
+
+#define TCA_FLOWER_KEY_ENC_OPT_GTP_MAX \
+		(__TCA_FLOWER_KEY_ENC_OPT_GTP_MAX - 1)
+
+
 enum {
 	TCA_FLOWER_KEY_MPLS_OPTS_UNSPEC,
 	TCA_FLOWER_KEY_MPLS_OPTS_LSE,
diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 4541d9372684..f918a06d2927 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -89,6 +89,8 @@ flower \- flow based traffic control filter
 .B vxlan_opts
 |
 .B erspan_opts
+|
+.B gtp_opts
 }
 .IR OPTIONS " | "
 .BR ip_flags
@@ -411,6 +413,8 @@ Match the connection zone, and can be masked.
 .BI vxlan_opts " OPTIONS"
 .TQ
 .BI erspan_opts " OPTIONS"
+.TQ
+.BI gtp_opts " OPTIONS"
 Match on IP tunnel metadata. Key id
 .I NUMBER
 is a 32 bit tunnel key id (e.g. VNI for VXLAN tunnel).
@@ -446,6 +450,12 @@ VERSION:INDEX:DIR:HWID/VERSION:INDEX_MASK:DIR_MASK:HWID_MASK, where VERSION is
 represented as a 8bit number, INDEX as an 32bit number, DIR and HWID as a 8bit
 number. Multiple options is not supported. Note INDEX/INDEX_MASK is used when
 VERSION is 1, and DIR/DIR_MASK and HWID/HWID_MASK are used when VERSION is 2.
+gtp_opts
+.I OPTIONS
+doesn't support multiple options, and it consists of a key followed by a slash
+and corresponding mask. If the mask is missing, \fBtc\fR assumes a full-length
+match. The option can be described in the form PDU_TYPE:QFI/PDU_TYPE_MASK:QFI_MASK
+where both PDU_TYPE and QFI are represented as a 8bit hexadecimal values.
 .TP
 .BI ip_flags " IP_FLAGS"
 .I IP_FLAGS
diff --git a/tc/f_flower.c b/tc/f_flower.c
index d3f79bdf4252..1ff8341d27a6 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -1028,6 +1028,52 @@ static int flower_parse_erspan_opt(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int flower_parse_gtp_opt(char *str, struct nlmsghdr *n)
+{
+	struct rtattr *nest;
+	char *token;
+	int arg, err;
+
+	nest = addattr_nest(n, MAX_MSG, TCA_FLOWER_KEY_ENC_OPTS_GTP | NLA_F_NESTED);
+
+	token = strsep(&str, ":");
+	for (arg = 1; arg <= TCA_FLOWER_KEY_ENC_OPT_GTP_MAX; arg++) {
+		switch (arg) {
+		case TCA_FLOWER_KEY_ENC_OPT_GTP_PDU_TYPE:
+		{
+			__u8 pdu_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&pdu_type, token, 16);
+			if (err)
+				return err;
+			addattr8(n, MAX_MSG, arg, pdu_type);
+			break;
+		}
+		case TCA_FLOWER_KEY_ENC_OPT_GTP_QFI:
+		{
+			__u8 qfi;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&qfi, token, 16);
+			if (err)
+				return err;
+			addattr8(n, MAX_MSG, arg, qfi);
+			break;
+		}
+		default:
+			fprintf(stderr, "Unknown \"gtp_opts\" type\n");
+			return -1;
+		}
+		token = strsep(&str, ":");
+	}
+	addattr_nest_end(n, nest);
+
+	return 0;
+}
+
 static int flower_parse_geneve_opts(char *str, struct nlmsghdr *n)
 {
 	char *token;
@@ -1211,6 +1257,41 @@ static int flower_parse_enc_opts_erspan(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int flower_parse_enc_opts_gtp(char *str, struct nlmsghdr *n)
+{
+	char key[XATTR_SIZE_MAX], mask[XATTR_SIZE_MAX];
+	struct rtattr *nest;
+	char *slash;
+	int err;
+
+	slash = strchr(str, '/');
+	if (slash) {
+		*slash++ = '\0';
+		if (strlen(slash) > XATTR_SIZE_MAX)
+			return -1;
+		strcpy(mask, slash);
+	} else
+		strcpy(mask, "ff:ff");
+
+	if (strlen(str) > XATTR_SIZE_MAX)
+		return -1;
+	strcpy(key, str);
+
+	nest = addattr_nest(n, MAX_MSG, TCA_FLOWER_KEY_ENC_OPTS | NLA_F_NESTED);
+	err = flower_parse_gtp_opt(key, n);
+	if (err)
+		return err;
+	addattr_nest_end(n, nest);
+
+	nest = addattr_nest(n, MAX_MSG, TCA_FLOWER_KEY_ENC_OPTS_MASK | NLA_F_NESTED);
+	err = flower_parse_gtp_opt(mask, n);
+	if (err)
+		return err;
+	addattr_nest_end(n, nest);
+
+	return 0;
+}
+
 static int flower_parse_mpls_lse(int *argc_p, char ***argv_p,
 				 struct nlmsghdr *nlh)
 {
@@ -1863,6 +1944,13 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 				fprintf(stderr, "Illegal \"erspan_opts\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "gtp_opts") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_enc_opts_gtp(*argv, n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"gtp_opts\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "action") == 0) {
 			NEXT_ARG();
 			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -2332,6 +2420,21 @@ static void flower_print_erspan_opts(const char *name, struct rtattr *attr,
 	sprintf(strbuf, "%u:%u:%u:%u", ver, idx, dir, hwid);
 }
 
+static void flower_print_gtp_opts(const char *name, struct rtattr *attr,
+				  char *strbuf, int len)
+{
+	struct rtattr *tb[TCA_FLOWER_KEY_ENC_OPT_GTP_MAX + 1];
+	__u8 pdu_type, qfi;
+
+	parse_rtattr(tb, TCA_FLOWER_KEY_ENC_OPT_GTP_MAX, RTA_DATA(attr),
+		     RTA_PAYLOAD(attr));
+
+	pdu_type = rta_getattr_u8(tb[TCA_FLOWER_KEY_ENC_OPT_GTP_PDU_TYPE]);
+	qfi = rta_getattr_u8(tb[TCA_FLOWER_KEY_ENC_OPT_GTP_QFI]);
+
+	snprintf(strbuf, len, "%02x:%02x", pdu_type, qfi);
+}
+
 static void __attribute__((format(printf, 2, 0)))
 flower_print_enc_parts(const char *name, const char *namefrm,
 		       struct rtattr *attr, char *key, char *mask)
@@ -2364,15 +2467,18 @@ static void flower_print_enc_opts(const char *name, struct rtattr *attr,
 	struct rtattr *key_tb[TCA_FLOWER_KEY_ENC_OPTS_MAX + 1];
 	struct rtattr *msk_tb[TCA_FLOWER_KEY_ENC_OPTS_MAX + 1];
 	char *key, *msk;
+	int len;
 
 	if (!attr)
 		return;
 
-	key = malloc(RTA_PAYLOAD(attr) * 2 + 1);
+	len = RTA_PAYLOAD(attr) * 2 + 1;
+
+	key = malloc(len);
 	if (!key)
 		return;
 
-	msk = malloc(RTA_PAYLOAD(attr) * 2 + 1);
+	msk = malloc(len);
 	if (!msk)
 		goto err_key_free;
 
@@ -2409,6 +2515,18 @@ static void flower_print_enc_opts(const char *name, struct rtattr *attr,
 
 		flower_print_enc_parts(name, "  erspan_opts %s", attr, key,
 				       msk);
+	} else if (key_tb[TCA_FLOWER_KEY_ENC_OPTS_GTP]) {
+		flower_print_gtp_opts("gtp_opt_key",
+				      key_tb[TCA_FLOWER_KEY_ENC_OPTS_GTP],
+				      key, len);
+
+		if (msk_tb[TCA_FLOWER_KEY_ENC_OPTS_GTP])
+			flower_print_gtp_opts("gtp_opt_mask",
+					      msk_tb[TCA_FLOWER_KEY_ENC_OPTS_GTP],
+					      msk, len);
+
+		flower_print_enc_parts(name, "  gtp_opts %s", attr, key,
+				       msk);
 	}
 
 	free(msk);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next v3 1/2] ip: GTP support in ip link
  2022-02-11 18:29 ` [PATCH iproute2-next v3 1/2] ip: GTP support in ip link Wojciech Drewek
@ 2022-02-12  8:27   ` Harald Welte
  2022-02-27  6:57   ` Jonas Bonn
  1 sibling, 0 replies; 7+ messages in thread
From: Harald Welte @ 2022-02-12  8:27 UTC (permalink / raw)
  To: Wojciech Drewek; +Cc: netdev, dsahern, stephen

Hi Wojciech,

I'm not an iproute2 developer, but LGTM.

On Fri, Feb 11, 2022 at 07:29:01PM +0100, Wojciech Drewek wrote:
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>

Reviewed-by: Harald Welte <laforge@gnumonks.org>

-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next v3 1/2] ip: GTP support in ip link
  2022-02-11 18:29 ` [PATCH iproute2-next v3 1/2] ip: GTP support in ip link Wojciech Drewek
  2022-02-12  8:27   ` Harald Welte
@ 2022-02-27  6:57   ` Jonas Bonn
  2022-02-27  7:55     ` Harald Welte
  1 sibling, 1 reply; 7+ messages in thread
From: Jonas Bonn @ 2022-02-27  6:57 UTC (permalink / raw)
  To: Wojciech Drewek, netdev; +Cc: dsahern, stephen, laforge

Hi,

On 11/02/2022 19:29, Wojciech Drewek wrote:
> Support for creating GTP devices through ip link. Two arguments
> can be specified by the user when adding device of the GTP type.
>   - role (sgsn or ggsn) - indicates whether we are on the GGSN or SGSN

It would be really nice to modernize these names before exposing this API.

When I added the role property to the driver, it was largely to 
complement the behaviour of the OpenGGSN library, who was essentially 
the only user of this module at the time.  However, even at that time 
the choice of name was awkward because we were well into the 4G era so 
SGSN/GGSN was already somewhat legacy terminology; today, these terms 
are starting to raise some eyebrows amongst younger developers who may 
be well versed in 4G/5G, but for whom 3G is somewhat ancient history.

3GPP has a well-accepted definition of "uplink" and "downlink" which is 
probably what we should be using instead.  So sgsn becomes "uplink" and 
ggsn becomes "downlink", with the distinction here being whether packets 
are routed by source or destination IP address.

/Jonas




>   - hsize - indicates the size of the hash table where PDP sessions
>     are stored
> 
> IFLA_GTP_FD0 and IFLA_GTP_FD1 arguments would not be provided. Those
> are file descriptores to the sockets created in the userspace. Since
> we are not going to create sockets in ip link, we don't have to
> provide them.
> 
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
> v2: use SPDX tag, use strcmp() instead of matches(), parse
>      IFLA_GTP_RESTART_COUNT arg
> v3: IFLA_GTP_CREATE_SOCKETS attribute introduced, fix options
>      alpha order
> ---
>   include/uapi/linux/if_link.h |   2 +
>   ip/Makefile                  |   2 +-
>   ip/iplink.c                  |   2 +-
>   ip/iplink_gtp.c              | 128 +++++++++++++++++++++++++++++++++++
>   man/man8/ip-link.8.in        |  29 +++++++-
>   5 files changed, 160 insertions(+), 3 deletions(-)
>   create mode 100644 ip/iplink_gtp.c
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 41708e26a3c9..c8ed41ee4efd 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -820,6 +820,8 @@ enum {
>   	IFLA_GTP_FD1,
>   	IFLA_GTP_PDP_HASHSIZE,
>   	IFLA_GTP_ROLE,
> +	IFLA_GTP_CREATE_SOCKETS,
> +	IFLA_GTP_RESTART_COUNT,
>   	__IFLA_GTP_MAX,
>   };
>   #define IFLA_GTP_MAX (__IFLA_GTP_MAX - 1)
> diff --git a/ip/Makefile b/ip/Makefile
> index 2a7a51c313c6..06ba60b341af 100644
> --- a/ip/Makefile
> +++ b/ip/Makefile
> @@ -12,7 +12,7 @@ IPOBJ=ip.o ipaddress.o ipaddrlabel.o iproute.o iprule.o ipnetns.o \
>       iplink_geneve.o iplink_vrf.o iproute_lwtunnel.o ipmacsec.o ipila.o \
>       ipvrf.o iplink_xstats.o ipseg6.o iplink_netdevsim.o iplink_rmnet.o \
>       ipnexthop.o ipmptcp.o iplink_bareudp.o iplink_wwan.o ipioam6.o \
> -    iplink_amt.o
> +    iplink_amt.o iplink_gtp.o
>   
>   RTMONOBJ=rtmon.o
>   
> diff --git a/ip/iplink.c b/ip/iplink.c
> index c0a3a9ad3e62..1fe163794d35 100644
> --- a/ip/iplink.c
> +++ b/ip/iplink.c
> @@ -51,7 +51,7 @@ void iplink_types_usage(void)
>   	/* Remember to add new entry here if new type is added. */
>   	fprintf(stderr,
>   		"TYPE := { amt | bareudp | bond | bond_slave | bridge | bridge_slave |\n"
> -		"          dummy | erspan | geneve | gre | gretap | ifb |\n"
> +		"          dummy | erspan | geneve | gre | gretap | gtp | ifb |\n"
>   		"          ip6erspan | ip6gre | ip6gretap | ip6tnl |\n"
>   		"          ipip | ipoib | ipvlan | ipvtap |\n"
>   		"          macsec | macvlan | macvtap |\n"
> diff --git a/ip/iplink_gtp.c b/ip/iplink_gtp.c
> new file mode 100644
> index 000000000000..6ba684876a66
> --- /dev/null
> +++ b/ip/iplink_gtp.c
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <stdio.h>
> +
> +#include "rt_names.h"
> +#include "utils.h"
> +#include "ip_common.h"
> +
> +#define GTP_ATTRSET(attrs, type) (((attrs) & (1L << (type))) != 0)
> +
> +static void print_explain(FILE *f)
> +{
> +	fprintf(f,
> +		"Usage: ... gtp role ROLE\n"
> +		"		[ hsize HSIZE ]\n"
> +		"		[ restart_count RESTART_COUNT ]\n"
> +		"\n"
> +		"Where:	ROLE		:= { sgsn | ggsn }\n"
> +		"	HSIZE		:= 1-131071\n"
> +		"	RESTART_COUNT	:= 0-255\n"
> +	);
> +}
> +
> +static void check_duparg(__u32 *attrs, int type, const char *key,
> +			 const char *argv)
> +{
> +	if (!GTP_ATTRSET(*attrs, type)) {
> +		*attrs |= (1L << type);
> +		return;
> +	}
> +	duparg2(key, argv);
> +}
> +
> +static int gtp_parse_opt(struct link_util *lu, int argc, char **argv,
> +			 struct nlmsghdr *n)
> +{
> +	__u32 attrs = 0;
> +
> +	/* When creating GTP device through ip link,
> +	 * this flag has to be set.
> +	 */
> +	addattr8(n, 1024, IFLA_GTP_CREATE_SOCKETS, true);
> +
> +	while (argc > 0) {
> +		if (!strcmp(*argv, "role")) {
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_GTP_ROLE, "role", *argv);
> +			if (!strcmp(*argv, "sgsn"))
> +				addattr32(n, 1024, IFLA_GTP_ROLE, GTP_ROLE_SGSN);
> +			else if (!strcmp(*argv, "ggsn"))
> +				addattr32(n, 1024, IFLA_GTP_ROLE, GTP_ROLE_GGSN);
> +			else
> +				invarg("invalid role, use sgsn or ggsn", *argv);
> +		} else if (!strcmp(*argv, "hsize")) {
> +			__u32 hsize;
> +
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_GTP_PDP_HASHSIZE, "hsize", *argv);
> +
> +			if (get_u32(&hsize, *argv, 0))
> +				invarg("invalid PDP hash size", *argv);
> +			if (hsize >= 1u << 17)
> +				invarg("PDP hash size too big", *argv);
> +			addattr32(n, 1024, IFLA_GTP_PDP_HASHSIZE, hsize);
> +		} else if (!strcmp(*argv, "restart_count")) {
> +			__u8 restart_count;
> +
> +			NEXT_ARG();
> +			check_duparg(&attrs, IFLA_GTP_RESTART_COUNT, "restart_count", *argv);
> +
> +			if (get_u8(&restart_count, *argv, 10))
> +				invarg("invalid restart_count", *argv);
> +			addattr8(n, 1024, IFLA_GTP_RESTART_COUNT, restart_count);
> +		} else if (!strcmp(*argv, "help")) {
> +			print_explain(stderr);
> +			return -1;
> +		}
> +		argc--, argv++;
> +	}
> +
> +	if (!GTP_ATTRSET(attrs, IFLA_GTP_ROLE)) {
> +		fprintf(stderr, "gtp: role of the gtp device was not specified\n");
> +		return -1;
> +	}
> +
> +	if (!GTP_ATTRSET(attrs, IFLA_GTP_PDP_HASHSIZE))
> +		addattr32(n, 1024, IFLA_GTP_PDP_HASHSIZE, 1024);
> +
> +	return 0;
> +}
> +
> +static void gtp_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
> +{
> +
> +	if (tb[IFLA_GTP_ROLE]) {
> +		__u32 role = rta_getattr_u32(tb[IFLA_GTP_ROLE]);
> +
> +		print_string(PRINT_ANY, "role", "role %s ",
> +			     role == GTP_ROLE_SGSN ? "sgsn" : "ggsn");
> +	}
> +
> +	if (tb[IFLA_GTP_PDP_HASHSIZE]) {
> +		__u32 hsize = rta_getattr_u32(tb[IFLA_GTP_PDP_HASHSIZE]);
> +
> +		print_uint(PRINT_ANY, "hsize", "hsize %u ", hsize);
> +	}
> +
> +	if (tb[IFLA_GTP_RESTART_COUNT]) {
> +		__u8 restart_count = rta_getattr_u8(tb[IFLA_GTP_RESTART_COUNT]);
> +
> +		print_uint(PRINT_ANY, "restart_count",
> +			   "restart_count %u ", restart_count);
> +	}
> +}
> +
> +static void gtp_print_help(struct link_util *lu, int argc, char **argv,
> +			   FILE *f)
> +{
> +	print_explain(f);
> +}
> +
> +struct link_util gtp_link_util = {
> +	.id		= "gtp",
> +	.maxattr	= IFLA_GTP_MAX,
> +	.parse_opt	= gtp_parse_opt,
> +	.print_opt	= gtp_print_opt,
> +	.print_help	= gtp_print_help,
> +};
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index 5f5b835cb2e3..56b8c7b2917e 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -243,7 +243,8 @@ ip-link \- network device configuration
>   .BR macsec " |"
>   .BR netdevsim " |"
>   .BR rmnet " |"
> -.BR xfrm " ]"
> +.BR xfrm " |"
> +.BR gtp " ]"
>   
>   .ti -8
>   .IR ETYPE " := [ " TYPE " |"
> @@ -392,6 +393,9 @@ Link types:
>   .sp
>   .BR xfrm
>   - Virtual xfrm interface
> +.sp
> +.BR gtp
> +- GPRS Tunneling Protocol
>   .in -8
>   
>   .TP
> @@ -1941,6 +1945,29 @@ policies. Policies must be configured with the same key. If not set, the key def
>   
>   .in -8
>   
> +.TP
> +GTP Type Support
> +For a link of type
> +.I GTP
> +the following additional arguments are supported:
> +
> +.BI "ip link add " DEVICE " type gtp role " ROLE " hsize " HSIZE
> +
> +.in +8
> +.sp
> +.BI role " ROLE "
> +- specifies the role of the GTP device, either sgsn or ggsn
> +
> +.sp
> +.BI hsize " HSIZE "
> +- specifies size of the hashtable which stores PDP contexts
> +
> +.sp
> +.BI restart_count " RESTART_COUNT "
> +- GTP instance restart counter
> +
> +.in -8
> +
>   .SS ip link delete - delete virtual link
>   
>   .TP


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next v3 1/2] ip: GTP support in ip link
  2022-02-27  6:57   ` Jonas Bonn
@ 2022-02-27  7:55     ` Harald Welte
  2022-02-27  9:12       ` Jonas Bonn
  0 siblings, 1 reply; 7+ messages in thread
From: Harald Welte @ 2022-02-27  7:55 UTC (permalink / raw)
  To: Jonas Bonn; +Cc: Wojciech Drewek, netdev, dsahern, stephen

Dear Jonas,

On Sun, Feb 27, 2022 at 07:57:02AM +0100, Jonas Bonn wrote:
> On 11/02/2022 19:29, Wojciech Drewek wrote:
> > Support for creating GTP devices through ip link. Two arguments
> > can be specified by the user when adding device of the GTP type.
> >   - role (sgsn or ggsn) - indicates whether we are on the GGSN or SGSN
> 
> It would be really nice to modernize these names before exposing this API.

I am very skeptical about this.  The features were implemented with this use
case in mind, and as you know, they only match rather partially to the requirements
of later generation technology (4G/LTE/EPC) due to their lack of support for
dedicated bearers / TFTs.

> When I added the role property to the driver, it was largely to complement
> the behaviour of the OpenGGSN library, who was essentially the only user of
> this module at the time.  However, even at that time the choice of name was
> awkward because we were well into the 4G era so SGSN/GGSN was already
> somewhat legacy terminology; today, these terms are starting to raise some
> eyebrows amongst younger developers who may be well versed in 4G/5G, but for
> whom 3G is somewhat ancient history.

The fact that some later generation of technology _also_ is using parts of older
generations of technology does not mean that the older terminology is in any way
superseded.  A SGSN remains a SGSN even today, likewise a GGSN.  And those network
elements are used in production, very much so even in 2022.

> 3GPP has a well-accepted definition of "uplink" and "downlink" which is
> probably what we should be using instead.  So sgsn becomes "uplink" and ggsn
> becomes "downlink", with the distinction here being whether packets are
> routed by source or destination IP address.

Could you please provide a 3GPP spec reference for this?  I am working
every day in the 3GPP world for something like 15+ years now, and I
would be _seriously_ surprised if such terminology was adopted for the
use case you describe. I have not come across it so far in that way.

"uplink" and "downlink" to me
a) define a direction, and not a network element / function.
b) are very general terms which depend on the point of view.

This is why directions, in 3GPP, traditionally, are called
"mobile-originated" and "mobile-terminated".  This has an unambiguous
meaning as to which direction is used.

But in any case, here we want to name logical network elements or
functions within such elements, and not directions.

Those elements / functions have new names in each generation of mobile
technology, so you have SGSN or S-GW on the one hand side, and GGSN or
P-GW on the other side.  The P-GW has then optionally been decomposed
into the UPF and SMF.  And then you have a variety of other use cases
(interfaces) where the GTPv1U protocol was later introduced, such as the
use between eNB and S-GW.

So you cannot use S-GW and P-GW as names for the roles of the GTP tunnel
driver, as S-GW actually performs both "roles": You can think of it as
decapsulationg the traffic on the eNB-SGW interface and as encapsulating
the traffic on the SGW-PGW side.

I'm not fundamentally opposed to any renaming, but any such renaming
must have a unambiguous definition.

In the context of TS 29.060, there are a number of references to uplink
and downlink.  However, they - as far as I can tell -

* specify a direction (like the QoS Parameters like AMBR for uplink / downlink)

* are used within the context of TEIDs.  So there is an "uplink tunnel
  endpoint identifier" which is chosen by the GGSN, and which is used by
  the SGSN to send data.  So again it is used to signify a direction.

If we look at 3GPP TS 29.281, there are one mention of 'uplink' and
'downlink', and both are also again referring to a direction of traffic.

Regards,
	Harald

-- 
- Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2-next v3 1/2] ip: GTP support in ip link
  2022-02-27  7:55     ` Harald Welte
@ 2022-02-27  9:12       ` Jonas Bonn
  0 siblings, 0 replies; 7+ messages in thread
From: Jonas Bonn @ 2022-02-27  9:12 UTC (permalink / raw)
  To: Harald Welte; +Cc: Wojciech Drewek, netdev, dsahern, stephen



On 27/02/2022 08:55, Harald Welte wrote:
> Dear Jonas,
> 
> On Sun, Feb 27, 2022 at 07:57:02AM +0100, Jonas Bonn wrote:
>> On 11/02/2022 19:29, Wojciech Drewek wrote:
>>> Support for creating GTP devices through ip link. Two arguments
>>> can be specified by the user when adding device of the GTP type.
>>>    - role (sgsn or ggsn) - indicates whether we are on the GGSN or SGSN
>>
>> It would be really nice to modernize these names before exposing this API.
> 
> I am very skeptical about this.  The features were implemented with this use
> case in mind, and as you know, they only match rather partially to the requirements
> of later generation technology (4G/LTE/EPC) due to their lack of support for
> dedicated bearers / TFTs.
> 
>> When I added the role property to the driver, it was largely to complement
>> the behaviour of the OpenGGSN library, who was essentially the only user of
>> this module at the time.  However, even at that time the choice of name was
>> awkward because we were well into the 4G era so SGSN/GGSN was already
>> somewhat legacy terminology; today, these terms are starting to raise some
>> eyebrows amongst younger developers who may be well versed in 4G/5G, but for
>> whom 3G is somewhat ancient history.
> 
> The fact that some later generation of technology _also_ is using parts of older
> generations of technology does not mean that the older terminology is in any way
> superseded.  A SGSN remains a SGSN even today, likewise a GGSN.  And those network
> elements are used in production, very much so even in 2022.
> 
>> 3GPP has a well-accepted definition of "uplink" and "downlink" which is
>> probably what we should be using instead.  So sgsn becomes "uplink" and ggsn
>> becomes "downlink", with the distinction here being whether packets are
>> routed by source or destination IP address.
> 
> Could you please provide a 3GPP spec reference for this?  I am working
> every day in the 3GPP world for something like 15+ years now, and I
> would be _seriously_ surprised if such terminology was adopted for the
> use case you describe. I have not come across it so far in that way.
> 
> "uplink" and "downlink" to me
> a) define a direction, and not a network element / function.
> b) are very general terms which depend on the point of view.
> 
> This is why directions, in 3GPP, traditionally, are called
> "mobile-originated" and "mobile-terminated".  This has an unambiguous
> meaning as to which direction is used.
> 
> But in any case, here we want to name logical network elements or
> functions within such elements, and not directions.
> 
> Those elements / functions have new names in each generation of mobile
> technology, so you have SGSN or S-GW on the one hand side, and GGSN or
> P-GW on the other side.  The P-GW has then optionally been decomposed
> into the UPF and SMF.  And then you have a variety of other use cases
> (interfaces) where the GTPv1U protocol was later introduced, such as the
> use between eNB and S-GW.
> 
> So you cannot use S-GW and P-GW as names for the roles of the GTP tunnel
> driver, as S-GW actually performs both "roles": You can think of it as
> decapsulationg the traffic on the eNB-SGW interface and as encapsulating
> the traffic on the SGW-PGW side.

So this may all come down sloppy usage of the terminology, but everyone 
I interact with tend to refer to the interfaces on the S-GW, I-UPF, UPF, 
etc as the uplink or downlink interface depending on whether it is 
connected to the uplink network segment or the downlink network segment. 
  That's all a bit ambiguous, as you alluded to above, until you 
consider that these directions are always relative to the UE:  the 
uplink carries traffic FROM the UE, the downlink carries traffic TO the UE.

That all maps quite nicely to the GTP module where the peer tunnel 
endpoint is determined from either the source or destination address 
depending on which direction the traffic is flowing:  either TO or FROM 
the UE.  Traffic sent on an uplink interface is destined for the UE, 
hence the destination IP determines the TEID; traffic sent out on a 
downlink interface moves away FROM the UE, so the source address 
determines the TEID.

With that, the S-GW/I-UPF has a both an uplink and a downlink GTP 
interface between which packets are forwarded.  The decision as to which 
peer TEID to send to is made based on which interface the packet is 
going out on, which of course is synonymous with the direction the 
packets are flowing in.  That's exactly what the SSGN/GGSN "mode" 
parameter decides, as well.

Anyway, I don't disagree with anything that you wrote here and I'm not 
going to push hard for a name change; I just wanted to raise the issue. 
  I realize that uplink/downlink aren't crystal clear either.  That 
said, if I'm putting together an I-UPF, then it certainly isn't more 
clear why I need an interface in "SSGN" mode rather than "downlink" mode.

And I'm sure that I've seen a one-liner or something in some 3GPP spec 
that handwavingly justifies the above usage, but of course I can't find 
it now... so I'll happily concede that this isn't a 3GPP standard, but 
rather just "common usage", at least within in my bubble.


> 
> I'm not fundamentally opposed to any renaming, but any such renaming
> must have a unambiguous definition.
> 
> In the context of TS 29.060, there are a number of references to uplink
> and downlink.  However, they - as far as I can tell -
> 
> * specify a direction (like the QoS Parameters like AMBR for uplink / downlink)
> 
> * are used within the context of TEIDs.  So there is an "uplink tunnel
>    endpoint identifier" which is chosen by the GGSN, and which is used by
>    the SGSN to send data.  So again it is used to signify a direction.


The "uplink tunnel" terminates at the "uplink interface" of the GGSN, so 
you'd want the GTP module to provide an interface in "uplink" mode.

Equally, the SGSN allocates a "downlink tunnel endpoint identifier" for 
packets terminating at the "downlink interface" (originating from the 
GGSN), so here you'd want the GTP module giving you an interface in 
"downlink" mode.

/Jonas


> 
> If we look at 3GPP TS 29.281, there are one mention of 'uplink' and
> 'downlink', and both are also again referring to a direction of traffic.
> 
> Regards,
> 	Harald
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-02-27  9:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-11 18:29 [PATCH iproute2-next v3 0/2] GTP support for ip link and tc flower Wojciech Drewek
2022-02-11 18:29 ` [PATCH iproute2-next v3 1/2] ip: GTP support in ip link Wojciech Drewek
2022-02-12  8:27   ` Harald Welte
2022-02-27  6:57   ` Jonas Bonn
2022-02-27  7:55     ` Harald Welte
2022-02-27  9:12       ` Jonas Bonn
2022-02-11 18:29 ` [PATCH iproute2-next v3 2/2] f_flower: Implement gtp options support Wojciech Drewek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.