[PATCHv3 iproute2-next 0/7] iproute2: fully support for geneve/vxlan/erspan options

[PATCHv3 iproute2-next 0/7] iproute2: fully support for geneve/vxlan/erspan options

[Xin Long]

Patch 1-3 add the geneve/vxlan/erspan options support for
iproute_lwtunnel, and Patch 4-5 add the vxlan/erspan options
for tc m_tunnel_key, and Patch 6-7 add the vxlan/erspan options
for tc f_flower.

In kernel space, these features have been supported since these
patchsets:

  https://patchwork.ozlabs.org/cover/1190172/
  https://patchwork.ozlabs.org/cover/1198854/

v1->v2:
  - improve the bash commands in changelog as David A. suggested.
  - use PRINT_ANY to support dummping with json format as Stephen
    suggested.
v2->v3:
  - implement proper JSON array for opts as Stephen suggested.

Xin Long (7):
  iproute_lwtunnel: add options support for geneve metadata
  iproute_lwtunnel: add options support for vxlan metadata
  iproute_lwtunnel: add options support for erspan metadata
  tc: m_tunnel_key: add options support for vxlan
  tc: m_tunnel_key: add options support for erpsan
  tc: f_flower: add options support for vxlan
  tc: f_flower: add options support for erspan

 ip/iproute_lwtunnel.c    | 398 ++++++++++++++++++++++++++++++++++++++++++++++-
 man/man8/tc-flower.8     |  26 ++++
 man/man8/tc-tunnel_key.8 |  20 ++-
 tc/f_flower.c            | 301 +++++++++++++++++++++++++++++++++--
 tc/m_tunnel_key.c        | 205 ++++++++++++++++++++++--
 5 files changed, 922 insertions(+), 28 deletions(-)

-- 
2.1.0

[PATCHv3 iproute2-next 1/7] iproute_lwtunnel: add options support for geneve metadata

[Xin Long]

This patch is to add LWTUNNEL_IP(6)_OPTS and LWTUNNEL_IP_OPTS_GENEVE's
parse and print to implement geneve options support in iproute_lwtunnel.

Options are expressed as class:type:data and multiple options may be
listed using a comma delimiter.

With this patch, users can add and dump geneve options like:

  # ip netns add a
  # ip netns add b
  # ip -n a link add eth0 type veth peer name eth0 netns b
  # ip -n a link set eth0 up; ip -n b link set eth0 up
  # ip -n a addr add 10.1.0.1/24 dev eth0
  # ip -n b addr add 10.1.0.2/24 dev eth0
  # ip -n b link add geneve1 type geneve id 1 remote 10.1.0.1 ttl 64
  # ip -n b addr add 1.1.1.1/24 dev geneve1
  # ip -n b link set geneve1 up
  # ip -n b route add 2.1.1.0/24 dev geneve1
  # ip -n a link add geneve1 type geneve external
  # ip -n a addr add 2.1.1.1/24 dev geneve1
  # ip -n a link set geneve1 up
  # ip -n a route add 1.1.1.0/24 encap ip id 1 geneve_opts \
    1:1:1212121234567890,1:1:1212121234567890,1:1:1212121234567890 \
    dst 10.1.0.2 dev geneve1
  # ip -n a route show
  # ip netns exec a ping 1.1.1.1 -c 1

   1.1.1.0/24  encap ip id 1 src 0.0.0.0 dst 10.1.0.2 ttl 0 tos 0
     geneve_opts 0001:01:1212121234567890,0001:01:1212121234567890 ...

   PING 1.1.1.1 (1.1.1.1) 56(84) bytes of data.
   64 bytes from 1.1.1.1: icmp_seq=1 ttl=64 time=0.079 ms

v1->v2:
  - improve the changelog.
  - use PRINT_ANY to support dumping with json format.
v2->v3:
  - implement proper JSON array for opts instead of just bunch of strings.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 ip/iproute_lwtunnel.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 181 insertions(+), 2 deletions(-)

diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 0d7d714..737d7da 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -294,6 +294,63 @@ static void print_encap_mpls(FILE *fp, struct rtattr *encap)
 			rta_getattr_u8(tb[MPLS_IPTUNNEL_TTL]));
 }
 
+static void lwtunnel_print_geneve_opts(struct rtattr *attr)
+{
+	struct rtattr *tb[LWTUNNEL_IP_OPT_GENEVE_MAX + 1];
+	struct rtattr *i = RTA_DATA(attr);
+	int ii, data_len, offset = 0;
+	int rem = RTA_PAYLOAD(attr);
+	char *name = "geneve_opts";
+	char strbuf[rem * 2 + 1];
+	char data[rem * 2 + 1];
+	uint8_t data_r[rem];
+	__u16 class;
+	__u8 type;
+
+	open_json_array(PRINT_JSON, name);
+	print_nl();
+	print_string(PRINT_FP, name, "\t%s ", name);
+
+	while (rem) {
+		parse_rtattr(tb, LWTUNNEL_IP_OPT_GENEVE_MAX, i, rem);
+		class = rta_getattr_be16(tb[LWTUNNEL_IP_OPT_GENEVE_CLASS]);
+		type = rta_getattr_u8(tb[LWTUNNEL_IP_OPT_GENEVE_TYPE]);
+		data_len = RTA_PAYLOAD(tb[LWTUNNEL_IP_OPT_GENEVE_DATA]);
+		hexstring_n2a(RTA_DATA(tb[LWTUNNEL_IP_OPT_GENEVE_DATA]),
+			      data_len, data, sizeof(data));
+		hex2mem(data, data_r, data_len);
+		offset += data_len + 20;
+		rem -= data_len + 20;
+		i = RTA_DATA(attr) + offset;
+
+		open_json_object(NULL);
+		print_uint(PRINT_JSON, "class", NULL, class);
+		print_uint(PRINT_JSON, "type", NULL, type);
+		open_json_array(PRINT_JSON, "data");
+		for (ii = 0; ii < data_len; ii++)
+			print_uint(PRINT_JSON, NULL, NULL, data_r[ii]);
+		close_json_array(PRINT_JSON, "data");
+		close_json_object();
+
+		sprintf(strbuf, "%04x:%02x:%s", class, type, data);
+		if (rem)
+			print_string(PRINT_FP, NULL, "%s,", strbuf);
+		else
+			print_string(PRINT_FP, NULL, "%s ", strbuf);
+	}
+
+	close_json_array(PRINT_JSON, name);
+}
+
+static void lwtunnel_print_opts(struct rtattr *attr)
+{
+	struct rtattr *tb_opt[LWTUNNEL_IP_OPTS_MAX + 1];
+
+	parse_rtattr_nested(tb_opt, LWTUNNEL_IP_OPTS_MAX, attr);
+	if (tb_opt[LWTUNNEL_IP_OPTS_GENEVE])
+		lwtunnel_print_geneve_opts(tb_opt[LWTUNNEL_IP_OPTS_GENEVE]);
+}
+
 static void print_encap_ip(FILE *fp, struct rtattr *encap)
 {
 	struct rtattr *tb[LWTUNNEL_IP_MAX+1];
@@ -332,6 +389,9 @@ static void print_encap_ip(FILE *fp, struct rtattr *encap)
 		if (flags & TUNNEL_SEQ)
 			print_bool(PRINT_ANY, "seq", "seq ", true);
 	}
+
+	if (tb[LWTUNNEL_IP_OPTS])
+		lwtunnel_print_opts(tb[LWTUNNEL_IP_OPTS]);
 }
 
 static void print_encap_ila(FILE *fp, struct rtattr *encap)
@@ -404,6 +464,9 @@ static void print_encap_ip6(FILE *fp, struct rtattr *encap)
 		if (flags & TUNNEL_SEQ)
 			print_bool(PRINT_ANY, "seq", "seq ", true);
 	}
+
+	if (tb[LWTUNNEL_IP6_OPTS])
+		lwtunnel_print_opts(tb[LWTUNNEL_IP6_OPTS]);
 }
 
 static void print_encap_bpf(FILE *fp, struct rtattr *encap)
@@ -798,11 +861,97 @@ static int parse_encap_mpls(struct rtattr *rta, size_t len,
 	return 0;
 }
 
+static int lwtunnel_parse_geneve_opt(char *str, size_t len, struct rtattr *rta)
+{
+	struct rtattr *nest;
+	char *token;
+	int i, err;
+
+	nest = rta_nest(rta, len, LWTUNNEL_IP_OPTS_GENEVE | NLA_F_NESTED);
+	i = 1;
+	token = strsep(&str, ":");
+	while (token) {
+		switch (i) {
+		case LWTUNNEL_IP_OPT_GENEVE_CLASS:
+		{
+			__be16 opt_class;
+
+			if (!strlen(token))
+				break;
+			err = get_be16(&opt_class, token, 16);
+			if (err)
+				return err;
+
+			rta_addattr16(rta, len, i, opt_class);
+			break;
+		}
+		case LWTUNNEL_IP_OPT_GENEVE_TYPE:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			rta_addattr8(rta, len, i, opt_type);
+			break;
+		}
+		case LWTUNNEL_IP_OPT_GENEVE_DATA:
+		{
+			size_t token_len = strlen(token);
+			__u8 *opts;
+
+			if (!token_len)
+				break;
+			opts = malloc(token_len / 2);
+			if (!opts)
+				return -1;
+			if (hex2mem(token, opts, token_len / 2) < 0) {
+				free(opts);
+				return -1;
+			}
+			rta_addattr_l(rta, len, i, opts, token_len / 2);
+			free(opts);
+
+			break;
+		}
+		default:
+			fprintf(stderr, "Unknown \"geneve_opts\" type\n");
+			return -1;
+		}
+
+		token = strsep(&str, ":");
+		i++;
+	}
+	rta_nest_end(rta, nest);
+
+	return 0;
+}
+
+static int lwtunnel_parse_geneve_opts(char *str, size_t len, struct rtattr *rta)
+{
+	char *token;
+	int err;
+
+	token = strsep(&str, ",");
+	while (token) {
+		err = lwtunnel_parse_geneve_opt(token, len, rta);
+		if (err)
+			return err;
+
+		token = strsep(&str, ",");
+	}
+
+	return 0;
+}
+
 static int parse_encap_ip(struct rtattr *rta, size_t len,
 			  int *argcp, char ***argvp)
 {
 	int id_ok = 0, dst_ok = 0, src_ok = 0, tos_ok = 0, ttl_ok = 0;
-	int key_ok = 0, csum_ok = 0, seq_ok = 0;
+	int key_ok = 0, csum_ok = 0, seq_ok = 0, opts_ok = 0;
 	char **argv = *argvp;
 	int argc = *argcp;
 	int ret = 0;
@@ -854,6 +1003,21 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
 			if (get_u8(&ttl, *argv, 0))
 				invarg("\"ttl\" value is invalid\n", *argv);
 			ret = rta_addattr8(rta, len, LWTUNNEL_IP_TTL, ttl);
+		} else if (strcmp(*argv, "geneve_opts") == 0) {
+			struct rtattr *nest;
+
+			if (opts_ok++)
+				duparg2("opts", *argv);
+
+			NEXT_ARG();
+
+			nest = rta_nest(rta, len,
+					LWTUNNEL_IP_OPTS | NLA_F_NESTED);
+			ret = lwtunnel_parse_geneve_opts(*argv, len, rta);
+			if (ret)
+				invarg("\"geneve_opts\" value is invalid\n",
+				       *argv);
+			rta_nest_end(rta, nest);
 		} else if (strcmp(*argv, "key") == 0) {
 			if (key_ok++)
 				duparg2("key", *argv);
@@ -969,7 +1133,7 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
 			   int *argcp, char ***argvp)
 {
 	int id_ok = 0, dst_ok = 0, src_ok = 0, tos_ok = 0, ttl_ok = 0;
-	int key_ok = 0, csum_ok = 0, seq_ok = 0;
+	int key_ok = 0, csum_ok = 0, seq_ok = 0, opts_ok = 0;
 	char **argv = *argvp;
 	int argc = *argcp;
 	int ret = 0;
@@ -1023,6 +1187,21 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
 				       *argv);
 			ret = rta_addattr8(rta, len, LWTUNNEL_IP6_HOPLIMIT,
 					   hoplimit);
+		} else if (strcmp(*argv, "geneve_opts") == 0) {
+			struct rtattr *nest;
+
+			if (opts_ok++)
+				duparg2("opts", *argv);
+
+			NEXT_ARG();
+
+			nest = rta_nest(rta, len,
+					LWTUNNEL_IP_OPTS | NLA_F_NESTED);
+			ret = lwtunnel_parse_geneve_opts(*argv, len, rta);
+			if (ret)
+				invarg("\"geneve_opts\" value is invalid\n",
+				       *argv);
+			rta_nest_end(rta, nest);
 		} else if (strcmp(*argv, "key") == 0) {
 			if (key_ok++)
 				duparg2("key", *argv);
-- 
2.1.0

[PATCHv3 iproute2-next 2/7] iproute_lwtunnel: add options support for vxlan metadata

[Xin Long]

This patch is to add LWTUNNEL_IP_OPTS_VXLAN's parse and print to implement
vxlan options support in iproute_lwtunnel.

Option is expressed one hex value for gbp only, and vxlan doesn't support
multiple options.

With this patch, users can add and dump vxlan options like:

  # ip netns add a
  # ip netns add b
  # ip -n a link add eth0 type veth peer name eth0 netns b
  # ip -n a link set eth0 up
  # ip -n b link set eth0 up
  # ip -n a addr add 10.1.0.1/24 dev eth0
  # ip -n b addr add 10.1.0.2/24 dev eth0
  # ip -n b link add vxlan1 type vxlan id 1 local 10.1.0.2 remote 10.1.0.1 \
    dev eth0 ttl 64 gbp
  # ip -n b addr add 1.1.1.1/24 dev vxlan1
  # ip -n b link set vxlan1 up
  # ip -n b route add 2.1.1.0/24 dev vxlan1
  # ip -n a link add vxlan1 type vxlan local 10.1.0.1 dev eth0 ttl 64 \
    gbp external
  # ip -n a addr add 2.1.1.1/24 dev vxlan1
  # ip -n a link set vxlan1 up
  # ip -n a route add 1.1.1.0/24 encap ip id 1 \
    vxlan_opts 0x456 dst 10.1.0.2 dev vxlan1
  # ip -n a route show
  # ip netns exec a ping 1.1.1.1 -c 1

   1.1.1.0/24  encap ip id 1 src 0.0.0.0 dst 10.1.0.2 ttl 0 tos 0
     vxlan_opts 0x456 dev vxlan1 scope link

   PING 1.1.1.1 (1.1.1.1) 56(84) bytes of data.
   64 bytes from 1.1.1.1: icmp_seq=1 ttl=64 time=0.111 ms

v1->v2:
  - improve the changelog.
  - get_u32 with base = 0 for gbp.
  - use PRINT_ANY to support dumping with json format.
v2->v3:
  - implement proper JSON array for opts.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 ip/iproute_lwtunnel.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 737d7da..7691002 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -342,6 +342,28 @@ static void lwtunnel_print_geneve_opts(struct rtattr *attr)
 	close_json_array(PRINT_JSON, name);
 }
 
+static void lwtunnel_print_vxlan_opts(struct rtattr *attr)
+{
+        struct rtattr *tb[LWTUNNEL_IP_OPT_VXLAN_MAX + 1];
+        struct rtattr *i = RTA_DATA(attr);
+        int rem = RTA_PAYLOAD(attr);
+        char *name = "vxlan_opts";
+        __u32 gbp;
+
+	parse_rtattr(tb, LWTUNNEL_IP_OPT_VXLAN_MAX, i, rem);
+	gbp = rta_getattr_u32(tb[LWTUNNEL_IP_OPT_VXLAN_GBP]);
+
+	open_json_array(PRINT_JSON, name);
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "gbp", NULL, gbp);
+	close_json_object();
+	close_json_array(PRINT_JSON, name);
+
+	print_nl();
+	print_string(PRINT_FP, name, "\t%s ", name);
+	print_uint(PRINT_FP, NULL, "0x%x ", gbp);
+}
+
 static void lwtunnel_print_opts(struct rtattr *attr)
 {
 	struct rtattr *tb_opt[LWTUNNEL_IP_OPTS_MAX + 1];
@@ -349,6 +371,8 @@ static void lwtunnel_print_opts(struct rtattr *attr)
 	parse_rtattr_nested(tb_opt, LWTUNNEL_IP_OPTS_MAX, attr);
 	if (tb_opt[LWTUNNEL_IP_OPTS_GENEVE])
 		lwtunnel_print_geneve_opts(tb_opt[LWTUNNEL_IP_OPTS_GENEVE]);
+	else if (tb_opt[LWTUNNEL_IP_OPTS_VXLAN])
+		lwtunnel_print_vxlan_opts(tb_opt[LWTUNNEL_IP_OPTS_VXLAN]);
 }
 
 static void print_encap_ip(FILE *fp, struct rtattr *encap)
@@ -947,6 +971,22 @@ static int lwtunnel_parse_geneve_opts(char *str, size_t len, struct rtattr *rta)
 	return 0;
 }
 
+static int lwtunnel_parse_vxlan_opts(char *str, size_t len, struct rtattr *rta)
+{
+	struct rtattr *nest;
+	__u32 gbp;
+	int err;
+
+	nest = rta_nest(rta, len, LWTUNNEL_IP_OPTS_VXLAN | NLA_F_NESTED);
+	err = get_u32(&gbp, str, 0);
+	if (err)
+		return err;
+	rta_addattr32(rta, len, LWTUNNEL_IP_OPT_VXLAN_GBP, gbp);
+
+	rta_nest_end(rta, nest);
+	return 0;
+}
+
 static int parse_encap_ip(struct rtattr *rta, size_t len,
 			  int *argcp, char ***argvp)
 {
@@ -1018,6 +1058,21 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
 				invarg("\"geneve_opts\" value is invalid\n",
 				       *argv);
 			rta_nest_end(rta, nest);
+		} else if (strcmp(*argv, "vxlan_opts") == 0) {
+			struct rtattr *nest;
+
+			if (opts_ok++)
+				duparg2("opts", *argv);
+
+			NEXT_ARG();
+
+			nest = rta_nest(rta, len,
+					LWTUNNEL_IP_OPTS | NLA_F_NESTED);
+			ret = lwtunnel_parse_vxlan_opts(*argv, len, rta);
+			if (ret)
+				invarg("\"vxlan_opts\" value is invalid\n",
+				       *argv);
+			rta_nest_end(rta, nest);
 		} else if (strcmp(*argv, "key") == 0) {
 			if (key_ok++)
 				duparg2("key", *argv);
@@ -1202,6 +1257,21 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
 				invarg("\"geneve_opts\" value is invalid\n",
 				       *argv);
 			rta_nest_end(rta, nest);
+		} else if (strcmp(*argv, "vxlan_opts") == 0) {
+			struct rtattr *nest;
+
+			if (opts_ok++)
+				duparg2("opts", *argv);
+
+			NEXT_ARG();
+
+			nest = rta_nest(rta, len,
+					LWTUNNEL_IP_OPTS | NLA_F_NESTED);
+			ret = lwtunnel_parse_vxlan_opts(*argv, len, rta);
+			if (ret)
+				invarg("\"vxlan_opts\" value is invalid\n",
+				       *argv);
+			rta_nest_end(rta, nest);
 		} else if (strcmp(*argv, "key") == 0) {
 			if (key_ok++)
 				duparg2("key", *argv);
-- 
2.1.0

[PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

This patch is to add LWTUNNEL_IP_OPTS_ERSPAN's parse and print to implement
erspan options support in iproute_lwtunnel.

Option is expressed as version:index:dir:hwid, dir and hwid will be parsed
when version is 2, while index will be parsed when version is 1. erspan
doesn't support multiple options.

With this patch, users can add and dump erspan options like:

  # ip netns add a
  # ip netns add b
  # ip -n a link add eth0 type veth peer name eth0 netns b
  # ip -n a link set eth0 up
  # ip -n b link set eth0 up
  # ip -n a addr add 10.1.0.1/24 dev eth0
  # ip -n b addr add 10.1.0.2/24 dev eth0
  # ip -n b link add erspan1 type erspan key 1 seq erspan 123 \
    local 10.1.0.2 remote 10.1.0.1
  # ip -n b addr add 1.1.1.1/24 dev erspan1
  # ip -n b link set erspan1 up
  # ip -n b route add 2.1.1.0/24 dev erspan1
  # ip -n a link add erspan1 type erspan key 1 seq local 10.1.0.1 external
  # ip -n a addr add 2.1.1.1/24 dev erspan1
  # ip -n a link set erspan1 up
  # ip -n a route add 1.1.1.0/24 encap ip id 1 \
    erspan_opts 2:123:1:2 dst 10.1.0.2 dev erspan1
  # ip -n a route show
  # ip netns exec a ping 1.1.1.1 -c 1

   1.1.1.0/24  encap ip id 1 src 0.0.0.0 dst 10.1.0.2 ttl 0 tos 0
     erspan_opts 02:00000000:01:02 dev erspan1 scope link

   PING 1.1.1.1 (1.1.1.1) 56(84) bytes of data.
   64 bytes from 1.1.1.1: icmp_seq=1 ttl=64 time=0.124 ms

v1->v2:
  - improve the changelog.
  - use PRINT_ANY to support dumping with json format.
v2->v3:
  - implement proper JSON object for opts instead of just bunch of strings.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 ip/iproute_lwtunnel.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 145 insertions(+)

diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 7691002..f64ccb1 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -364,6 +364,43 @@ static void lwtunnel_print_vxlan_opts(struct rtattr *attr)
 	print_uint(PRINT_FP, NULL, "0x%x ", gbp);
 }
 
+static void lwtunnel_print_erspan_opts(struct rtattr *attr)
+{
+	struct rtattr *tb[LWTUNNEL_IP_OPT_ERSPAN_MAX + 1];
+        struct rtattr *i = RTA_DATA(attr);
+        int rem = RTA_PAYLOAD(attr);
+        char *name = "erspan_opts";
+        char strbuf[rem * 2 + 1];
+	__u8 ver, hwid, dir;
+	__u32 idx;
+
+	parse_rtattr(tb, LWTUNNEL_IP_OPT_ERSPAN_MAX, i, RTA_PAYLOAD(attr));
+	ver = rta_getattr_u8(tb[LWTUNNEL_IP_OPT_ERSPAN_VER]);
+	if (ver == 1) {
+		idx = rta_getattr_be32(tb[LWTUNNEL_IP_OPT_ERSPAN_INDEX]);
+		dir = 0;
+		hwid = 0;
+	} else {
+		idx = 0;
+		dir = rta_getattr_u8(tb[LWTUNNEL_IP_OPT_ERSPAN_DIR]);
+		hwid = rta_getattr_u8(tb[LWTUNNEL_IP_OPT_ERSPAN_HWID]);
+	}
+
+	open_json_array(PRINT_JSON, name);
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "ver", NULL, ver);
+	print_uint(PRINT_JSON, "index", NULL, idx);
+	print_uint(PRINT_JSON, "dir", NULL, dir);
+	print_uint(PRINT_JSON, "hwid", NULL, hwid);
+	close_json_object();
+	close_json_array(PRINT_JSON, name);
+
+	print_nl();
+	print_string(PRINT_FP, name, "\t%s ", name);
+	sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
+	print_string(PRINT_FP, NULL, "%s ", strbuf);
+}
+
 static void lwtunnel_print_opts(struct rtattr *attr)
 {
 	struct rtattr *tb_opt[LWTUNNEL_IP_OPTS_MAX + 1];
@@ -373,6 +410,8 @@ static void lwtunnel_print_opts(struct rtattr *attr)
 		lwtunnel_print_geneve_opts(tb_opt[LWTUNNEL_IP_OPTS_GENEVE]);
 	else if (tb_opt[LWTUNNEL_IP_OPTS_VXLAN])
 		lwtunnel_print_vxlan_opts(tb_opt[LWTUNNEL_IP_OPTS_VXLAN]);
+	else if (tb_opt[LWTUNNEL_IP_OPTS_ERSPAN])
+		lwtunnel_print_erspan_opts(tb_opt[LWTUNNEL_IP_OPTS_ERSPAN]);
 }
 
 static void print_encap_ip(FILE *fp, struct rtattr *encap)
@@ -987,6 +1026,82 @@ static int lwtunnel_parse_vxlan_opts(char *str, size_t len, struct rtattr *rta)
 	return 0;
 }
 
+static int lwtunnel_parse_erspan_opts(char *str, size_t len, struct rtattr *rta)
+{
+	struct rtattr *nest;
+	char *token;
+	int i, err;
+
+	nest = rta_nest(rta, len, LWTUNNEL_IP_OPTS_ERSPAN | NLA_F_NESTED);
+	i = 1;
+	token = strsep(&str, ":");
+	while (token) {
+		switch (i) {
+		case LWTUNNEL_IP_OPT_ERSPAN_VER:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			rta_addattr8(rta, len, i, opt_type);
+			break;
+		}
+		case LWTUNNEL_IP_OPT_ERSPAN_INDEX:
+		{
+			__be32 opt_class;
+
+			if (!strlen(token))
+				break;
+			err = get_be32(&opt_class, token, 16);
+			if (err)
+				return err;
+
+			rta_addattr32(rta, len, i, opt_class);
+			break;
+		}
+		case LWTUNNEL_IP_OPT_ERSPAN_DIR:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			rta_addattr8(rta, len, i, opt_type);
+			break;
+		}
+		case LWTUNNEL_IP_OPT_ERSPAN_HWID:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			rta_addattr8(rta, len, i, opt_type);
+			break;
+		}
+		default:
+			fprintf(stderr, "Unknown \"geneve_opts\" type\n");
+			return -1;
+		}
+
+		token = strsep(&str, ":");
+		i++;
+	}
+
+	rta_nest_end(rta, nest);
+	return 0;
+}
+
 static int parse_encap_ip(struct rtattr *rta, size_t len,
 			  int *argcp, char ***argvp)
 {
@@ -1073,6 +1188,21 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
 				invarg("\"vxlan_opts\" value is invalid\n",
 				       *argv);
 			rta_nest_end(rta, nest);
+		} else if (strcmp(*argv, "erspan_opts") == 0) {
+			struct rtattr *nest;
+
+			if (opts_ok++)
+				duparg2("opts", *argv);
+
+			NEXT_ARG();
+
+			nest = rta_nest(rta, len,
+					LWTUNNEL_IP_OPTS | NLA_F_NESTED);
+			ret = lwtunnel_parse_erspan_opts(*argv, len, rta);
+			if (ret)
+				invarg("\"erspan_opts\" value is invalid\n",
+				       *argv);
+			rta_nest_end(rta, nest);
 		} else if (strcmp(*argv, "key") == 0) {
 			if (key_ok++)
 				duparg2("key", *argv);
@@ -1272,6 +1402,21 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
 				invarg("\"vxlan_opts\" value is invalid\n",
 				       *argv);
 			rta_nest_end(rta, nest);
+		} else if (strcmp(*argv, "erspan_opts") == 0) {
+			struct rtattr *nest;
+
+			if (opts_ok++)
+				duparg2("opts", *argv);
+
+			NEXT_ARG();
+
+			nest = rta_nest(rta, len,
+					LWTUNNEL_IP_OPTS | NLA_F_NESTED);
+			ret = lwtunnel_parse_erspan_opts(*argv, len, rta);
+			if (ret)
+				invarg("\"erspan_opts\" value is invalid\n",
+				       *argv);
+			rta_nest_end(rta, nest);
 		} else if (strcmp(*argv, "key") == 0) {
 			if (key_ok++)
 				duparg2("key", *argv);
-- 
2.1.0

[PATCHv3 iproute2-next 4/7] tc: m_tunnel_key: add options support for vxlan

[Xin Long]

This patch is to add TCA_TUNNEL_KEY_ENC_OPTS_VXLAN's parse and
print to implement vxlan options support in m_tunnel_key, like
Commit 6217917a3826 ("tc: m_tunnel_key: Add tunnel option support
to act_tunnel_key") for geneve options support.

Option is expressed a 32bit hexadecimal value for gbp only, and
vxlan doesn't support multiple options.

With this patch, users can add and dump vxlan options like:

  # ip link add name vxlan1 type vxlan dstport 0 external
  # tc qdisc add dev eth0 ingress
  # tc filter add dev eth0 protocol ip parent ffff: \
      flower indev eth0 \
        ip_proto udp \
        action tunnel_key \
          set src_ip 10.0.99.192 \
          dst_ip 10.0.99.193 \
          dst_port 6081 \
          id 11 \
          vxlan_opts 0x10101 \
      action mirred egress redirect dev vxlan1
  # tc -s filter show dev eth0 parent ffff:

     filter protocol ip pref 49152 flower chain 0 handle 0x1
       indev eth0
       eth_type ipv4
       ip_proto udp
       not_in_hw
         action order 1: tunnel_key  set
         src_ip 10.0.99.192
         dst_ip 10.0.99.193
         key_id 11
         dst_port 6081
         vxlan_opts 0x10101
         ...

v1->v2:
  - get_u32 with base = 0 for gbp.
  - use to print_unint("0x%x") to print gbp.
v2->v3:
  - implement proper JSON array for opts.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 man/man8/tc-tunnel_key.8 | 10 +++++-
 tc/m_tunnel_key.c        | 86 +++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/man/man8/tc-tunnel_key.8 b/man/man8/tc-tunnel_key.8
index 2145eb6..f78227c 100644
--- a/man/man8/tc-tunnel_key.8
+++ b/man/man8/tc-tunnel_key.8
@@ -66,8 +66,10 @@ options.
 .B id
 ,
 .B dst_port
-and
+,
 .B geneve_opts
+and
+.B vxlan_opts
 are optional.
 .RS
 .TP
@@ -91,6 +93,12 @@ is specified in the form CLASS:TYPE:DATA, where CLASS is represented as a
 variable length hexadecimal value. Additionally multiple options may be
 listed using a comma delimiter.
 .TP
+.B vxlan_opts
+Vxlan metatdata options.
+.B vxlan_opts
+is specified in the form GBP, as a 32bit hexadecimal value. Multiple options
+is not supported.
+.TP
 .B tos
 Outer header TOS
 .TP
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index 8fde689..e9c2a5a 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -29,7 +29,7 @@ static void explain(void)
 		"src_ip <IP> (mandatory)\n"
 		"dst_ip <IP> (mandatory)\n"
 		"dst_port <UDP_PORT>\n"
-		"geneve_opts <OPTIONS>\n"
+		"geneve_opts | vxlan_opts <OPTIONS>\n"
 		"csum | nocsum (default is \"csum\")\n");
 }
 
@@ -112,6 +112,21 @@ static int tunnel_key_parse_u8(char *str, int base, int type,
 	return 0;
 }
 
+static int tunnel_key_parse_u32(char *str, int base, int type,
+				struct nlmsghdr *n)
+{
+	__u32 value;
+	int ret;
+
+	ret = get_u32(&value, str, base);
+	if (ret)
+		return ret;
+
+	addattr32(n, MAX_MSG, type, value);
+
+	return 0;
+}
+
 static int tunnel_key_parse_geneve_opt(char *str, struct nlmsghdr *n)
 {
 	char *token, *saveptr = NULL;
@@ -190,6 +205,27 @@ static int tunnel_key_parse_geneve_opts(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int tunnel_key_parse_vxlan_opt(char *str, struct nlmsghdr *n)
+{
+	struct rtattr *encap, *nest;
+	int ret;
+
+	encap = addattr_nest(n, MAX_MSG,
+			     TCA_TUNNEL_KEY_ENC_OPTS | NLA_F_NESTED);
+	nest = addattr_nest(n, MAX_MSG,
+			    TCA_TUNNEL_KEY_ENC_OPTS_VXLAN | NLA_F_NESTED);
+
+	ret = tunnel_key_parse_u32(str, 0,
+				   TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP, n);
+	if (ret)
+		return ret;
+
+	addattr_nest_end(n, nest);
+	addattr_nest_end(n, encap);
+
+	return 0;
+}
+
 static int tunnel_key_parse_tos_ttl(char *str, int type, struct nlmsghdr *n)
 {
 	int ret;
@@ -287,6 +323,13 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
 				fprintf(stderr, "Illegal \"geneve_opts\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "vxlan_opts") == 0) {
+			NEXT_ARG();
+
+			if (tunnel_key_parse_vxlan_opt(*argv, n)) {
+				fprintf(stderr, "Illegal \"vxlan_opts\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "tos") == 0) {
 			NEXT_ARG();
 			ret = tunnel_key_parse_tos_ttl(*argv,
@@ -406,13 +449,13 @@ static void tunnel_key_print_flag(FILE *f, const char *name_on,
 		     rta_getattr_u8(attr) ? name_on : name_off);
 }
 
-static void tunnel_key_print_geneve_options(const char *name,
-					    struct rtattr *attr)
+static void tunnel_key_print_geneve_options(struct rtattr *attr)
 {
 	struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX + 1];
 	struct rtattr *i = RTA_DATA(attr);
 	int ii, data_len = 0, offset = 0;
 	int rem = RTA_PAYLOAD(attr);
+	char *name = "geneve_opts";
 	char strbuf[rem * 2 + 1];
 	char data[rem * 2 + 1];
 	uint8_t data_r[rem];
@@ -421,7 +464,7 @@ static void tunnel_key_print_geneve_options(const char *name,
 
 	open_json_array(PRINT_JSON, name);
 	print_nl();
-	print_string(PRINT_FP, name, "\t%s ", "geneve_opt");
+	print_string(PRINT_FP, name, "\t%s ", name);
 
 	while (rem) {
 		parse_rtattr(tb, TCA_TUNNEL_KEY_ENC_OPT_GENEVE_MAX, i, rem);
@@ -454,7 +497,29 @@ static void tunnel_key_print_geneve_options(const char *name,
 	close_json_array(PRINT_JSON, name);
 }
 
-static void tunnel_key_print_key_opt(const char *name, struct rtattr *attr)
+static void tunnel_key_print_vxlan_options(struct rtattr *attr)
+{
+	struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX + 1];
+	struct rtattr *i = RTA_DATA(attr);
+	int rem = RTA_PAYLOAD(attr);
+	char *name = "vxlan_opts";
+	__u32 gbp;
+
+	parse_rtattr(tb, TCA_TUNNEL_KEY_ENC_OPT_VXLAN_MAX, i, rem);
+	gbp = rta_getattr_u32(tb[TCA_TUNNEL_KEY_ENC_OPT_VXLAN_GBP]);
+
+	open_json_array(PRINT_JSON, name);
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "gbp", NULL, gbp);
+	close_json_object();
+	close_json_array(PRINT_JSON, name);
+
+	print_nl();
+	print_string(PRINT_FP, name, "\t%s ", name);
+	print_uint(PRINT_FP, NULL, "0x%x", gbp);
+}
+
+static void tunnel_key_print_key_opt(struct rtattr *attr)
 {
 	struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPTS_MAX + 1];
 
@@ -462,8 +527,12 @@ static void tunnel_key_print_key_opt(const char *name, struct rtattr *attr)
 		return;
 
 	parse_rtattr_nested(tb, TCA_TUNNEL_KEY_ENC_OPTS_MAX, attr);
-	tunnel_key_print_geneve_options(name,
-					tb[TCA_TUNNEL_KEY_ENC_OPTS_GENEVE]);
+	if (tb[TCA_TUNNEL_KEY_ENC_OPTS_GENEVE])
+		tunnel_key_print_geneve_options(
+			tb[TCA_TUNNEL_KEY_ENC_OPTS_GENEVE]);
+	else if (tb[TCA_TUNNEL_KEY_ENC_OPTS_VXLAN])
+		tunnel_key_print_vxlan_options(
+			tb[TCA_TUNNEL_KEY_ENC_OPTS_VXLAN]);
 }
 
 static void tunnel_key_print_tos_ttl(FILE *f, char *name,
@@ -519,8 +588,7 @@ static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
 					tb[TCA_TUNNEL_KEY_ENC_KEY_ID]);
 		tunnel_key_print_dst_port(f, "dst_port",
 					  tb[TCA_TUNNEL_KEY_ENC_DST_PORT]);
-		tunnel_key_print_key_opt("geneve_opts",
-					 tb[TCA_TUNNEL_KEY_ENC_OPTS]);
+		tunnel_key_print_key_opt(tb[TCA_TUNNEL_KEY_ENC_OPTS]);
 		tunnel_key_print_flag(f, "nocsum", "csum",
 				      tb[TCA_TUNNEL_KEY_NO_CSUM]);
 		tunnel_key_print_tos_ttl(f, "tos",
-- 
2.1.0

[PATCHv3 iproute2-next 5/7] tc: m_tunnel_key: add options support for erpsan

[Xin Long]

This patch is to add TCA_TUNNEL_KEY_ENC_OPTS_ERSPAN's parse and
print to implement erspan options support in m_tunnel_key, like
Commit 6217917a3826 ("tc: m_tunnel_key: Add tunnel option support
to act_tunnel_key") for geneve options support.

Option is expressed as version:index:dir:hwid, dir and hwid will
be parsed when version is 2, while index will be parsed when
version is 1. erspan doesn't support multiple options.

With this patch, users can add and dump erspan options like:

  # ip link add name erspan1 type erspan external
  # tc qdisc add dev eth0 ingress
  # tc filter add dev eth0 protocol ip parent ffff: \
      flower indev eth0 \
        ip_proto udp \
        action tunnel_key \
          set src_ip 10.0.99.192 \
          dst_ip 10.0.99.193 \
          dst_port 6081 \
          id 11 \
          erspan_opts 1:2:0:0 \
      action mirred egress redirect dev erspan1
  # tc -s filter show dev eth0 parent ffff:

     filter protocol ip pref 49151 flower chain 0 handle 0x1
       indev eth0
       eth_type ipv4
       ip_proto udp
       not_in_hw
         action order 1: tunnel_key  set
         src_ip 10.0.99.192
         dst_ip 10.0.99.193
         key_id 11
         dst_port 6081
         erspan_opts 01:00000002:00:00
         csum pipe
           index 2 ref 1 bind 1
         ...
v1->v2:
  - no change.
v2->v3:
  - no change.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 man/man8/tc-tunnel_key.8 |  12 ++++-
 tc/m_tunnel_key.c        | 121 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 131 insertions(+), 2 deletions(-)

diff --git a/man/man8/tc-tunnel_key.8 b/man/man8/tc-tunnel_key.8
index f78227c..bf2ebaa 100644
--- a/man/man8/tc-tunnel_key.8
+++ b/man/man8/tc-tunnel_key.8
@@ -68,8 +68,10 @@ options.
 .B dst_port
 ,
 .B geneve_opts
-and
+,
 .B vxlan_opts
+and
+.B erspan_opts
 are optional.
 .RS
 .TP
@@ -99,6 +101,14 @@ Vxlan metatdata options.
 is specified in the form GBP, as a 32bit hexadecimal value. Multiple options
 is not supported.
 .TP
+.B erspan_opts
+Erspan metatdata options.
+.B erspan_opts
+is specified in the form VERSION:INDEX:DIR:HWID, where VERSION is represented
+as a 8bit hexadecimal value, INDEX as an 32bit hexadecimal value, DIR and HWID
+as a 8bit hexadecimal value. Multiple options is not supported. Note INDEX is
+used when VERSION is 1, and DIR and HWID are used when VERSION is 2.
+.TP
 .B tos
 Outer header TOS
 .TP
diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c
index e9c2a5a..164ae5c 100644
--- a/tc/m_tunnel_key.c
+++ b/tc/m_tunnel_key.c
@@ -29,7 +29,7 @@ static void explain(void)
 		"src_ip <IP> (mandatory)\n"
 		"dst_ip <IP> (mandatory)\n"
 		"dst_port <UDP_PORT>\n"
-		"geneve_opts | vxlan_opts <OPTIONS>\n"
+		"geneve_opts | vxlan_opts | erspan_opts <OPTIONS>\n"
 		"csum | nocsum (default is \"csum\")\n");
 }
 
@@ -97,6 +97,21 @@ static int tunnel_key_parse_be16(char *str, int base, int type,
 	return 0;
 }
 
+static int tunnel_key_parse_be32(char *str, int base, int type,
+				 struct nlmsghdr *n)
+{
+	__be32 value;
+	int ret;
+
+	ret = get_be32(&value, str, base);
+	if (ret)
+		return ret;
+
+	addattr32(n, MAX_MSG, type, value);
+
+	return 0;
+}
+
 static int tunnel_key_parse_u8(char *str, int base, int type,
 			       struct nlmsghdr *n)
 {
@@ -226,6 +241,63 @@ static int tunnel_key_parse_vxlan_opt(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int tunnel_key_parse_erspan_opt(char *str, struct nlmsghdr *n)
+{
+	char *token, *saveptr = NULL;
+	struct rtattr *encap, *nest;
+	int i, ret;
+
+	encap = addattr_nest(n, MAX_MSG,
+			     TCA_TUNNEL_KEY_ENC_OPTS | NLA_F_NESTED);
+	nest = addattr_nest(n, MAX_MSG,
+			    TCA_TUNNEL_KEY_ENC_OPTS_ERSPAN | NLA_F_NESTED);
+
+	token = strtok_r(str, ":", &saveptr);
+	i = 1;
+	while (token) {
+		switch (i) {
+		case TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_VER:
+		{
+			ret = tunnel_key_parse_u8(token, 16, i, n);
+			if (ret)
+				return ret;
+			break;
+		}
+		case TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_INDEX:
+		{
+			ret = tunnel_key_parse_be32(token, 16, i, n);
+			if (ret)
+				return ret;
+			break;
+		}
+		case TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_DIR:
+		{
+			ret = tunnel_key_parse_u8(token, 16, i, n);
+			if (ret)
+				return ret;
+			break;
+		}
+		case TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_HWID:
+		{
+			ret = tunnel_key_parse_u8(token, 16, i, n);
+			if (ret)
+				return ret;
+			break;
+		}
+		default:
+			return -1;
+		}
+
+		token = strtok_r(NULL, ":", &saveptr);
+		i++;
+	}
+
+	addattr_nest_end(n, nest);
+	addattr_nest_end(n, encap);
+
+	return 0;
+}
+
 static int tunnel_key_parse_tos_ttl(char *str, int type, struct nlmsghdr *n)
 {
 	int ret;
@@ -330,6 +402,13 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p,
 				fprintf(stderr, "Illegal \"vxlan_opts\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "erspan_opts") == 0) {
+			NEXT_ARG();
+
+			if (tunnel_key_parse_erspan_opt(*argv, n)) {
+				fprintf(stderr, "Illegal \"erspan_opts\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "tos") == 0) {
 			NEXT_ARG();
 			ret = tunnel_key_parse_tos_ttl(*argv,
@@ -519,6 +598,43 @@ static void tunnel_key_print_vxlan_options(struct rtattr *attr)
 	print_uint(PRINT_FP, NULL, "0x%x", gbp);
 }
 
+static void tunnel_key_print_erspan_options(struct rtattr *attr)
+{
+	struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_MAX + 1];
+	struct rtattr *i = RTA_DATA(attr);
+	int rem = RTA_PAYLOAD(attr);
+	char *name = "erspan_opts";
+	char strbuf[rem * 2 + 1];
+	__u8 ver, hwid, dir;
+	__u32 idx;
+
+	parse_rtattr(tb, TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_MAX, i, rem);
+	ver = rta_getattr_u8(tb[TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_VER]);
+	if (ver == 1) {
+		idx = rta_getattr_be32(tb[TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_INDEX]);
+		dir = 0;
+		hwid = 0;
+	} else {
+		idx = 0;
+		dir = rta_getattr_u8(tb[TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_DIR]);
+		hwid = rta_getattr_u8(tb[TCA_TUNNEL_KEY_ENC_OPT_ERSPAN_HWID]);
+	}
+
+	open_json_array(PRINT_JSON, name);
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "ver", NULL, ver);
+	print_uint(PRINT_JSON, "index", NULL, idx);
+	print_uint(PRINT_JSON, "dir", NULL, dir);
+	print_uint(PRINT_JSON, "hwid", NULL, hwid);
+	close_json_object();
+	close_json_array(PRINT_JSON, name);
+
+	print_nl();
+	print_string(PRINT_FP, name, "\t%s ", name);
+	sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
+	print_string(PRINT_FP, NULL, "%s", strbuf);
+}
+
 static void tunnel_key_print_key_opt(struct rtattr *attr)
 {
 	struct rtattr *tb[TCA_TUNNEL_KEY_ENC_OPTS_MAX + 1];
@@ -533,6 +649,9 @@ static void tunnel_key_print_key_opt(struct rtattr *attr)
 	else if (tb[TCA_TUNNEL_KEY_ENC_OPTS_VXLAN])
 		tunnel_key_print_vxlan_options(
 			tb[TCA_TUNNEL_KEY_ENC_OPTS_VXLAN]);
+	else if (tb[TCA_TUNNEL_KEY_ENC_OPTS_ERSPAN])
+		tunnel_key_print_erspan_options(
+			tb[TCA_TUNNEL_KEY_ENC_OPTS_ERSPAN]);
 }
 
 static void tunnel_key_print_tos_ttl(FILE *f, char *name,
-- 
2.1.0

[PATCHv3 iproute2-next 6/7] tc: f_flower: add options support for vxlan

[Xin Long]

This patch is to add TCA_FLOWER_KEY_ENC_OPTS_VXLAN's parse and
print to implement vxlan options support in m_tunnel_key, like
Commit 56155d4df86d ("tc: f_flower: add geneve option match
support to flower") for geneve options support.

Option is expressed a 32bit hexadecimal value for gbp only, and
vxlan doesn't support multiple options.

With this patch, users can add and dump vxlan options like:

  # ip link add name vxlan1 type vxlan dstport 0 external
  # tc qdisc add dev vxlan1 ingress
  # tc filter add dev vxlan1 protocol ip parent ffff: \
      flower \
        enc_src_ip 10.0.99.192 \
        enc_dst_ip 10.0.99.193 \
        enc_key_id 11 \
        vxlan_opts 0x10101/0xeeeeee3e \
        ip_proto udp \
        action mirred egress redirect dev eth1
  # tc -s filter show dev vxlan1 parent ffff:

     filter protocol ip pref 49152 flower chain 0 handle 0x1
       eth_type ipv4
       ip_proto udp
       enc_dst_ip 10.0.99.193
       enc_src_ip 10.0.99.192
       enc_key_id 11
       vxlan_opts 0x10101/0xeeeeee3e
       not_in_hw
         action order 1: mirred (Egress Redirect to device eth1) stolen
         index 3 ref 1 bind 1
         Action statistics:
         Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
         backlog 0b 0p requeues 0

v1->v2:
  - get_u32 with base = 0 for gbp.
v2->v3:
  - implement proper JSON array for opts.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 man/man8/tc-flower.8 |  12 +++++
 tc/f_flower.c        | 130 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 126 insertions(+), 16 deletions(-)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index eb9eb5f..819932d 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -81,7 +81,11 @@ flower \- flow based traffic control filter
 .IR TOS " | "
 .B enc_ttl
 .IR TTL " | "
+{
 .B geneve_opts
+|
+.B vxlan_opts
+}
 .IR OPTIONS " | "
 .BR ip_flags
 .IR IP_FLAGS
@@ -326,6 +330,8 @@ Match the connection zone, and can be masked.
 .RE
 .TP
 .BI geneve_opts " OPTIONS"
+.TQ
+.BI vxlan_opts " OPTIONS"
 Match on IP tunnel metadata. Key id
 .I NUMBER
 is a 32 bit tunnel key id (e.g. VNI for VXLAN tunnel).
@@ -346,6 +352,12 @@ the masks is missing, \fBtc\fR assumes a full-length match. The options can
 be described in the form CLASS:TYPE:DATA/CLASS_MASK:TYPE_MASK:DATA_MASK,
 where CLASS is represented as a 16bit hexadecimal value, TYPE as an 8bit
 hexadecimal value and DATA as a variable length hexadecimal value.
+vxlan_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 GBP/GBP_MASK, where GBP is
+represented as a 32bit hexadecimal value.
 .TP
 .BI ip_flags " IP_FLAGS"
 .I IP_FLAGS
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 9d59d71..8b195c5 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -81,6 +81,7 @@ static void explain(void)
 		"			enc_tos MASKED-IP_TOS |\n"
 		"			enc_ttl MASKED-IP_TTL |\n"
 		"			geneve_opts MASKED-OPTIONS |\n"
+		"			vxlan_opts MASKED-OPTIONS |\n"
 		"			ip_flags IP-FLAGS | \n"
 		"			enc_dst_port [ port_number ] |\n"
 		"			ct_state MASKED_CT_STATE |\n"
@@ -847,7 +848,7 @@ static int flower_parse_enc_port(char *str, int type, struct nlmsghdr *n)
 	return 0;
 }
 
-static int flower_parse_geneve_opts(char *str, struct nlmsghdr *n)
+static int flower_parse_geneve_opt(char *str, struct nlmsghdr *n)
 {
 	struct rtattr *nest;
 	char *token;
@@ -917,14 +918,33 @@ static int flower_parse_geneve_opts(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
-static int flower_parse_enc_opt_part(char *str, struct nlmsghdr *n)
+static int flower_parse_vxlan_opt(char *str, struct nlmsghdr *n)
+{
+	struct rtattr *nest;
+	__u32 gbp;
+	int err;
+
+	nest = addattr_nest(n, MAX_MSG,
+			    TCA_FLOWER_KEY_ENC_OPTS_VXLAN | NLA_F_NESTED);
+
+	err = get_u32(&gbp, str, 0);
+	if (err)
+		return err;
+	addattr32(n, MAX_MSG, TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP, gbp);
+
+	addattr_nest_end(n, nest);
+
+	return 0;
+}
+
+static int flower_parse_geneve_opts(char *str, struct nlmsghdr *n)
 {
 	char *token;
 	int err;
 
 	token = strsep(&str, ",");
 	while (token) {
-		err = flower_parse_geneve_opts(token, n);
+		err = flower_parse_geneve_opt(token, n);
 		if (err)
 			return err;
 
@@ -954,7 +974,7 @@ static int flower_check_enc_opt_key(char *key)
 	return 0;
 }
 
-static int flower_parse_enc_opts(char *str, struct nlmsghdr *n)
+static int flower_parse_enc_opts_geneve(char *str, struct nlmsghdr *n)
 {
 	char key[XATTR_SIZE_MAX], mask[XATTR_SIZE_MAX];
 	int data_len, key_len, mask_len, err;
@@ -1006,13 +1026,50 @@ static int flower_parse_enc_opts(char *str, struct nlmsghdr *n)
 	mask[mask_len - 1] = '\0';
 
 	nest = addattr_nest(n, MAX_MSG, TCA_FLOWER_KEY_ENC_OPTS);
-	err = flower_parse_enc_opt_part(key, n);
+	err = flower_parse_geneve_opts(key, n);
 	if (err)
 		return err;
 	addattr_nest_end(n, nest);
 
 	nest = addattr_nest(n, MAX_MSG, TCA_FLOWER_KEY_ENC_OPTS_MASK);
-	err = flower_parse_enc_opt_part(mask, n);
+	err = flower_parse_geneve_opts(mask, n);
+	if (err)
+		return err;
+	addattr_nest_end(n, nest);
+
+	return 0;
+}
+
+static int flower_parse_enc_opts_vxlan(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, "ffffffff");
+	}
+
+	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_vxlan_opt(str, 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_vxlan_opt(mask, n);
 	if (err)
 		return err;
 	addattr_nest_end(n, nest);
@@ -1502,11 +1559,18 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 			}
 		} else if (matches(*argv, "geneve_opts") == 0) {
 			NEXT_ARG();
-			ret = flower_parse_enc_opts(*argv, n);
+			ret = flower_parse_enc_opts_geneve(*argv, n);
 			if (ret < 0) {
 				fprintf(stderr, "Illegal \"geneve_opts\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "vxlan_opts") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_enc_opts_vxlan(*argv, n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"vxlan_opts\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "action") == 0) {
 			NEXT_ARG();
 			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -1940,10 +2004,29 @@ static void flower_print_geneve_opts(const char *name, struct rtattr *attr,
 	close_json_array(PRINT_JSON, name);
 }
 
-static void flower_print_geneve_parts(const char *name, struct rtattr *attr,
-				      char *key, char *mask)
+static void flower_print_vxlan_opts(const char *name, struct rtattr *attr,
+				    char *strbuf)
+{
+	struct rtattr *tb[TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX + 1];
+	struct rtattr *i = RTA_DATA(attr);
+	int rem = RTA_PAYLOAD(attr);
+	__u32 gbp;
+
+	parse_rtattr(tb, TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX, i, rem);
+	gbp = rta_getattr_u32(tb[TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP]);
+
+	open_json_array(PRINT_JSON, name);
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "gbp", NULL, gbp);
+	close_json_object();
+	close_json_array(PRINT_JSON, name);
+
+	sprintf(strbuf, "%#x", gbp);
+}
+
+static void flower_print_enc_parts(const char *name, const char *namefrm,
+				   struct rtattr *attr, char *key, char *mask)
 {
-	char *namefrm = "  geneve_opt %s";
 	char *key_token, *mask_token, *out;
 	int len;
 
@@ -1985,14 +2068,29 @@ static void flower_print_enc_opts(const char *name, struct rtattr *attr,
 		goto err_key_free;
 
 	parse_rtattr_nested(key_tb, TCA_FLOWER_KEY_ENC_OPTS_MAX, attr);
-	flower_print_geneve_opts("geneve_opt_key",
-				 key_tb[TCA_FLOWER_KEY_ENC_OPTS_GENEVE], key);
-
 	parse_rtattr_nested(msk_tb, TCA_FLOWER_KEY_ENC_OPTS_MAX, mask_attr);
-	flower_print_geneve_opts("geneve_opt_mask",
-				 msk_tb[TCA_FLOWER_KEY_ENC_OPTS_GENEVE], msk);
 
-	flower_print_geneve_parts(name, attr, key, msk);
+	if (key_tb[TCA_FLOWER_KEY_ENC_OPTS_GENEVE]) {
+		flower_print_geneve_opts("geneve_opt_key",
+				key_tb[TCA_FLOWER_KEY_ENC_OPTS_GENEVE], key);
+
+		if (msk_tb[TCA_FLOWER_KEY_ENC_OPTS_GENEVE])
+			flower_print_geneve_opts("geneve_opt_mask",
+				msk_tb[TCA_FLOWER_KEY_ENC_OPTS_GENEVE], msk);
+
+		flower_print_enc_parts(name, "  geneve_opts %s", attr, key,
+				       msk);
+	} else if (key_tb[TCA_FLOWER_KEY_ENC_OPTS_VXLAN]) {
+		flower_print_vxlan_opts("vxlan_opt_key",
+				key_tb[TCA_FLOWER_KEY_ENC_OPTS_VXLAN], key);
+
+		if (msk_tb[TCA_FLOWER_KEY_ENC_OPTS_VXLAN])
+			flower_print_vxlan_opts("vxlan_opt_mask",
+				msk_tb[TCA_FLOWER_KEY_ENC_OPTS_VXLAN], msk);
+
+		flower_print_enc_parts(name, "  vxlan_opts %s", attr, key,
+				       msk);
+	}
 
 	free(msk);
 err_key_free:
-- 
2.1.0

[PATCHv3 iproute2-next 7/7] tc: f_flower: add options support for erspan

[Xin Long]

This patch is to add TCA_FLOWER_KEY_ENC_OPTS_ERSPAN's parse and
print to implement erspan options support in m_tunnel_key, like
Commit 56155d4df86d ("tc: f_flower: add geneve option match
support to flower") for geneve options support.

Option is expressed as version:index:dir:hwid, dir and hwid will
be parsed when version is 2, while index will be parsed when
version is 1. erspan doesn't support multiple options.

With this patch, users can add and dump erspan options like:

  # ip link add name erspan1 type erspan external
  # tc qdisc add dev erspan1 ingress
  # tc filter add dev erspan1 protocol ip parent ffff: \
      flower \
        enc_src_ip 10.0.99.192 \
        enc_dst_ip 10.0.99.193 \
        enc_key_id 11 \
        erspan_opts 1:2:0:0/01:ffffffff:00:00 \
        ip_proto udp \
        action mirred egress redirect dev eth1
  # tc -s filter show dev erspan1 parent ffff:

     filter protocol ip pref 49152 flower chain 0 handle 0x1
       eth_type ipv4
       ip_proto udp
       enc_dst_ip 10.0.99.193
       enc_src_ip 10.0.99.192
       enc_key_id 11
       erspan_opts 01:00000002:00:00/01:ffffffff:00:00
       not_in_hw
         action order 1: mirred (Egress Redirect to device eth1) stolen
         index 1 ref 1 bind 1
         Action statistics:
         Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
         backlog 0b 0p requeues 0

v1->v2:
  - no change.
v2->v3:
  - no change.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 man/man8/tc-flower.8 |  14 +++++
 tc/f_flower.c        | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 185 insertions(+)

diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
index 819932d..5e678ba 100644
--- a/man/man8/tc-flower.8
+++ b/man/man8/tc-flower.8
@@ -85,6 +85,8 @@ flower \- flow based traffic control filter
 .B geneve_opts
 |
 .B vxlan_opts
+|
+.B erspan_opts
 }
 .IR OPTIONS " | "
 .BR ip_flags
@@ -332,6 +334,8 @@ Match the connection zone, and can be masked.
 .BI geneve_opts " OPTIONS"
 .TQ
 .BI vxlan_opts " OPTIONS"
+.TQ
+.BI erspan_opts " OPTIONS"
 Match on IP tunnel metadata. Key id
 .I NUMBER
 is a 32 bit tunnel key id (e.g. VNI for VXLAN tunnel).
@@ -358,6 +362,16 @@ 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 GBP/GBP_MASK, where GBP is
 represented as a 32bit hexadecimal value.
+erspan_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
+VERSION:INDEX:DIR:HWID/VERSION:INDEX_MASK:DIR_MASK:HWID_MASK, where VERSION is
+represented as a 8bit hexadecimal value, INDEX as an 32bit hexadecimal value,
+DIR and HWID as a 8bit hexadecimal value. 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.
 .TP
 .BI ip_flags " IP_FLAGS"
 .I IP_FLAGS
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 8b195c5..1281479 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -82,6 +82,7 @@ static void explain(void)
 		"			enc_ttl MASKED-IP_TTL |\n"
 		"			geneve_opts MASKED-OPTIONS |\n"
 		"			vxlan_opts MASKED-OPTIONS |\n"
+		"                       erspan_opts MASKED-OPTIONS |\n"
 		"			ip_flags IP-FLAGS | \n"
 		"			enc_dst_port [ port_number ] |\n"
 		"			ct_state MASKED_CT_STATE |\n"
@@ -937,6 +938,84 @@ static int flower_parse_vxlan_opt(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int flower_parse_erspan_opt(char *str, struct nlmsghdr *n)
+{
+	struct rtattr *nest;
+	char *token;
+	int i, err;
+
+	nest = addattr_nest(n, MAX_MSG,
+			    TCA_FLOWER_KEY_ENC_OPTS_ERSPAN | NLA_F_NESTED);
+
+	i = 1;
+	token = strsep(&str, ":");
+	while (token) {
+		switch (i) {
+		case TCA_FLOWER_KEY_ENC_OPT_ERSPAN_VER:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			addattr8(n, MAX_MSG, i, opt_type);
+			break;
+		}
+		case TCA_FLOWER_KEY_ENC_OPT_ERSPAN_INDEX:
+		{
+			__be32 opt_index;
+
+			if (!strlen(token))
+				break;
+			err = get_be32(&opt_index, token, 16);
+			if (err)
+				return err;
+
+			addattr32(n, MAX_MSG, i, opt_index);
+			break;
+		}
+		case TCA_FLOWER_KEY_ENC_OPT_ERSPAN_DIR:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			addattr8(n, MAX_MSG, i, opt_type);
+			break;
+		}
+		case TCA_FLOWER_KEY_ENC_OPT_ERSPAN_HWID:
+		{
+			__u8 opt_type;
+
+			if (!strlen(token))
+				break;
+			err = get_u8(&opt_type, token, 16);
+			if (err)
+				return err;
+
+			addattr8(n, MAX_MSG, i, opt_type);
+			break;
+		}
+		default:
+			fprintf(stderr, "Unknown \"geneve_opts\" type\n");
+			return -1;
+		}
+
+		token = strsep(&str, ":");
+		i++;
+	}
+	addattr_nest_end(n, nest);
+
+	return 0;
+}
+
 static int flower_parse_geneve_opts(char *str, struct nlmsghdr *n)
 {
 	char *token;
@@ -1077,6 +1156,49 @@ static int flower_parse_enc_opts_vxlan(char *str, struct nlmsghdr *n)
 	return 0;
 }
 
+static int flower_parse_enc_opts_erspan(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 {
+		int index;
+
+		slash = strchr(str, ':');
+		index = (int)(slash - str);
+		memcpy(mask, str, index);
+		strcpy(mask + index, ":ffffffff: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_erspan_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_erspan_opt(mask, n);
+	if (err)
+		return err;
+	addattr_nest_end(n, nest);
+
+	return 0;
+}
+
 static int flower_parse_opt(struct filter_util *qu, char *handle,
 			    int argc, char **argv, struct nlmsghdr *n)
 {
@@ -1571,6 +1693,13 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
 				fprintf(stderr, "Illegal \"vxlan_opts\"\n");
 				return -1;
 			}
+		} else if (matches(*argv, "erspan_opts") == 0) {
+			NEXT_ARG();
+			ret = flower_parse_enc_opts_erspan(*argv, n);
+			if (ret < 0) {
+				fprintf(stderr, "Illegal \"erspan_opts\"\n");
+				return -1;
+			}
 		} else if (matches(*argv, "action") == 0) {
 			NEXT_ARG();
 			ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -2024,6 +2153,38 @@ static void flower_print_vxlan_opts(const char *name, struct rtattr *attr,
 	sprintf(strbuf, "%#x", gbp);
 }
 
+static void flower_print_erspan_opts(const char *name, struct rtattr *attr,
+				     char *strbuf)
+{
+	struct rtattr *tb[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX + 1];
+	__u8 ver, hwid, dir;
+	__u32 idx;
+
+	parse_rtattr(tb, TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX, RTA_DATA(attr),
+		     RTA_PAYLOAD(attr));
+	ver = rta_getattr_u8(tb[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_VER]);
+	if (ver == 1) {
+		idx = rta_getattr_be32(tb[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_INDEX]);
+		hwid = 0;
+		dir = 0;
+	} else {
+		idx = 0;
+		hwid = rta_getattr_u8(tb[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_HWID]);
+		dir = rta_getattr_u8(tb[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_DIR]);
+	}
+
+	open_json_array(PRINT_JSON, name);
+	open_json_object(NULL);
+	print_uint(PRINT_JSON, "ver", NULL, ver);
+	print_uint(PRINT_JSON, "index", NULL, idx);
+	print_uint(PRINT_JSON, "dir", NULL, dir);
+	print_uint(PRINT_JSON, "hwid", NULL, hwid);
+	close_json_object();
+	close_json_array(PRINT_JSON, name);
+
+	sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
+}
+
 static void flower_print_enc_parts(const char *name, const char *namefrm,
 				   struct rtattr *attr, char *key, char *mask)
 {
@@ -2090,6 +2251,16 @@ static void flower_print_enc_opts(const char *name, struct rtattr *attr,
 
 		flower_print_enc_parts(name, "  vxlan_opts %s", attr, key,
 				       msk);
+	} else if (key_tb[TCA_FLOWER_KEY_ENC_OPTS_ERSPAN]) {
+		flower_print_erspan_opts("erspan_opt_key",
+				key_tb[TCA_FLOWER_KEY_ENC_OPTS_ERSPAN], key);
+
+		if (msk_tb[TCA_FLOWER_KEY_ENC_OPTS_ERSPAN])
+			flower_print_erspan_opts("erspan_opt_mask",
+				msk_tb[TCA_FLOWER_KEY_ENC_OPTS_ERSPAN], msk);
+
+		flower_print_enc_parts(name, "  erspan_opts %s", attr, key,
+				       msk);
 	}
 
 	free(msk);
-- 
2.1.0

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Stephen Hemminger]

On Fri, 14 Feb 2020 18:30:47 +0800
Xin Long <lucien.xin@gmail.com> wrote:

> +
> +	open_json_array(PRINT_JSON, name);
> +	open_json_object(NULL);
> +	print_uint(PRINT_JSON, "ver", NULL, ver);
> +	print_uint(PRINT_JSON, "index", NULL, idx);
> +	print_uint(PRINT_JSON, "dir", NULL, dir);
> +	print_uint(PRINT_JSON, "hwid", NULL, hwid);
> +	close_json_object();
> +	close_json_array(PRINT_JSON, name);
> +
> +	print_nl();
> +	print_string(PRINT_FP, name, "\t%s ", name);
> +	sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
> +	print_string(PRINT_FP, NULL, "%s ", strbuf);
> +}

Instead of having two sets of prints, is it possible to do this
	print_nl();
	print_string(PRINT_FP, NULL, "\t", NULL);

	open_json_array(PRINT_ANY, name);
	open_json_object(NULL);
	print_0xhex(PRINT_ANY, "ver", " %02x", ver);
	print_0xhex(PRINT_ANY, "idx", ":%08x", idx);
	print_0xhex(PRINT_ANY, "dir", ":%02x", dir);
	print_0xhex(PRINT_ANY, "hwid", ":%02x", hwid)
	close_json_object();
	close_json_array(PRINT_ANY, " ");

Also, you seem to not hear the request to not use opaque hex values
in the iproute2 interface. The version, index, etc should be distinct
parameter values not a hex string.

I think this still needs more work before merging.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Sat, Feb 15, 2020 at 12:13 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri, 14 Feb 2020 18:30:47 +0800
> Xin Long <lucien.xin@gmail.com> wrote:
>
> > +
> > +     open_json_array(PRINT_JSON, name);
> > +     open_json_object(NULL);
> > +     print_uint(PRINT_JSON, "ver", NULL, ver);
> > +     print_uint(PRINT_JSON, "index", NULL, idx);
> > +     print_uint(PRINT_JSON, "dir", NULL, dir);
> > +     print_uint(PRINT_JSON, "hwid", NULL, hwid);
> > +     close_json_object();
> > +     close_json_array(PRINT_JSON, name);
> > +
> > +     print_nl();
> > +     print_string(PRINT_FP, name, "\t%s ", name);
> > +     sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
> > +     print_string(PRINT_FP, NULL, "%s ", strbuf);
> > +}
>
> Instead of having two sets of prints, is it possible to do this
>         print_nl();
>         print_string(PRINT_FP, NULL, "\t", NULL);
>
>         open_json_array(PRINT_ANY, name);
>         open_json_object(NULL);
>         print_0xhex(PRINT_ANY, "ver", " %02x", ver);
>         print_0xhex(PRINT_ANY, "idx", ":%08x", idx);
>         print_0xhex(PRINT_ANY, "dir", ":%02x", dir);
>         print_0xhex(PRINT_ANY, "hwid", ":%02x", hwid)
>         close_json_object();
>         close_json_array(PRINT_ANY, " ");
Hi Stephen,

This's not gonna work. as the output will be:
{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
instead of
{"ver":2,"index":0,"dir":1,"hwid":2} (number)

>
> Also, you seem to not hear the request to not use opaque hex values
> in the iproute2 interface. The version, index, etc should be distinct
> parameter values not a hex string.
The opts STRING, especially these like "XX:YY:ZZ" are represented
as hex string on both adding and dumping. It is to keep consistent with
geneve_opts in m_tunnel_key and f_flower,  see

commit 6217917a382682d8e8a7ecdeb0c6626f701a0933
Author: Simon Horman <simon.horman@netronome.com>
Date:   Thu Jul 5 17:12:00 2018 -0700

    tc: m_tunnel_key: Add tunnel option support to act_tunnel_key

and
commit 56155d4df86d489c4207444c8a90ce4e0e22e49f
Author: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Date:   Fri Sep 28 16:03:39 2018 +0200

    tc: f_flower: add geneve option match support to flower

and actually, the code type I'm using is exactly from these 2 patches.
please take a look.

I don't think this patchset should go to another type of format for opts.

Thanks.

>
> I think this still needs more work before merging.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Stephen Hemminger]

On Sat, 15 Feb 2020 01:40:27 +0800
Xin Long <lucien.xin@gmail.com> wrote:

> This's not gonna work. as the output will be:
> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> instead of
> {"ver":2,"index":0,"dir":1,"hwid":2} (number)

JSON is typeless. Lots of values are already printed in hex

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sat, 15 Feb 2020 01:40:27 +0800
> Xin Long <lucien.xin@gmail.com> wrote:
>
> > This's not gonna work. as the output will be:
> > {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> > instead of
> > {"ver":2,"index":0,"dir":1,"hwid":2} (number)
>
> JSON is typeless. Lots of values are already printed in hex
You may mean JSON data itself is typeless.
But JSON objects are typed when parsing JSON data, which includes
string, number, array, boolean. So it matters how to define the
members' 'type' in JSON data.

For example, in python's 'json' module:

#!/usr/bin/python2
import json
json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
parsed_json_1 = (json.loads(json_data_1))
parsed_json_2 = (json.loads(json_data_2))
print type(parsed_json_1["hwid"])
print type(parsed_json_2["hwid"])

The output is:
<type 'unicode'>
<type 'int'>

Also, '{"result": true}' is different from '{"result": "true"}' when
loading it in a 3rd-party lib.

I think the JSON data coming from iproute2 is designed to be used by
a 3rd-party lib to parse, not just to show to users. To keep these
members' original type (numbers) is more appropriate, IMO.

Thanks.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[David Ahern]

On 2/14/20 9:18 PM, Xin Long wrote:
> On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>>
>> On Sat, 15 Feb 2020 01:40:27 +0800
>> Xin Long <lucien.xin@gmail.com> wrote:
>>
>>> This's not gonna work. as the output will be:
>>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
>>> instead of
>>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
>>
>> JSON is typeless. Lots of values are already printed in hex
> You may mean JSON data itself is typeless.
> But JSON objects are typed when parsing JSON data, which includes
> string, number, array, boolean. So it matters how to define the
> members' 'type' in JSON data.
> 
> For example, in python's 'json' module:
> 
> #!/usr/bin/python2
> import json
> json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
> json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
> parsed_json_1 = (json.loads(json_data_1))
> parsed_json_2 = (json.loads(json_data_2))
> print type(parsed_json_1["hwid"])
> print type(parsed_json_2["hwid"])
> 
> The output is:
> <type 'unicode'>
> <type 'int'>
> 
> Also, '{"result": true}' is different from '{"result": "true"}' when
> loading it in a 3rd-party lib.
> 
> I think the JSON data coming from iproute2 is designed to be used by
> a 3rd-party lib to parse, not just to show to users. To keep these
> members' original type (numbers) is more appropriate, IMO.
> 

Stephen: why do you think all of the numbers should be in hex?

It seems like consistency with existing output should matter more.
ip/link_gre.c for instance prints index as an int, version as an int,
direction as a string and only hwid in hex.

Xin: any reason you did not follow the output of the existing netdev
based solutions?

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Sun, Feb 16, 2020 at 12:51 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 2/14/20 9:18 PM, Xin Long wrote:
> > On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> >>
> >> On Sat, 15 Feb 2020 01:40:27 +0800
> >> Xin Long <lucien.xin@gmail.com> wrote:
> >>
> >>> This's not gonna work. as the output will be:
> >>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> >>> instead of
> >>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
> >>
> >> JSON is typeless. Lots of values are already printed in hex
> > You may mean JSON data itself is typeless.
> > But JSON objects are typed when parsing JSON data, which includes
> > string, number, array, boolean. So it matters how to define the
> > members' 'type' in JSON data.
> >
> > For example, in python's 'json' module:
> >
> > #!/usr/bin/python2
> > import json
> > json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
> > json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
> > parsed_json_1 = (json.loads(json_data_1))
> > parsed_json_2 = (json.loads(json_data_2))
> > print type(parsed_json_1["hwid"])
> > print type(parsed_json_2["hwid"])
> >
> > The output is:
> > <type 'unicode'>
> > <type 'int'>
> >
> > Also, '{"result": true}' is different from '{"result": "true"}' when
> > loading it in a 3rd-party lib.
> >
> > I think the JSON data coming from iproute2 is designed to be used by
> > a 3rd-party lib to parse, not just to show to users. To keep these
> > members' original type (numbers) is more appropriate, IMO.
> >
>
> Stephen: why do you think all of the numbers should be in hex?
>
> It seems like consistency with existing output should matter more.
> ip/link_gre.c for instance prints index as an int, version as an int,
> direction as a string and only hwid in hex.
>
> Xin: any reason you did not follow the output of the existingg netdev
> based solutions?
Hi David,

Option is expressed as "version:index:dir:hwid", I made all fields
in this string of hex, just like "class:type:data" in:

commit 0ed5269f9e41f495c8e9020c85f5e1644c1afc57
Author: Simon Horman <simon.horman@netronome.com>
Date:   Tue Jun 26 21:39:37 2018 -0700

    net/sched: add tunnel option support to act_tunnel_key

I'm not sure if it's good to mix multiple types in this string. wdyt?

but for the JSON data, of course, these are all numbers(not string).

Thanks.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[David Ahern]

On 2/15/20 11:38 PM, Xin Long wrote:
> On Sun, Feb 16, 2020 at 12:51 AM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 2/14/20 9:18 PM, Xin Long wrote:
>>> On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
>>> <stephen@networkplumber.org> wrote:
>>>>
>>>> On Sat, 15 Feb 2020 01:40:27 +0800
>>>> Xin Long <lucien.xin@gmail.com> wrote:
>>>>
>>>>> This's not gonna work. as the output will be:
>>>>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
>>>>> instead of
>>>>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
>>>>
>>>> JSON is typeless. Lots of values are already printed in hex
>>> You may mean JSON data itself is typeless.
>>> But JSON objects are typed when parsing JSON data, which includes
>>> string, number, array, boolean. So it matters how to define the
>>> members' 'type' in JSON data.
>>>
>>> For example, in python's 'json' module:
>>>
>>> #!/usr/bin/python2
>>> import json
>>> json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
>>> json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
>>> parsed_json_1 = (json.loads(json_data_1))
>>> parsed_json_2 = (json.loads(json_data_2))
>>> print type(parsed_json_1["hwid"])
>>> print type(parsed_json_2["hwid"])
>>>
>>> The output is:
>>> <type 'unicode'>
>>> <type 'int'>
>>>
>>> Also, '{"result": true}' is different from '{"result": "true"}' when
>>> loading it in a 3rd-party lib.
>>>
>>> I think the JSON data coming from iproute2 is designed to be used by
>>> a 3rd-party lib to parse, not just to show to users. To keep these
>>> members' original type (numbers) is more appropriate, IMO.
>>>
>>
>> Stephen: why do you think all of the numbers should be in hex?
>>
>> It seems like consistency with existing output should matter more.
>> ip/link_gre.c for instance prints index as an int, version as an int,
>> direction as a string and only hwid in hex.
>>
>> Xin: any reason you did not follow the output of the existingg netdev
>> based solutions?
> Hi David,
> 
> Option is expressed as "version:index:dir:hwid", I made all fields
> in this string of hex, just like "class:type:data" in:
> 
> commit 0ed5269f9e41f495c8e9020c85f5e1644c1afc57
> Author: Simon Horman <simon.horman@netronome.com>
> Date:   Tue Jun 26 21:39:37 2018 -0700
> 
>     net/sched: add tunnel option support to act_tunnel_key
> 
> I'm not sure if it's good to mix multiple types in this string. wdyt?
> 
> but for the JSON data, of course, these are all numbers(not string).
> 

I don't understand why Stephen is pushing for hex; it does not make
sense for version, index or direction. I don't have a clear
understanding of hwid to know uint vs hex, so your current JSON prints
seem fine.

As for the stdout print and hex fields, staring at the tc and lwtunnel
code, it seems like those 2 have a lot of parallels in expressing
options for encoding vs lwtunnel and netdev based code. ie., I think
this latest set is correct.

Stephen?

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Stephen Hemminger]

On Mon, 17 Feb 2020 12:53:14 -0700
David Ahern <dsahern@gmail.com> wrote:

> On 2/15/20 11:38 PM, Xin Long wrote:
> > On Sun, Feb 16, 2020 at 12:51 AM David Ahern <dsahern@gmail.com> wrote:  
> >>
> >> On 2/14/20 9:18 PM, Xin Long wrote:  
> >>> On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
> >>> <stephen@networkplumber.org> wrote:  
> >>>>
> >>>> On Sat, 15 Feb 2020 01:40:27 +0800
> >>>> Xin Long <lucien.xin@gmail.com> wrote:
> >>>>  
> >>>>> This's not gonna work. as the output will be:
> >>>>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> >>>>> instead of
> >>>>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)  
> >>>>
> >>>> JSON is typeless. Lots of values are already printed in hex  
> >>> You may mean JSON data itself is typeless.
> >>> But JSON objects are typed when parsing JSON data, which includes
> >>> string, number, array, boolean. So it matters how to define the
> >>> members' 'type' in JSON data.
> >>>
> >>> For example, in python's 'json' module:
> >>>
> >>> #!/usr/bin/python2
> >>> import json
> >>> json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
> >>> json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
> >>> parsed_json_1 = (json.loads(json_data_1))
> >>> parsed_json_2 = (json.loads(json_data_2))
> >>> print type(parsed_json_1["hwid"])
> >>> print type(parsed_json_2["hwid"])
> >>>
> >>> The output is:
> >>> <type 'unicode'>
> >>> <type 'int'>
> >>>
> >>> Also, '{"result": true}' is different from '{"result": "true"}' when
> >>> loading it in a 3rd-party lib.
> >>>
> >>> I think the JSON data coming from iproute2 is designed to be used by
> >>> a 3rd-party lib to parse, not just to show to users. To keep these
> >>> members' original type (numbers) is more appropriate, IMO.
> >>>  
> >>
> >> Stephen: why do you think all of the numbers should be in hex?
> >>
> >> It seems like consistency with existing output should matter more.
> >> ip/link_gre.c for instance prints index as an int, version as an int,
> >> direction as a string and only hwid in hex.
> >>
> >> Xin: any reason you did not follow the output of the existingg netdev
> >> based solutions?  
> > Hi David,
> > 
> > Option is expressed as "version:index:dir:hwid", I made all fields
> > in this string of hex, just like "class:type:data" in:
> > 
> > commit 0ed5269f9e41f495c8e9020c85f5e1644c1afc57
> > Author: Simon Horman <simon.horman@netronome.com>
> > Date:   Tue Jun 26 21:39:37 2018 -0700
> > 
> >     net/sched: add tunnel option support to act_tunnel_key
> > 
> > I'm not sure if it's good to mix multiple types in this string. wdyt?
> > 
> > but for the JSON data, of course, these are all numbers(not string).
> >   
> 
> I don't understand why Stephen is pushing for hex; it does not make
> sense for version, index or direction. I don't have a clear
> understanding of hwid to know uint vs hex, so your current JSON prints
> seem fine.
> 
> As for the stdout print and hex fields, staring at the tc and lwtunnel
> code, it seems like those 2 have a lot of parallels in expressing
> options for encoding vs lwtunnel and netdev based code. ie., I think
> this latest set is correct.
> 
> Stephen?

I just wanted:
1. The parse and print functions should have the same formats.
I.e. if you take the output and do a little massaging of the ifindex
it should be accepted as an input set of parameters.

2. As much as possible, the JSON and non-JSON output should be similar.
If non-JSON prints in hex, then JSON should display hex and vice/versa.

Ideally all inputs would be human format (not machine formats like hex).
But I guess the mistake was already made with some of the other tunnels.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Tue, Feb 18, 2020 at 5:03 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 17 Feb 2020 12:53:14 -0700
> David Ahern <dsahern@gmail.com> wrote:
>
> > On 2/15/20 11:38 PM, Xin Long wrote:
> > > On Sun, Feb 16, 2020 at 12:51 AM David Ahern <dsahern@gmail.com> wrote:
> > >>
> > >> On 2/14/20 9:18 PM, Xin Long wrote:
> > >>> On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
> > >>> <stephen@networkplumber.org> wrote:
> > >>>>
> > >>>> On Sat, 15 Feb 2020 01:40:27 +0800
> > >>>> Xin Long <lucien.xin@gmail.com> wrote:
> > >>>>
> > >>>>> This's not gonna work. as the output will be:
> > >>>>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> > >>>>> instead of
> > >>>>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
> > >>>>
> > >>>> JSON is typeless. Lots of values are already printed in hex
> > >>> You may mean JSON data itself is typeless.
> > >>> But JSON objects are typed when parsing JSON data, which includes
> > >>> string, number, array, boolean. So it matters how to define the
> > >>> members' 'type' in JSON data.
> > >>>
> > >>> For example, in python's 'json' module:
> > >>>
> > >>> #!/usr/bin/python2
> > >>> import json
> > >>> json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
> > >>> json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
> > >>> parsed_json_1 = (json.loads(json_data_1))
> > >>> parsed_json_2 = (json.loads(json_data_2))
> > >>> print type(parsed_json_1["hwid"])
> > >>> print type(parsed_json_2["hwid"])
> > >>>
> > >>> The output is:
> > >>> <type 'unicode'>
> > >>> <type 'int'>
> > >>>
> > >>> Also, '{"result": true}' is different from '{"result": "true"}' when
> > >>> loading it in a 3rd-party lib.
> > >>>
> > >>> I think the JSON data coming from iproute2 is designed to be used by
> > >>> a 3rd-party lib to parse, not just to show to users. To keep these
> > >>> members' original type (numbers) is more appropriate, IMO.
> > >>>
> > >>
> > >> Stephen: why do you think all of the numbers should be in hex?
> > >>
> > >> It seems like consistency with existing output should matter more.
> > >> ip/link_gre.c for instance prints index as an int, version as an int,
> > >> direction as a string and only hwid in hex.
> > >>
> > >> Xin: any reason you did not follow the output of the existingg netdev
> > >> based solutions?
> > > Hi David,
> > >
> > > Option is expressed as "version:index:dir:hwid", I made all fields
> > > in this string of hex, just like "class:type:data" in:
> > >
> > > commit 0ed5269f9e41f495c8e9020c85f5e1644c1afc57
> > > Author: Simon Horman <simon.horman@netronome.com>
> > > Date:   Tue Jun 26 21:39:37 2018 -0700
> > >
> > >     net/sched: add tunnel option support to act_tunnel_key
> > >
> > > I'm not sure if it's good to mix multiple types in this string. wdyt?
> > >
> > > but for the JSON data, of course, these are all numbers(not string).
> > >
> >
> > I don't understand why Stephen is pushing for hex; it does not make
> > sense for version, index or direction. I don't have a clear
> > understanding of hwid to know uint vs hex, so your current JSON prints
> > seem fine.
> >
> > As for the stdout print and hex fields, staring at the tc and lwtunnel
> > code, it seems like those 2 have a lot of parallels in expressing
> > options for encoding vs lwtunnel and netdev based code. ie., I think
> > this latest set is correct.
> >
> > Stephen?
>
> I just wanted:
> 1. The parse and print functions should have the same formats.
> I.e. if you take the output and do a little massaging of the ifindex
> it should be accepted as an input set of parameters.
>
> 2. As much as possible, the JSON and non-JSON output should be similar.
> If non-JSON prints in hex, then JSON should display hex and vice/versa.
>
> Ideally all inputs would be human format (not machine formats like hex).
> But I guess the mistake was already made with some of the other tunnels.
I guess we can't 'fix' these in other tunnels in tc.

So I'm thinking we can either use the latest patchset,
or keep the geneve opts format in lwtunnel consistent
with the geneve opts in tc only and parse all with
unint in the new erspan/vxlan tunnels opts.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Tue, Feb 18, 2020 at 12:29 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> On Tue, Feb 18, 2020 at 5:03 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Mon, 17 Feb 2020 12:53:14 -0700
> > David Ahern <dsahern@gmail.com> wrote:
> >
> > > On 2/15/20 11:38 PM, Xin Long wrote:
> > > > On Sun, Feb 16, 2020 at 12:51 AM David Ahern <dsahern@gmail.com> wrote:
> > > >>
> > > >> On 2/14/20 9:18 PM, Xin Long wrote:
> > > >>> On Sat, Feb 15, 2020 at 8:21 AM Stephen Hemminger
> > > >>> <stephen@networkplumber.org> wrote:
> > > >>>>
> > > >>>> On Sat, 15 Feb 2020 01:40:27 +0800
> > > >>>> Xin Long <lucien.xin@gmail.com> wrote:
> > > >>>>
> > > >>>>> This's not gonna work. as the output will be:
> > > >>>>> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> > > >>>>> instead of
> > > >>>>> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
> > > >>>>
> > > >>>> JSON is typeless. Lots of values are already printed in hex
> > > >>> You may mean JSON data itself is typeless.
> > > >>> But JSON objects are typed when parsing JSON data, which includes
> > > >>> string, number, array, boolean. So it matters how to define the
> > > >>> members' 'type' in JSON data.
> > > >>>
> > > >>> For example, in python's 'json' module:
> > > >>>
> > > >>> #!/usr/bin/python2
> > > >>> import json
> > > >>> json_data_1 = '{"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}'
> > > >>> json_data_2 = '{"ver":2,"index":0,"dir":1,"hwid":2}'
> > > >>> parsed_json_1 = (json.loads(json_data_1))
> > > >>> parsed_json_2 = (json.loads(json_data_2))
> > > >>> print type(parsed_json_1["hwid"])
> > > >>> print type(parsed_json_2["hwid"])
> > > >>>
> > > >>> The output is:
> > > >>> <type 'unicode'>
> > > >>> <type 'int'>
> > > >>>
> > > >>> Also, '{"result": true}' is different from '{"result": "true"}' when
> > > >>> loading it in a 3rd-party lib.
> > > >>>
> > > >>> I think the JSON data coming from iproute2 is designed to be used by
> > > >>> a 3rd-party lib to parse, not just to show to users. To keep these
> > > >>> members' original type (numbers) is more appropriate, IMO.
> > > >>>
> > > >>
> > > >> Stephen: why do you think all of the numbers should be in hex?
> > > >>
> > > >> It seems like consistency with existing output should matter more.
> > > >> ip/link_gre.c for instance prints index as an int, version as an int,
> > > >> direction as a string and only hwid in hex.
> > > >>
> > > >> Xin: any reason you did not follow the output of the existingg netdev
> > > >> based solutions?
> > > > Hi David,
> > > >
> > > > Option is expressed as "version:index:dir:hwid", I made all fields
> > > > in this string of hex, just like "class:type:data" in:
> > > >
> > > > commit 0ed5269f9e41f495c8e9020c85f5e1644c1afc57
> > > > Author: Simon Horman <simon.horman@netronome.com>
> > > > Date:   Tue Jun 26 21:39:37 2018 -0700
> > > >
> > > >     net/sched: add tunnel option support to act_tunnel_key
> > > >
> > > > I'm not sure if it's good to mix multiple types in this string. wdyt?
> > > >
> > > > but for the JSON data, of course, these are all numbers(not string).
> > > >
> > >
> > > I don't understand why Stephen is pushing for hex; it does not make
> > > sense for version, index or direction. I don't have a clear
> > > understanding of hwid to know uint vs hex, so your current JSON prints
> > > seem fine.
> > >
> > > As for the stdout print and hex fields, staring at the tc and lwtunnel
> > > code, it seems like those 2 have a lot of parallels in expressing
> > > options for encoding vs lwtunnel and netdev based code. ie., I think
> > > this latest set is correct.
> > >
> > > Stephen?
> >
> > I just wanted:
> > 1. The parse and print functions should have the same formats.
> > I.e. if you take the output and do a little massaging of the ifindex
> > it should be accepted as an input set of parameters.
> >
> > 2. As much as possible, the JSON and non-JSON output should be similar.
> > If non-JSON prints in hex, then JSON should display hex and vice/versa.
> >
> > Ideally all inputs would be human format (not machine formats like hex).
> > But I guess the mistake was already made with some of the other tunnels.
> I guess we can't 'fix' these in other tunnels in tc.
>
> So I'm thinking we can either use the latest patchset,
> or keep the geneve opts format in lwtunnel consistent
> with the geneve opts in tc only and parse all with
> unint in the new erspan/vxlan tunnels opts.
Hi, Stephen and David A.

This patchset is in "deferred" status for a long time.
What should we do about this one?
should I improve something then repost or the lastest one will be fine.

Thanks.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[David Ahern]

On 4/19/20 2:39 AM, Xin Long wrote:
> This patchset is in "deferred" status for a long time.
> What should we do about this one?
> should I improve something then repost or the lastest one will be fine.

I am fine with this set as is; Stephen had some concerns.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

Hi, Stephen,

any update?

This is your last reply:

"""
1. The parse and print functions should have the same formats.
I.e. if you take the output and do a little massaging of the ifindex
it should be accepted as an input set of parameters.

2. As much as possible, the JSON and non-JSON output should be similar.
If non-JSON prints in hex, then JSON should display hex and vice/versa.
"""

Do you still hope to change things like that (just note it won't be
consistent with:)

6217917a tc: m_tunnel_key: Add tunnel option support to act_tunnel_key
56155d4d tc: f_flower: add geneve option match support to flower

or go with the current set? I'm now okay to do either of them.

Thanks.

On Mon, Apr 20, 2020 at 6:28 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 4/19/20 2:39 AM, Xin Long wrote:
> > This patchset is in "deferred" status for a long time.
> > What should we do about this one?
> > should I improve something then repost or the lastest one will be fine.
>
> I am fine with this set as is; Stephen had some concerns.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[David Ahern]

On 4/19/20 4:28 PM, David Ahern wrote:
> On 4/19/20 2:39 AM, Xin Long wrote:
>> This patchset is in "deferred" status for a long time.
>> What should we do about this one?
>> should I improve something then repost or the lastest one will be fine.
> 
> I am fine with this set as is; Stephen had some concerns.
> 

Please re-send the patches.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Mon, Apr 27, 2020 at 2:29 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 4/19/20 4:28 PM, David Ahern wrote:
> > On 4/19/20 2:39 AM, Xin Long wrote:
> >> This patchset is in "deferred" status for a long time.
> >> What should we do about this one?
> >> should I improve something then repost or the lastest one will be fine.
> >
> > I am fine with this set as is; Stephen had some concerns.
> >
>
> Please re-send the patches.
OK, will post v4. thanks.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Stephen Hemminger]

On Sat, 15 Feb 2020 01:40:27 +0800
Xin Long <lucien.xin@gmail.com> wrote:

> On Sat, Feb 15, 2020 at 12:13 AM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Fri, 14 Feb 2020 18:30:47 +0800
> > Xin Long <lucien.xin@gmail.com> wrote:
> >  
> > > +
> > > +     open_json_array(PRINT_JSON, name);
> > > +     open_json_object(NULL);
> > > +     print_uint(PRINT_JSON, "ver", NULL, ver);
> > > +     print_uint(PRINT_JSON, "index", NULL, idx);
> > > +     print_uint(PRINT_JSON, "dir", NULL, dir);
> > > +     print_uint(PRINT_JSON, "hwid", NULL, hwid);
> > > +     close_json_object();
> > > +     close_json_array(PRINT_JSON, name);
> > > +
> > > +     print_nl();
> > > +     print_string(PRINT_FP, name, "\t%s ", name);
> > > +     sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
> > > +     print_string(PRINT_FP, NULL, "%s ", strbuf);
> > > +}  
> >
> > Instead of having two sets of prints, is it possible to do this
> >         print_nl();
> >         print_string(PRINT_FP, NULL, "\t", NULL);
> >
> >         open_json_array(PRINT_ANY, name);
> >         open_json_object(NULL);
> >         print_0xhex(PRINT_ANY, "ver", " %02x", ver);
> >         print_0xhex(PRINT_ANY, "idx", ":%08x", idx);
> >         print_0xhex(PRINT_ANY, "dir", ":%02x", dir);
> >         print_0xhex(PRINT_ANY, "hwid", ":%02x", hwid)
> >         close_json_object();
> >         close_json_array(PRINT_ANY, " ");  
> Hi Stephen,
> 
> This's not gonna work. as the output will be:
> {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> instead of
> {"ver":2,"index":0,"dir":1,"hwid":2} (number)
> 
> >
> > Also, you seem to not hear the request to not use opaque hex values
> > in the iproute2 interface. The version, index, etc should be distinct
> > parameter values not a hex string.  
> The opts STRING, especially these like "XX:YY:ZZ" are represented
> as hex string on both adding and dumping. It is to keep consistent with
> geneve_opts in m_tunnel_key and f_flower,  see

There are several different requests.

 1. The format of the output must match the input.
 2. Printing values in hex would be nice if they are bit fields
 3. If non json uses hex, then json should use hex
    json is type less so { "ver":2 } and { "ver":"0x2" } are the same

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Jakub Kicinski]

On Thu, 23 Apr 2020 08:23:51 -0700 Stephen Hemminger wrote:
>  3. If non json uses hex, then json should use hex
>     json is type less so { "ver":2 } and { "ver":"0x2" } are the same

I may be missing something or misunderstanding you, but in my humble
experience that's emphatically not true:

$ echo '{ "a" : 2 }' | python -c 'import sys, json; print(json.load(sys.stdin)["a"] + 1)'
3
$ echo '{ "a" : "2" }' | python -c 'import sys, json; print(json.load(sys.stdin)["a"] + 1)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: can only concatenate str (not "int") to str

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[David Ahern]

On 4/23/20 12:03 PM, Jakub Kicinski wrote:
> On Thu, 23 Apr 2020 08:23:51 -0700 Stephen Hemminger wrote:
>>  3. If non json uses hex, then json should use hex
>>     json is type less so { "ver":2 } and { "ver":"0x2" } are the same
> 
> I may be missing something or misunderstanding you, but in my humble
> experience that's emphatically not true:
> 
> $ echo '{ "a" : 2 }' | python -c 'import sys, json; print(json.load(sys.stdin)["a"] + 1)'
> 3
> $ echo '{ "a" : "2" }' | python -c 'import sys, json; print(json.load(sys.stdin)["a"] + 1)'
> Traceback (most recent call last):
>   File "<string>", line 1, in <module>
> TypeError: can only concatenate str (not "int") to str
> 

I don't know which site is the definitive source for json, but several
do state json has several types - strings, number, true / false / null,
object, array.

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Stephen Hemminger]

On Mon, 27 Apr 2020 06:38:03 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 4/23/20 12:03 PM, Jakub Kicinski wrote:
> > On Thu, 23 Apr 2020 08:23:51 -0700 Stephen Hemminger wrote:  
> >>  3. If non json uses hex, then json should use hex
> >>     json is type less so { "ver":2 } and { "ver":"0x2" } are the same  
> > 
> > I may be missing something or misunderstanding you, but in my humble
> > experience that's emphatically not true:
> > 
> > $ echo '{ "a" : 2 }' | python -c 'import sys, json; print(json.load(sys.stdin)["a"] + 1)'
> > 3
> > $ echo '{ "a" : "2" }' | python -c 'import sys, json; print(json.load(sys.stdin)["a"] + 1)'
> > Traceback (most recent call last):
> >   File "<string>", line 1, in <module>
> > TypeError: can only concatenate str (not "int") to str
> >   
> 
> I don't know which site is the definitive source for json, but several
> do state json has several types - strings, number, true / false / null,
> object, array.

Probably I got confused because Python tries to be helpful...
JSON is ossified/standardized http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf

Best answer on the web was https://stackoverflow.com/questions/52671719/can-hex-format-be-used-with-json-files-if-so-how

   "JSON does not support hexadecimal numbers but they are supported in JSON5. json5.org"

Re: [PATCHv3 iproute2-next 3/7] iproute_lwtunnel: add options support for erspan metadata

[Xin Long]

On Thu, Apr 23, 2020 at 11:24 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sat, 15 Feb 2020 01:40:27 +0800
> Xin Long <lucien.xin@gmail.com> wrote:
>
> > On Sat, Feb 15, 2020 at 12:13 AM Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > >
> > > On Fri, 14 Feb 2020 18:30:47 +0800
> > > Xin Long <lucien.xin@gmail.com> wrote:
> > >
> > > > +
> > > > +     open_json_array(PRINT_JSON, name);
> > > > +     open_json_object(NULL);
> > > > +     print_uint(PRINT_JSON, "ver", NULL, ver);
> > > > +     print_uint(PRINT_JSON, "index", NULL, idx);
> > > > +     print_uint(PRINT_JSON, "dir", NULL, dir);
> > > > +     print_uint(PRINT_JSON, "hwid", NULL, hwid);
> > > > +     close_json_object();
> > > > +     close_json_array(PRINT_JSON, name);
> > > > +
> > > > +     print_nl();
> > > > +     print_string(PRINT_FP, name, "\t%s ", name);
> > > > +     sprintf(strbuf, "%02x:%08x:%02x:%02x", ver, idx, dir, hwid);
> > > > +     print_string(PRINT_FP, NULL, "%s ", strbuf);
> > > > +}
> > >
> > > Instead of having two sets of prints, is it possible to do this
> > >         print_nl();
> > >         print_string(PRINT_FP, NULL, "\t", NULL);
> > >
> > >         open_json_array(PRINT_ANY, name);
> > >         open_json_object(NULL);
> > >         print_0xhex(PRINT_ANY, "ver", " %02x", ver);
> > >         print_0xhex(PRINT_ANY, "idx", ":%08x", idx);
> > >         print_0xhex(PRINT_ANY, "dir", ":%02x", dir);
> > >         print_0xhex(PRINT_ANY, "hwid", ":%02x", hwid)
> > >         close_json_object();
> > >         close_json_array(PRINT_ANY, " ");
> > Hi Stephen,
> >
> > This's not gonna work. as the output will be:
> > {"ver":"0x2","idx":"0","dir":"0x1","hwid":"0x2"}  (string)
> > instead of
> > {"ver":2,"index":0,"dir":1,"hwid":2} (number)
> >
> > >
> > > Also, you seem to not hear the request to not use opaque hex values
> > > in the iproute2 interface. The version, index, etc should be distinct
> > > parameter values not a hex string.
> > The opts STRING, especially these like "XX:YY:ZZ" are represented
> > as hex string on both adding and dumping. It is to keep consistent with
> > geneve_opts in m_tunnel_key and f_flower,  see
>
> There are several different requests.
>
>  1. The format of the output must match the input.
>  2. Printing values in hex would be nice if they are bit fields
>  3. If non json uses hex, then json should use hex
>     json is type less so { "ver":2 } and { "ver":"0x2" } are the same
>
V4 has been posted, with respect to these 3 rules.
And all numbers are in uint type, no problems about { "ver":2 } and {
"ver":"0x2" } things.

Thanks.