[PATCH nf-next 0/7] netfilter: nft_tunnel: reinforce key opts support

[PATCH nf-next 0/7] netfilter: nft_tunnel: reinforce key opts support

[Xin Long]

This patchset improves quite a few places to make vxlan/erspan
opts in nft_tunnel work with userspace nftables/libnftnl, and
also keep consistent with the support for vxlan/erspan opts in
act_tunnel_key, cls_flower and ip_tunnel_core.

Meanwhile, add support for geneve opts in nft_tunnel. One patch
for nftables and one for libnftnl will be posted here for the
testing. With them, nft_tunnel can be set and used by:

  # nft add table ip filter
  # nft add chain ip filter input { type filter hook input priority 0 \; }
  # nft add tunnel filter vxlan_01 { type vxlan\; id 2\; \
    ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
    sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
    opts \"ffff\"\; }
  # nft add tunnel filter erspan_01 { type erspan\; id 2\; \
    ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
    sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
    opts \"1:1:0:0\"\; }
  # nft add tunnel filter erspan_02 { type erspan\; id 2\; \
    ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
    sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
    opts \"2:0:1:1\"\; }
  # nft add tunnel filter geneve_01 { type geneve\; id 2\; \
    ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
    sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
    opts \"1:1:1212121234567890\"\; }
  # nft add tunnel filter geneve_02 { type geneve\; id 2\; \
    ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
    sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
    opts \"1:1:34567890,2:2:12121212,3:3:1212121234567890\"\; }
  # nft list tunnels table filter
  # nft add rule filter input ip protocol udp tunnel name geneve_02
  # nft add rule filter input meta l4proto udp tunnel id 2 drop
  # nft add rule filter input meta l4proto udp tunnel path 0 drop
  # nft list chain filter input -a

Xin Long (7):
  netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8
  netfilter: nft_tunnel: parse VXLAN_GBP attr as u32 in nft_tunnel
  netfilter: nft_tunnel: no need to call htons() when dumping ports
  netfilter: nft_tunnel: also dump ERSPAN_VERSION
  netfilter: nft_tunnel: also dump OPTS_ERSPAN/VXLAN
  netfilter: nft_tunnel: add the missing nla_nest_cancel()
  netfilter: nft_tunnel: add support for geneve opts

 include/uapi/linux/netfilter/nf_tables.h |  10 ++
 net/netfilter/nft_tunnel.c               | 170 +++++++++++++++++++++++++------
 2 files changed, 151 insertions(+), 29 deletions(-)

-- 
2.1.0

[PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Xin Long]

To keep consistent with ipgre_policy, it's better to parse
ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
cls_flower and ip_tunnel_core.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nft_tunnel.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index 3d4c2ae..f76cd7d 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
 }
 
 static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
+	[NFTA_TUNNEL_KEY_ERSPAN_VERSION]	= { .type = NLA_U8 },
 	[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]	= { .type = NLA_U32 },
-	[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]	= { .type = NLA_U8 },
+	[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]		= { .type = NLA_U8 },
 	[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]	= { .type = NLA_U8 },
 };
 
@@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
 	if (err < 0)
 		return err;
 
-	version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
+	version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
 	switch (version) {
 	case ERSPAN_VERSION:
 		if (!tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX])
-- 
2.1.0

[PATCH nf-next 2/7] netfilter: nft_tunnel: parse VXLAN_GBP attr as u32 in nft_tunnel

[Xin Long]

Both user and kernel sides want VXLAN_GBP opt as u32, so there's no
need to convert it on each side.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nft_tunnel.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index f76cd7d..d9d6c0d 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -239,7 +239,7 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
 	if (!tb[NFTA_TUNNEL_KEY_VXLAN_GBP])
 		return -EINVAL;
 
-	opts->u.vxlan.gbp = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]));
+	opts->u.vxlan.gbp = nla_get_u32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]);
 
 	opts->len	= sizeof(struct vxlan_metadata);
 	opts->flags	= TUNNEL_VXLAN_OPT;
@@ -475,8 +475,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 		return -1;
 
 	if (opts->flags & TUNNEL_VXLAN_OPT) {
-		if (nla_put_be32(skb, NFTA_TUNNEL_KEY_VXLAN_GBP,
-				 htonl(opts->u.vxlan.gbp)))
+		if (nla_put_u32(skb, NFTA_TUNNEL_KEY_VXLAN_GBP,
+				opts->u.vxlan.gbp))
 			return -1;
 	} else if (opts->flags & TUNNEL_ERSPAN_OPT) {
 		switch (opts->u.erspan.version) {
-- 
2.1.0

[PATCH nf-next 3/7] netfilter: nft_tunnel: no need to call htons() when dumping ports

[Xin Long]

info->key.tp_src and tp_dst are __be16, when using nla_put_be16()
to dump them, htons() is not needed, so remove it in this patch.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nft_tunnel.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index d9d6c0d..e1184fa 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 static int nft_tunnel_ports_dump(struct sk_buff *skb,
 				 struct ip_tunnel_info *info)
 {
-	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 ||
-	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0)
+	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 ||
+	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0)
 		return -1;
 
 	return 0;
-- 
2.1.0

[PATCH nf-next 4/7] netfilter: nft_tunnel: also dump ERSPAN_VERSION

[Xin Long]

This is not necessary, but it'll be easier to parse in userspace,
also given that other places like act_tunnel_key, cls_flower and
ip_tunnel_core are also doing so.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nft_tunnel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index e1184fa..576437f 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -479,6 +479,9 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 				opts->u.vxlan.gbp))
 			return -1;
 	} else if (opts->flags & TUNNEL_ERSPAN_OPT) {
+		if (nla_put_u8(skb, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
+			       opts->u.erspan.version))
+			return -1;
 		switch (opts->u.erspan.version) {
 		case ERSPAN_VERSION:
 			if (nla_put_be32(skb, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
-- 
2.1.0

[PATCH nf-next 5/7] netfilter: nft_tunnel: also dump OPTS_ERSPAN/VXLAN

[Xin Long]

This patch is to add the nest attr OPTS_ERSPAN/VXLAN when dumping
KEY_OPTS, and it would be helpful when parsing in userpace. Also,
this is needed for supporting multiple geneve opts in the future
patches.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nft_tunnel.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index 576437f..e9b94b8 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -468,17 +468,24 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 				struct nft_tunnel_obj *priv)
 {
 	struct nft_tunnel_opts *opts = &priv->opts;
-	struct nlattr *nest;
+	struct nlattr *nest, *inner;
 
 	nest = nla_nest_start_noflag(skb, NFTA_TUNNEL_KEY_OPTS);
 	if (!nest)
 		return -1;
 
 	if (opts->flags & TUNNEL_VXLAN_OPT) {
+		inner = nla_nest_start_noflag(skb, NFTA_TUNNEL_KEY_OPTS_VXLAN);
+		if (!inner)
+			return -1;
 		if (nla_put_u32(skb, NFTA_TUNNEL_KEY_VXLAN_GBP,
 				opts->u.vxlan.gbp))
 			return -1;
+		nla_nest_end(skb, inner);
 	} else if (opts->flags & TUNNEL_ERSPAN_OPT) {
+		inner = nla_nest_start_noflag(skb, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
+		if (!inner)
+			return -1;
 		if (nla_put_u8(skb, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
 			       opts->u.erspan.version))
 			return -1;
@@ -496,6 +503,7 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 				return -1;
 			break;
 		}
+		nla_nest_end(skb, inner);
 	}
 	nla_nest_end(skb, nest);
 
-- 
2.1.0

[PATCH nf-next 6/7] netfilter: nft_tunnel: add the missing nla_nest_cancel()

[Xin Long]

When nla_put_xxx() fails under nla_nest_start_noflag(),
nla_nest_cancel() should be called, so that the skb can
be trimmed properly.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/netfilter/nft_tunnel.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index e9b94b8..32263dc 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -443,10 +443,15 @@ static int nft_tunnel_ip_dump(struct sk_buff *skb, struct ip_tunnel_info *info)
 		if (!nest)
 			return -1;
 
-		if (nla_put_in6_addr(skb, NFTA_TUNNEL_KEY_IP6_SRC, &info->key.u.ipv6.src) < 0 ||
-		    nla_put_in6_addr(skb, NFTA_TUNNEL_KEY_IP6_DST, &info->key.u.ipv6.dst) < 0 ||
-		    nla_put_be32(skb, NFTA_TUNNEL_KEY_IP6_FLOWLABEL, info->key.label))
+		if (nla_put_in6_addr(skb, NFTA_TUNNEL_KEY_IP6_SRC,
+				     &info->key.u.ipv6.src) < 0 ||
+		    nla_put_in6_addr(skb, NFTA_TUNNEL_KEY_IP6_DST,
+				     &info->key.u.ipv6.dst) < 0 ||
+		    nla_put_be32(skb, NFTA_TUNNEL_KEY_IP6_FLOWLABEL,
+				 info->key.label)) {
+			nla_nest_cancel(skb, nest);
 			return -1;
+		}
 
 		nla_nest_end(skb, nest);
 	} else {
@@ -454,9 +459,13 @@ static int nft_tunnel_ip_dump(struct sk_buff *skb, struct ip_tunnel_info *info)
 		if (!nest)
 			return -1;
 
-		if (nla_put_in_addr(skb, NFTA_TUNNEL_KEY_IP_SRC, info->key.u.ipv4.src) < 0 ||
-		    nla_put_in_addr(skb, NFTA_TUNNEL_KEY_IP_DST, info->key.u.ipv4.dst) < 0)
+		if (nla_put_in_addr(skb, NFTA_TUNNEL_KEY_IP_SRC,
+				    info->key.u.ipv4.src) < 0 ||
+		    nla_put_in_addr(skb, NFTA_TUNNEL_KEY_IP_DST,
+				    info->key.u.ipv4.dst) < 0) {
+			nla_nest_cancel(skb, nest);
 			return -1;
+		}
 
 		nla_nest_end(skb, nest);
 	}
@@ -477,37 +486,42 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 	if (opts->flags & TUNNEL_VXLAN_OPT) {
 		inner = nla_nest_start_noflag(skb, NFTA_TUNNEL_KEY_OPTS_VXLAN);
 		if (!inner)
-			return -1;
+			goto failure;
 		if (nla_put_u32(skb, NFTA_TUNNEL_KEY_VXLAN_GBP,
 				opts->u.vxlan.gbp))
-			return -1;
+			goto inner_failure;
 		nla_nest_end(skb, inner);
 	} else if (opts->flags & TUNNEL_ERSPAN_OPT) {
 		inner = nla_nest_start_noflag(skb, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
 		if (!inner)
-			return -1;
+			goto failure;
 		if (nla_put_u8(skb, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
 			       opts->u.erspan.version))
-			return -1;
+			goto inner_failure;
 		switch (opts->u.erspan.version) {
 		case ERSPAN_VERSION:
 			if (nla_put_be32(skb, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
 					 opts->u.erspan.u.index))
-				return -1;
+				goto inner_failure;
 			break;
 		case ERSPAN_VERSION2:
 			if (nla_put_u8(skb, NFTA_TUNNEL_KEY_ERSPAN_V2_HWID,
 				       get_hwid(&opts->u.erspan.u.md2)) ||
 			    nla_put_u8(skb, NFTA_TUNNEL_KEY_ERSPAN_V2_DIR,
 				       opts->u.erspan.u.md2.dir))
-				return -1;
+				goto inner_failure;
 			break;
 		}
 		nla_nest_end(skb, inner);
 	}
 	nla_nest_end(skb, nest);
-
 	return 0;
+
+inner_failure:
+	nla_nest_cancel(skb, inner);
+failure:
+	nla_nest_cancel(skb, nest);
+	return -1;
 }
 
 static int nft_tunnel_ports_dump(struct sk_buff *skb,
-- 
2.1.0

[PATCH nf-next 7/7] netfilter: nft_tunnel: add support for geneve opts

[Xin Long]

Like vxlan and erspan opts, geneve opts should also be supported in
nft_tunnel. The difference is geneve RFC (draft-ietf-nvo3-geneve-14)
allows a geneve packet to carry multiple geneve opts. So with this
patch, nftables/libnftnl would do:

  # nft add table ip filter
  # nft add chain ip filter input { type filter hook input priority 0 \; }
  # nft add tunnel filter geneve_02 { type geneve\; id 2\; \
    ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
    sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
    opts \"1:1:34567890,2:2:12121212,3:3:1212121234567890\"\; }
  # nft list tunnels table filter
    table ip filter {
    	tunnel geneve_02 {
    		id 2
    		ip saddr 192.168.1.1
    		ip daddr 192.168.1.2
    		sport 9000
    		dport 9001
    		tos 18
    		ttl 64
    		flags 1
    		geneve opts 1:1:34567890,2:2:12121212,3:3:1212121234567890
    	}
    }

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/netfilter/nf_tables.h |  10 +++
 net/netfilter/nft_tunnel.c               | 110 +++++++++++++++++++++++++++----
 2 files changed, 108 insertions(+), 12 deletions(-)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index bb9b049..f74b957 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1729,6 +1729,7 @@ enum nft_tunnel_opts_attributes {
 	NFTA_TUNNEL_KEY_OPTS_UNSPEC,
 	NFTA_TUNNEL_KEY_OPTS_VXLAN,
 	NFTA_TUNNEL_KEY_OPTS_ERSPAN,
+	NFTA_TUNNEL_KEY_OPTS_GENEVE,
 	__NFTA_TUNNEL_KEY_OPTS_MAX
 };
 #define NFTA_TUNNEL_KEY_OPTS_MAX	(__NFTA_TUNNEL_KEY_OPTS_MAX - 1)
@@ -1750,6 +1751,15 @@ enum nft_tunnel_opts_erspan_attributes {
 };
 #define NFTA_TUNNEL_KEY_ERSPAN_MAX	(__NFTA_TUNNEL_KEY_ERSPAN_MAX - 1)
 
+enum nft_tunnel_opts_geneve_attributes {
+	NFTA_TUNNEL_KEY_GENEVE_UNSPEC,
+	NFTA_TUNNEL_KEY_GENEVE_CLASS,
+	NFTA_TUNNEL_KEY_GENEVE_TYPE,
+	NFTA_TUNNEL_KEY_GENEVE_DATA,
+	__NFTA_TUNNEL_KEY_GENEVE_MAX
+};
+#define NFTA_TUNNEL_KEY_GENEVE_MAX	(__NFTA_TUNNEL_KEY_GENEVE_MAX - 1)
+
 enum nft_tunnel_flags {
 	NFT_TUNNEL_F_ZERO_CSUM_TX	= (1 << 0),
 	NFT_TUNNEL_F_DONT_FRAGMENT	= (1 << 1),
diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index 32263dc..f621d2b2 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -11,6 +11,7 @@
 #include <net/ip_tunnels.h>
 #include <net/vxlan.h>
 #include <net/erspan.h>
+#include <net/geneve.h>
 
 struct nft_tunnel {
 	enum nft_tunnel_keys	key:8;
@@ -144,6 +145,7 @@ struct nft_tunnel_opts {
 	union {
 		struct vxlan_metadata	vxlan;
 		struct erspan_metadata	erspan;
+		u8	data[IP_TUNNEL_OPTS_MAX];
 	} u;
 	u32	len;
 	__be16	flags;
@@ -298,9 +300,53 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
 	return 0;
 }
 
+static const struct nla_policy nft_tunnel_opts_geneve_policy[NFTA_TUNNEL_KEY_GENEVE_MAX + 1] = {
+	[NFTA_TUNNEL_KEY_GENEVE_CLASS]	= { .type = NLA_U16 },
+	[NFTA_TUNNEL_KEY_GENEVE_TYPE]	= { .type = NLA_U8 },
+	[NFTA_TUNNEL_KEY_GENEVE_DATA]	= { .type = NLA_BINARY, .len = 128 },
+};
+
+static int nft_tunnel_obj_geneve_init(const struct nlattr *attr,
+				      struct nft_tunnel_opts *opts)
+{
+	struct geneve_opt *opt = (struct geneve_opt *)opts->u.data + opts->len;
+	struct nlattr *tb[NFTA_TUNNEL_KEY_GENEVE_MAX + 1];
+	int err, data_len;
+
+	err = nla_parse_nested(tb, NFTA_TUNNEL_KEY_GENEVE_MAX, attr,
+			       nft_tunnel_opts_geneve_policy, NULL);
+	if (err < 0)
+		return err;
+
+	if (!tb[NFTA_TUNNEL_KEY_GENEVE_CLASS] ||
+	    !tb[NFTA_TUNNEL_KEY_GENEVE_TYPE] ||
+	    !tb[NFTA_TUNNEL_KEY_GENEVE_DATA])
+		return -EINVAL;
+
+	attr = tb[NFTA_TUNNEL_KEY_GENEVE_DATA];
+	data_len = nla_len(attr);
+	if (data_len % 4)
+		return -EINVAL;
+
+	opts->len += sizeof(*opt) + data_len;
+	if (opts->len > IP_TUNNEL_OPTS_MAX)
+		return -EINVAL;
+
+	memcpy(opt->opt_data, nla_data(attr), data_len);
+	opt->length = data_len / 4;
+	opt->opt_class = nla_get_be16(tb[NFTA_TUNNEL_KEY_GENEVE_CLASS]);
+	opt->type = nla_get_u8(tb[NFTA_TUNNEL_KEY_GENEVE_TYPE]);
+	opts->flags = TUNNEL_GENEVE_OPT;
+
+	return 0;
+}
+
 static const struct nla_policy nft_tunnel_opts_policy[NFTA_TUNNEL_KEY_OPTS_MAX + 1] = {
+	[NFTA_TUNNEL_KEY_OPTS_UNSPEC]	= {
+		.strict_start_type = NFTA_TUNNEL_KEY_OPTS_GENEVE },
 	[NFTA_TUNNEL_KEY_OPTS_VXLAN]	= { .type = NLA_NESTED, },
 	[NFTA_TUNNEL_KEY_OPTS_ERSPAN]	= { .type = NLA_NESTED, },
+	[NFTA_TUNNEL_KEY_OPTS_GENEVE]	= { .type = NLA_NESTED, },
 };
 
 static int nft_tunnel_obj_opts_init(const struct nft_ctx *ctx,
@@ -308,22 +354,43 @@ static int nft_tunnel_obj_opts_init(const struct nft_ctx *ctx,
 				    struct ip_tunnel_info *info,
 				    struct nft_tunnel_opts *opts)
 {
-	struct nlattr *tb[NFTA_TUNNEL_KEY_OPTS_MAX + 1];
-	int err;
+	int err, rem, type = 0;
+	struct nlattr *nla;
 
-	err = nla_parse_nested_deprecated(tb, NFTA_TUNNEL_KEY_OPTS_MAX, attr,
-					  nft_tunnel_opts_policy, NULL);
+	err = nla_validate_nested_deprecated(attr, NFTA_TUNNEL_KEY_OPTS_MAX,
+					     nft_tunnel_opts_policy, NULL);
 	if (err < 0)
 		return err;
 
-	if (tb[NFTA_TUNNEL_KEY_OPTS_VXLAN]) {
-		err = nft_tunnel_obj_vxlan_init(tb[NFTA_TUNNEL_KEY_OPTS_VXLAN],
-						opts);
-	} else if (tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]) {
-		err = nft_tunnel_obj_erspan_init(tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN],
-						 opts);
-	} else {
-		return -EOPNOTSUPP;
+	nla_for_each_attr(nla, nla_data(attr), nla_len(attr), rem) {
+		switch (nla_type(nla)) {
+		case NFTA_TUNNEL_KEY_OPTS_VXLAN:
+			if (type)
+				return -EINVAL;
+			err = nft_tunnel_obj_vxlan_init(nla, opts);
+			if (err)
+				return err;
+			type = TUNNEL_VXLAN_OPT;
+			break;
+		case NFTA_TUNNEL_KEY_OPTS_ERSPAN:
+			if (type)
+				return -EINVAL;
+			err = nft_tunnel_obj_erspan_init(nla, opts);
+			if (err)
+				return err;
+			type = TUNNEL_ERSPAN_OPT;
+			break;
+		case NFTA_TUNNEL_KEY_OPTS_GENEVE:
+			if (type && type != TUNNEL_GENEVE_OPT)
+				return -EINVAL;
+			err = nft_tunnel_obj_geneve_init(nla, opts);
+			if (err)
+				return err;
+			type = TUNNEL_GENEVE_OPT;
+			break;
+		default:
+			return -EOPNOTSUPP;
+		}
 	}
 
 	return err;
@@ -513,6 +580,25 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
 			break;
 		}
 		nla_nest_end(skb, inner);
+	} else if (opts->flags & TUNNEL_GENEVE_OPT) {
+		struct geneve_opt *opt;
+		int offset = 0;
+
+		inner = nla_nest_start_noflag(skb, NFTA_TUNNEL_KEY_OPTS_GENEVE);
+		if (!inner)
+			goto failure;
+		while (opts->len > offset) {
+			opt = (struct geneve_opt *)opts->u.data + offset;
+			if (nla_put_be16(skb, NFTA_TUNNEL_KEY_GENEVE_CLASS,
+					 opt->opt_class) ||
+			    nla_put_u8(skb, NFTA_TUNNEL_KEY_GENEVE_TYPE,
+				       opt->type) ||
+			    nla_put(skb, NFTA_TUNNEL_KEY_GENEVE_DATA,
+				    opt->length * 4, opt->opt_data))
+				goto inner_failure;
+			offset += sizeof(*opt) + opt->length * 4;
+		}
+		nla_nest_end(skb, inner);
 	}
 	nla_nest_end(skb, nest);
 	return 0;
-- 
2.1.0

Re: [PATCH nf-next 0/7] netfilter: nft_tunnel: reinforce key opts support

[Xin Long]

[-- Attachment #1: Type: text/plain, Size: 3060 bytes --]

The 1st patch file for libnftnl is
     Based on:
       git://git.netfilter.org/libnftnl
The 2nd patch file for nftables is:
    Based on:
      git://git.netfilter.org/nftables: d42e9a1 (tag: v0.9.2) build:
Bump version to v0.9.2
      plusing:
        https://patchwork.ozlabs.org/patch/1136755/
        https://patchwork.ozlabs.org/patch/1136756/
    Dependencies:
       Some fixes in libnftnl, which are in another patch.

On Sun, Dec 8, 2019 at 12:41 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> This patchset improves quite a few places to make vxlan/erspan
> opts in nft_tunnel work with userspace nftables/libnftnl, and
> also keep consistent with the support for vxlan/erspan opts in
> act_tunnel_key, cls_flower and ip_tunnel_core.
>
> Meanwhile, add support for geneve opts in nft_tunnel. One patch
> for nftables and one for libnftnl will be posted here for the
> testing. With them, nft_tunnel can be set and used by:
>
>   # nft add table ip filter
>   # nft add chain ip filter input { type filter hook input priority 0 \; }
>   # nft add tunnel filter vxlan_01 { type vxlan\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"ffff\"\; }
>   # nft add tunnel filter erspan_01 { type erspan\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"1:1:0:0\"\; }
>   # nft add tunnel filter erspan_02 { type erspan\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"2:0:1:1\"\; }
>   # nft add tunnel filter geneve_01 { type geneve\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"1:1:1212121234567890\"\; }
>   # nft add tunnel filter geneve_02 { type geneve\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"1:1:34567890,2:2:12121212,3:3:1212121234567890\"\; }
>   # nft list tunnels table filter
>   # nft add rule filter input ip protocol udp tunnel name geneve_02
>   # nft add rule filter input meta l4proto udp tunnel id 2 drop
>   # nft add rule filter input meta l4proto udp tunnel path 0 drop
>   # nft list chain filter input -a
>
> Xin Long (7):
>   netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8
>   netfilter: nft_tunnel: parse VXLAN_GBP attr as u32 in nft_tunnel
>   netfilter: nft_tunnel: no need to call htons() when dumping ports
>   netfilter: nft_tunnel: also dump ERSPAN_VERSION
>   netfilter: nft_tunnel: also dump OPTS_ERSPAN/VXLAN
>   netfilter: nft_tunnel: add the missing nla_nest_cancel()
>   netfilter: nft_tunnel: add support for geneve opts
>
>  include/uapi/linux/netfilter/nf_tables.h |  10 ++
>  net/netfilter/nft_tunnel.c               | 170 +++++++++++++++++++++++++------
>  2 files changed, 151 insertions(+), 29 deletions(-)
>
> --
> 2.1.0
>

[-- Attachment #2: 0001-libnftnl-nft_tunnel-add-support-for-parsing-geneve-o.patch --]
[-- Type: application/octet-stream, Size: 11334 bytes --]

From 2456ad53c66489d6f2d4aac7d95216d553f94b83 Mon Sep 17 00:00:00 2001
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 6 Dec 2019 04:31:10 -0500
Subject: [PATCH] libnftnl: nft_tunnel: add support for parsing geneve opts

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/libnftnl/object.h           |   3 +
 include/linux/netfilter/nf_tables.h |  10 +++
 include/obj.h                       |  13 +++-
 src/libnftnl.map                    |   1 +
 src/obj/tunnel.c                    | 109 ++++++++++++++++++++++++++--
 src/object.c                        |   4 +-
 6 files changed, 129 insertions(+), 11 deletions(-)

diff --git a/include/libnftnl/object.h b/include/libnftnl/object.h
index 221b15c..e17b8d5 100644
--- a/include/libnftnl/object.h
+++ b/include/libnftnl/object.h
@@ -109,6 +109,9 @@ enum {
 	NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX,
 	NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID,
 	NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR,
+	NFTNL_OBJ_TUNNEL_GENEVE_CLASS,
+	NFTNL_OBJ_TUNNEL_GENEVE_TYPE,
+	NFTNL_OBJ_TUNNEL_GENEVE_DATA,
 };
 
 enum {
diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index bb9b049..f74b957 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -1729,6 +1729,7 @@ enum nft_tunnel_opts_attributes {
 	NFTA_TUNNEL_KEY_OPTS_UNSPEC,
 	NFTA_TUNNEL_KEY_OPTS_VXLAN,
 	NFTA_TUNNEL_KEY_OPTS_ERSPAN,
+	NFTA_TUNNEL_KEY_OPTS_GENEVE,
 	__NFTA_TUNNEL_KEY_OPTS_MAX
 };
 #define NFTA_TUNNEL_KEY_OPTS_MAX	(__NFTA_TUNNEL_KEY_OPTS_MAX - 1)
@@ -1750,6 +1751,15 @@ enum nft_tunnel_opts_erspan_attributes {
 };
 #define NFTA_TUNNEL_KEY_ERSPAN_MAX	(__NFTA_TUNNEL_KEY_ERSPAN_MAX - 1)
 
+enum nft_tunnel_opts_geneve_attributes {
+	NFTA_TUNNEL_KEY_GENEVE_UNSPEC,
+	NFTA_TUNNEL_KEY_GENEVE_CLASS,
+	NFTA_TUNNEL_KEY_GENEVE_TYPE,
+	NFTA_TUNNEL_KEY_GENEVE_DATA,
+	__NFTA_TUNNEL_KEY_GENEVE_MAX
+};
+#define NFTA_TUNNEL_KEY_GENEVE_MAX	(__NFTA_TUNNEL_KEY_GENEVE_MAX - 1)
+
 enum nft_tunnel_flags {
 	NFT_TUNNEL_F_ZERO_CSUM_TX	= (1 << 0),
 	NFT_TUNNEL_F_DONT_FRAGMENT	= (1 << 1),
diff --git a/include/obj.h b/include/obj.h
index 10f806c..eb77445 100644
--- a/include/obj.h
+++ b/include/obj.h
@@ -19,7 +19,7 @@ struct nftnl_obj {
 	uint32_t		family;
 	uint32_t		use;
 
-	uint32_t		flags;
+	uint64_t		flags;
 	uint64_t		handle;
 
 	union {
@@ -78,7 +78,7 @@ struct nftnl_obj {
 					uint32_t	gbp;
 				} tun_vxlan;
 				struct {
-					uint32_t	version;
+					uint8_t	version;
 					union {
 						uint32_t	v1_index;
 						struct {
@@ -87,6 +87,15 @@ struct nftnl_obj {
 						} v2;
 					} u;
 				} tun_erspan;
+				struct {
+					struct geneve_opt {
+						uint16_t	class;
+						uint8_t		type;
+						uint8_t		*data;
+						uint8_t		data_len;
+					}	opts[64];
+					uint8_t		opt_num;
+				} tun_geneve;
 			} u;
 		} tunnel;
 		struct nftnl_obj_secmark {
diff --git a/src/libnftnl.map b/src/libnftnl.map
index 8230d15..3b1b891 100644
--- a/src/libnftnl.map
+++ b/src/libnftnl.map
@@ -357,5 +357,6 @@ LIBNFTNL_12 {
 LIBNFTNL_13 {
   nftnl_set_list_lookup_byname;
   nftnl_obj_set_data;
+  nftnl_obj_get_data;
   nftnl_flowtable_set_data;
 } LIBNFTNL_12;
diff --git a/src/obj/tunnel.c b/src/obj/tunnel.c
index 100aa09..120ec3d 100644
--- a/src/obj/tunnel.c
+++ b/src/obj/tunnel.c
@@ -76,6 +76,22 @@ nftnl_obj_tunnel_set(struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
 		memcpy(&tun->u.tun_erspan.u.v2.dir, data, sizeof(tun->u.tun_erspan.u.v2.dir));
 		break;
+	case NFTNL_OBJ_TUNNEL_GENEVE_CLASS:
+		tun->u.tun_geneve.opt_num++;
+		memcpy(&tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].class, data, sizeof(uint16_t));
+		break;
+	case NFTNL_OBJ_TUNNEL_GENEVE_TYPE:
+		memcpy(&tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].type, data, sizeof(uint8_t));
+		break;
+	case NFTNL_OBJ_TUNNEL_GENEVE_DATA:
+		{
+			uint8_t *opt_data = malloc(data_len);
+
+			memcpy(opt_data, data, data_len);
+			tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].data = opt_data;
+			tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].data_len = data_len;
+			break;
+		}
 	default:
 		return -1;
 	}
@@ -137,6 +153,27 @@ nftnl_obj_tunnel_get(const struct nftnl_obj *e, uint16_t type,
 	case NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR:
 		*data_len = sizeof(tun->u.tun_erspan.u.v2.dir);
 		return &tun->u.tun_erspan.u.v2.dir;
+	case NFTNL_OBJ_TUNNEL_GENEVE_CLASS:
+		{
+			int pos = *data_len;
+
+			*data_len = sizeof(uint16_t);
+			return &tun->u.tun_geneve.opts[pos].class;
+		}
+	case NFTNL_OBJ_TUNNEL_GENEVE_TYPE:
+		{
+			int pos = *data_len;
+
+			*data_len = sizeof(uint16_t);
+			return &tun->u.tun_geneve.opts[pos].type;
+		}
+	case NFTNL_OBJ_TUNNEL_GENEVE_DATA:
+		{
+			int pos = *data_len;
+
+			*data_len = tun->u.tun_geneve.opts[pos].data_len;
+			return tun->u.tun_geneve.opts[pos].data;
+		}
 	}
 	return NULL;
 }
@@ -219,9 +256,13 @@ nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_FLAGS))
 		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_FLAGS, htonl(tun->tun_flags));
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_VXLAN_GBP)) {
+		struct nlattr *nest_inner;
+
 		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
+		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_VXLAN);
 		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_VXLAN_GBP,
-				 htonl(tun->u.tun_vxlan.gbp));
+				 tun->u.tun_vxlan.gbp);
+		mnl_attr_nest_end(nlh, nest_inner);
 		mnl_attr_nest_end(nlh, nest);
 	}
 	if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_VERSION) &&
@@ -232,8 +273,8 @@ nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 
 		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
 		nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_ERSPAN);
-		mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
-				 htonl(tun->u.tun_erspan.version));
+		mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_ERSPAN_VERSION,
+				tun->u.tun_erspan.version);
 		if (e->flags & (1 << NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX))
 			mnl_attr_put_u32(nlh, NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX,
 					 htonl(tun->u.tun_erspan.u.v1_index));
@@ -246,6 +287,26 @@ nftnl_obj_tunnel_build(struct nlmsghdr *nlh, const struct nftnl_obj *e)
 		mnl_attr_nest_end(nlh, nest_inner);
 		mnl_attr_nest_end(nlh, nest);
 	}
+	if (e->flags & (1lu << NFTNL_OBJ_TUNNEL_GENEVE_CLASS)  &&
+	    e->flags & (1lu << NFTNL_OBJ_TUNNEL_GENEVE_TYPE) &&
+	    e->flags & (1lu << NFTNL_OBJ_TUNNEL_GENEVE_DATA)) {
+		struct nlattr *nest_inner;
+		int i;
+
+		nest = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS);
+		for (i = 0; i < tun->u.tun_geneve.opt_num; i++) {
+			nest_inner = mnl_attr_nest_start(nlh, NFTA_TUNNEL_KEY_OPTS_GENEVE);
+			mnl_attr_put_u16(nlh, NFTA_TUNNEL_KEY_GENEVE_CLASS,
+					 tun->u.tun_geneve.opts[i].class);
+			mnl_attr_put_u8(nlh, NFTA_TUNNEL_KEY_GENEVE_TYPE,
+					 tun->u.tun_geneve.opts[i].type);
+			mnl_attr_put(nlh, NFTA_TUNNEL_KEY_GENEVE_DATA,
+				     tun->u.tun_geneve.opts[i].data_len,
+				     tun->u.tun_geneve.opts[i].data);
+			mnl_attr_nest_end(nlh, nest_inner);
+		}
+		mnl_attr_nest_end(nlh, nest);
+	}
 }
 
 static int nftnl_obj_tunnel_ip_cb(const struct nlattr *attr, void *data)
@@ -371,7 +432,7 @@ nftnl_obj_tunnel_parse_vxlan(struct nftnl_obj *e, struct nlattr *attr,
 
 	if (tb[NFTA_TUNNEL_KEY_VXLAN_GBP]) {
 		tun->u.tun_vxlan.gbp =
-			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]));
+			mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]);
 		e->flags |= (1 << NFTNL_OBJ_TUNNEL_VXLAN_GBP);
 	}
 
@@ -387,11 +448,11 @@ static int nftnl_obj_tunnel_erspan_cb(const struct nlattr *attr, void *data)
 		return MNL_CB_OK;
 
 	switch (type) {
-	case NFTA_TUNNEL_KEY_ERSPAN_VERSION:
 	case NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX:
 		if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0)
 			abi_breakage();
 		break;
+	case NFTA_TUNNEL_KEY_ERSPAN_VERSION:
 	case NFTA_TUNNEL_KEY_ERSPAN_V2_HWID:
 	case NFTA_TUNNEL_KEY_ERSPAN_V2_DIR:
 		if (mnl_attr_validate(attr, MNL_TYPE_U8) < 0)
@@ -414,7 +475,7 @@ nftnl_obj_tunnel_parse_erspan(struct nftnl_obj *e, struct nlattr *attr,
 
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]) {
 		tun->u.tun_erspan.version =
-			ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
+			mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
 		e->flags |= (1 << NFTNL_OBJ_TUNNEL_ERSPAN_VERSION);
 	}
 	if (tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]) {
@@ -436,6 +497,36 @@ nftnl_obj_tunnel_parse_erspan(struct nftnl_obj *e, struct nlattr *attr,
 	return 0;
 }
 
+static int
+nftnl_obj_tunnel_parse_geneve(struct nftnl_obj *e, struct nlattr *attr,
+			      struct nftnl_obj_tunnel *tun)
+{
+	struct nlattr *nla;
+
+	mnl_attr_for_each_nested(nla, attr) {
+		if (mnl_attr_get_type(nla) == NFTA_TUNNEL_KEY_GENEVE_CLASS) {
+			tun->u.tun_geneve.opt_num++;
+			tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].class =
+				mnl_attr_get_u16(nla);
+			e->flags |= (1lu << NFTNL_OBJ_TUNNEL_GENEVE_CLASS);
+		} else if (mnl_attr_get_type(nla) == NFTA_TUNNEL_KEY_GENEVE_TYPE) {
+			tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].type =
+				mnl_attr_get_u8(nla);
+			e->flags |= (1lu << NFTNL_OBJ_TUNNEL_GENEVE_TYPE);
+		} else if (mnl_attr_get_type(nla) == NFTA_TUNNEL_KEY_GENEVE_DATA) {
+			tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].data =
+				mnl_attr_get_payload(nla);
+			tun->u.tun_geneve.opts[tun->u.tun_geneve.opt_num - 1].data_len =
+				mnl_attr_get_payload_len(nla);
+			e->flags |= (1lu << NFTNL_OBJ_TUNNEL_GENEVE_DATA);
+		} else {
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
 static int nftnl_obj_tunnel_opts_cb(const struct nlattr *attr, void *data)
 {
 	const struct nlattr **tb = data;
@@ -447,6 +538,7 @@ static int nftnl_obj_tunnel_opts_cb(const struct nlattr *attr, void *data)
 	switch (type) {
 	case NFTA_TUNNEL_KEY_OPTS_VXLAN:
 	case NFTA_TUNNEL_KEY_OPTS_ERSPAN:
+	case NFTA_TUNNEL_KEY_OPTS_GENEVE:
 		if (mnl_attr_validate(attr, MNL_TYPE_NESTED) < 0)
 			abi_breakage();
 		break;
@@ -472,6 +564,9 @@ nftnl_obj_tunnel_parse_opts(struct nftnl_obj *e, struct nlattr *attr,
 	} else if (tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN]) {
 		err = nftnl_obj_tunnel_parse_erspan(e, tb[NFTA_TUNNEL_KEY_OPTS_ERSPAN],
 						    tun);
+	} else if (tb[NFTA_TUNNEL_KEY_OPTS_GENEVE]) {
+		err = nftnl_obj_tunnel_parse_geneve(e, tb[NFTA_TUNNEL_KEY_OPTS_GENEVE],
+						    tun);
 	}
 
 	return err;
@@ -518,7 +613,7 @@ nftnl_obj_tunnel_parse(struct nftnl_obj *e, struct nlattr *attr)
 		e->flags |= (1 << NFTNL_OBJ_TUNNEL_TTL);
 	}
 	if (tb[NFTA_TUNNEL_KEY_FLAGS]) {
-		tun->tun_flags = mnl_attr_get_u8(tb[NFTA_TUNNEL_KEY_FLAGS]);
+		tun->tun_flags = ntohl(mnl_attr_get_u32(tb[NFTA_TUNNEL_KEY_FLAGS]));
 		e->flags |= (1 << NFTNL_OBJ_TUNNEL_FLAGS);
 	}
 	if (tb[NFTA_TUNNEL_KEY_OPTS]) {
diff --git a/src/object.c b/src/object.c
index c876add..09c635a 100644
--- a/src/object.c
+++ b/src/object.c
@@ -109,7 +109,7 @@ void nftnl_obj_set_data(struct nftnl_obj *obj, uint16_t attr,
 			obj->ops->set(obj, attr, data, data_len);
 		break;
 	}
-	obj->flags |= (1 << attr);
+	obj->flags |= (1lu << attr);
 }
 
 void nftnl_obj_set(struct nftnl_obj *obj, uint16_t attr, const void *data) __visible;
@@ -152,7 +152,7 @@ EXPORT_SYMBOL(nftnl_obj_get_data);
 const void *nftnl_obj_get_data(struct nftnl_obj *obj, uint16_t attr,
 			       uint32_t *data_len)
 {
-	if (!(obj->flags & (1 << attr)))
+	if (!(obj->flags & (1lu << attr)))
 		return NULL;
 
 	switch(attr) {
-- 
2.18.1


[-- Attachment #3: 0001-nftables-nft_tunnel-add-support-for-dumping-vxlan-er.patch --]
[-- Type: application/octet-stream, Size: 14741 bytes --]

From ac451f621050fcb80f023084fb890363f8646776 Mon Sep 17 00:00:00 2001
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 6 Dec 2019 04:19:53 -0500
Subject: [PATCH] nftables: nft_tunnel: add support for dumping
 vxlan/erspan/geneve opts

Based on:

  git://git.netfilter.org/nftables: d42e9a1 (tag: v0.9.2) build: Bump version to v0.9.2
  plusing:
    https://patchwork.ozlabs.org/patch/1136755/
    https://patchwork.ozlabs.org/patch/1136756/

Dependencies:

   Some fixes in libnftnl, which are in another patch.

Examples:

  # nft add table ip filter
  # nft add chain ip filter input { type filter hook input priority 0 \; }
  # nft add tunnel filter vxlan_01  { type vxlan\;  id 2\; ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; opts \"ffff\"\; }
  # nft add tunnel filter erspan_01 { type erspan\; id 2\; ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; opts \"1:1:0:0\"\; }
  # nft add tunnel filter erspan_02 { type erspan\; id 2\; ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; opts \"2:0:1:1\"\; }
  # nft add tunnel filter geneve_01 { type geneve\; id 2\; ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; opts \"1:1:1212121234567890\"\; }
  # nft add tunnel filter geneve_02 { type geneve\; id 2\; ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; opts \"1:1:1212121234567890,2:2:1212121234567890,3:3:1212121234567890\"\; }
  # nft list tunnels table filter
  # nft add rule filter input ip protocol udp tunnel name geneve_02
  # nft add rule filter input meta l4proto udp tunnel id 2 drop
  # nft add rule filter input meta l4proto udp tunnel path 0 drop
  # nft list chain filter input -a

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/rule.h     |   3 +-
 src/evaluate.c     |   4 ++
 src/json.c         |   4 ++
 src/mnl.c          | 108 +++++++++++++++++++++++++++++++++++++++++++++
 src/netlink.c      |  72 ++++++++++++++++++++++++++++++
 src/parser_bison.y |  27 +++++++++++-
 src/rule.c         |  21 +++++++++
 src/scanner.l      |   1 +
 8 files changed, 237 insertions(+), 3 deletions(-)

diff --git a/include/rule.h b/include/rule.h
index cc221d6..6241760 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -408,11 +408,12 @@ struct tunnel {
 	uint32_t	id;
 	struct expr	*src;
 	struct expr	*dst;
-	uint32_t	label;
+	uint32_t	flags;
 	uint16_t	sport;
 	uint16_t	dport;
 	uint8_t		tos;
 	uint8_t		ttl;
+	const char	*opts;
 };
 
 /**
diff --git a/src/evaluate.c b/src/evaluate.c
index 62c5f52..74fa739 100755
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3717,6 +3717,7 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_CT_HELPER:
 	case CMD_OBJ_CT_TIMEOUT:
 	case CMD_OBJ_LIMIT:
+	case CMD_OBJ_TUNNEL:
 	case CMD_OBJ_SECMARK:
 	case CMD_OBJ_CT_EXPECT:
 		return 0;
@@ -3860,6 +3861,8 @@ static int cmd_evaluate_list(struct eval_ctx *ctx, struct cmd *cmd)
 		return cmd_evaluate_list_obj(ctx, cmd, NFT_OBJECT_CT_TIMEOUT);
 	case CMD_OBJ_LIMIT:
 		return cmd_evaluate_list_obj(ctx, cmd, NFT_OBJECT_LIMIT);
+	case CMD_OBJ_TUNNEL:
+		return cmd_evaluate_list_obj(ctx, cmd, NFT_OBJECT_TUNNEL);
 	case CMD_OBJ_SECMARK:
 		return cmd_evaluate_list_obj(ctx, cmd, NFT_OBJECT_SECMARK);
 	case CMD_OBJ_CT_EXPECT:
@@ -3868,6 +3871,7 @@ static int cmd_evaluate_list(struct eval_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_QUOTAS:
 	case CMD_OBJ_CT_HELPERS:
 	case CMD_OBJ_LIMITS:
+	case CMD_OBJ_TUNNELS:
 	case CMD_OBJ_SETS:
 	case CMD_OBJ_FLOWTABLES:
 	case CMD_OBJ_SECMARKS:
diff --git a/src/json.c b/src/json.c
index 55ce053..3ad7aba 100644
--- a/src/json.c
+++ b/src/json.c
@@ -1772,6 +1772,10 @@ int do_command_list_json(struct netlink_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_COUNTERS:
 		root = do_list_obj_json(ctx, cmd, NFT_OBJECT_COUNTER);
 		break;
+	case CMD_OBJ_TUNNEL:
+	case CMD_OBJ_TUNNELS:
+		root = do_list_obj_json(ctx, cmd, NFT_OBJECT_TUNNEL);
+		break;
 	case CMD_OBJ_QUOTA:
 	case CMD_OBJ_QUOTAS:
 		root = do_list_obj_json(ctx, cmd, NFT_OBJECT_QUOTA);
diff --git a/src/mnl.c b/src/mnl.c
index 00e088b..660ed49 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -26,6 +26,7 @@
 
 #include <mnl.h>
 #include <string.h>
+#include <stdlib.h>
 #include <sys/socket.h>
 #include <arpa/inet.h>
 #include <fcntl.h>
@@ -961,6 +962,40 @@ err:
 	return NULL;
 }
 
+static int get_hex(char c)
+{
+	if (c >= 'A' && c <= 'F')
+		return c - 'A' + 10;
+	if (c >= 'a' && c <= 'f')
+		return c - 'a' + 10;
+	if (c >= '0' && c <= '9')
+		return c - '0';
+
+	return -1;
+}
+
+static int hex2mem(const char *buf, uint8_t *mem, int count)
+{
+	int i, j;
+	int c;
+
+	for (i = 0, j = 0; i < count; i++, j += 2) {
+		c = get_hex(buf[j]);
+		if (c < 0)
+			return -1;
+
+		mem[i] = c << 4;
+
+		c = get_hex(buf[j + 1]);
+		if (c < 0)
+			return -1;
+
+		mem[i] |= c;
+	}
+
+	return 0;
+}
+
 int mnl_nft_obj_add(struct netlink_ctx *ctx, const struct cmd *cmd,
 		    unsigned int flags)
 {
@@ -1036,6 +1071,79 @@ int mnl_nft_obj_add(struct netlink_ctx *ctx, const struct cmd *cmd,
 					      nld.value);
 			}
 		}
+		if (obj->tunnel.flags)
+			nftnl_obj_set_u32(nlo, NFTNL_OBJ_TUNNEL_FLAGS,
+					  obj->tunnel.flags);
+		if (obj->tunnel.opts) {
+			const char *type = obj->tunnel.type;
+
+			if (strcmp(type, "vxlan") == 0) {
+				nftnl_obj_set_u32(nlo, NFTNL_OBJ_TUNNEL_VXLAN_GBP,
+						  strtoul(obj->tunnel.opts, NULL, 16));
+			}
+			if (strcmp(type, "erspan") == 0) {
+				char *token, *str = strdup(obj->tunnel.opts);
+				int i = NFTNL_OBJ_TUNNEL_ERSPAN_VERSION;
+				uint8_t ver;
+
+				token = strsep(&str, ":");
+				while (token) {
+					if (i == NFTNL_OBJ_TUNNEL_ERSPAN_VERSION) {
+						ver = strtoul(token, NULL, 16);
+						nftnl_obj_set_u8(nlo, i, ver);
+					} else if (i == NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX) {
+						if (ver == 1)
+							nftnl_obj_set_u32(nlo, i, strtoul(token, NULL, 16));
+					} else if (i == NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID) {
+						if (ver == 2)
+							nftnl_obj_set_u8(nlo, i, strtoul(token, NULL, 16));
+					} else if (i == NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR) {
+						if (ver == 2)
+							nftnl_obj_set_u8(nlo, i, strtoul(token, NULL, 16));
+					} else {
+						BUG("Unknown attr %d\n", i);
+					}
+					token = strsep(&str, ":");
+					i++;
+				}
+			}
+			if (strcmp(type, "geneve") == 0) {
+				char *str, *out_str = strdup(obj->tunnel.opts);
+
+				str = strsep(&out_str, ",");
+				while (str) {
+					int i = NFTNL_OBJ_TUNNEL_GENEVE_CLASS;
+					char *token = strsep(&str, ":");
+
+					while (token) {
+						if (i == NFTNL_OBJ_TUNNEL_GENEVE_CLASS) {
+							nftnl_obj_set_u16(nlo, i, strtoul(token, NULL, 16));
+						} else if (i == NFTNL_OBJ_TUNNEL_GENEVE_TYPE) {
+							nftnl_obj_set_u8(nlo, i, strtoul(token, NULL, 16));
+						} else if (i == NFTNL_OBJ_TUNNEL_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;
+							}
+							nftnl_obj_set_data(nlo, i, opts, token_len / 2);
+						} else {
+							BUG("Unknown attr %d\n", i);
+						}
+						token = strsep(&str, ":");
+						i++;
+					}
+					str = strsep(&out_str, ",");
+				}
+			}
+		}
 		break;
 	case NFT_OBJECT_CT_HELPER:
 		nftnl_obj_set_str(nlo, NFTNL_OBJ_CT_HELPER_NAME,
diff --git a/src/netlink.c b/src/netlink.c
index b04e315..f2db3be 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1003,6 +1003,21 @@ static struct expr *netlink_obj_tunnel_parse_addr(struct nftnl_obj *nlo,
 	return expr;
 }
 
+static char *hexstring_n2a(uint8_t *str, int len, char *buf, int blen)
+{
+	char *ptr = buf;
+	int i;
+
+	for (i = 0; i < len; i++) {
+		if (blen < 3)
+			break;
+		sprintf(ptr, "%02x", str[i]);
+		ptr += 2;
+		blen -= 2;
+	}
+	return buf;
+}
+
 struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 				    struct nftnl_obj *nlo)
 {
@@ -1085,6 +1100,8 @@ struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 			nftnl_obj_get_u16(nlo, NFTNL_OBJ_TUNNEL_TOS) >> 2;
 		obj->tunnel.ttl =
 			nftnl_obj_get_u16(nlo, NFTNL_OBJ_TUNNEL_TTL);
+		obj->tunnel.flags =
+			nftnl_obj_get_u32(nlo, NFTNL_OBJ_TUNNEL_FLAGS);
 
 		if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_IPV4_SRC)) {
 			obj->tunnel.src =
@@ -1106,6 +1123,61 @@ struct obj *netlink_delinearize_obj(struct netlink_ctx *ctx,
 				netlink_obj_tunnel_parse_addr(nlo,
 					NFTNL_OBJ_TUNNEL_IPV6_DST);
 		}
+		if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_VXLAN_GBP)) {
+			uint32_t gbp = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TUNNEL_VXLAN_GBP);
+			char opts[255] = {};
+
+			obj->tunnel.type = strdup("vxlan");
+			sprintf(opts, "%x", gbp);
+			obj->tunnel.opts = strdup(opts);
+		} else if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_ERSPAN_VERSION)) {
+			uint8_t ver = nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_ERSPAN_VERSION);
+			uint8_t hwid = 0, dir = 0;
+			uint32_t index = 0;
+			char opts[255] = {};
+
+			if (ver == 1)
+				index = nftnl_obj_get_u32(nlo, NFTNL_OBJ_TUNNEL_ERSPAN_V1_INDEX);
+
+			if (ver == 2) {
+				hwid = nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_ERSPAN_V2_HWID);
+				dir = nftnl_obj_get_u8(nlo, NFTNL_OBJ_TUNNEL_ERSPAN_V2_DIR);
+			}
+
+			obj->tunnel.type = strdup("erspan");
+			sprintf(opts, "%x:%x:%x:%x", ver, index, hwid, dir);
+			obj->tunnel.opts = strdup(opts);
+		} else if (nftnl_obj_is_set(nlo, NFTNL_OBJ_TUNNEL_GENEVE_CLASS)) {
+			char opts[255] = {};
+			char *optp = opts;
+			uint32_t i = 0, pos;
+			uint16_t class;
+			uint8_t type, *opt_data;
+			char data[128];
+
+			pos = 0;
+			class = *((uint16_t *)nftnl_obj_get_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_CLASS, &pos));
+			pos = 0;
+			type = *((uint8_t *)nftnl_obj_get_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_TYPE, &pos));
+			pos = 0;
+			opt_data = (uint8_t *)nftnl_obj_get_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_DATA, &pos);
+			hexstring_n2a(opt_data, pos, data, sizeof(data));
+			optp += sprintf(optp, "%x:%x:%s", class, type, data);
+			for (i = 1; i < 64; i++) {
+				pos = i;
+				class = *((uint16_t *)nftnl_obj_get_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_CLASS, &pos));
+				pos = i;
+				type = *((uint8_t *)nftnl_obj_get_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_TYPE, &pos));
+				pos = i;
+				opt_data = (uint8_t *)nftnl_obj_get_data(nlo, NFTNL_OBJ_TUNNEL_GENEVE_DATA, &pos);
+				if (!opt_data)
+					break;
+				hexstring_n2a(opt_data, pos, data, sizeof(data));
+				optp += sprintf(optp, ",%x:%x:%s", class, type, data);
+			}
+			obj->tunnel.type = strdup("geneve");
+			obj->tunnel.opts = strdup(opts);
+		}
 		break;
 	}
 	obj->type = type;
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 4eade0e..5429fd9 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -546,6 +546,7 @@ int nft_lex(void *, void *, void *);
 
 %token IN			"in"
 %token OUT			"out"
+%token OPTS			"opts"
 
 %type <string>			identifier type_identifier string comment_spec
 %destructor { xfree($$); }	identifier type_identifier string comment_spec
@@ -1016,7 +1017,7 @@ add_cmd			:	TABLE		table_spec
 			}
 			|	TUNNEL		obj_spec	tunnel_obj	'{' tunnel_block '}'	stmt_separator
 			{
-				$$ = cmd_alloc_obj_ct(CMD_ADD, CMD_OBJ_TUNNEL, &$2, &@$, $3);
+				$$ = cmd_alloc(CMD_ADD, CMD_OBJ_TUNNEL, &$2, &@$, $3);
 			}
 			;
 
@@ -1113,7 +1114,7 @@ create_cmd		:	TABLE		table_spec
 			}
 			|	TUNNEL		obj_spec	tunnel_obj	'{' tunnel_block '}'	stmt_separator
 			{
-				$$ = cmd_alloc_obj_ct(CMD_CREATE, CMD_OBJ_TUNNEL, &$2, &@$, $3);
+				$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_TUNNEL, &$2, &@$, $3);
 			}
 			;
 
@@ -1199,6 +1200,14 @@ delete_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_SECMARK, &$2, &@$, NULL);
 			}
+			| 	TUNNEL		obj_spec
+			{
+				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_TUNNEL, &$2, &@$, NULL);
+			}
+			| 	TUNNEL 		objid_spec
+			{
+				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_TUNNEL, &$2, &@$, NULL);
+			}
 			;
 
 get_cmd			:	ELEMENT		set_spec	set_block_expr
@@ -1271,6 +1280,18 @@ list_cmd		:	TABLE		table_spec
 			{
 				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_LIMIT, &$2, &@$, NULL);
 			}
+			|	TUNNELS		ruleset_spec
+			{
+				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_TUNNELS, &$2, &@$, NULL);
+			}
+			|	TUNNELS		TABLE	table_spec
+			{
+				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_TUNNELS, &$3, &@$, NULL);
+			}
+			|	TUNNEL		obj_spec
+			{
+				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_TUNNEL, &$2, &@$, NULL);
+			}
 			|	SECMARKS	ruleset_spec
 			{
 				$$ = cmd_alloc(CMD_LIST, CMD_OBJ_SECMARKS, &$2, &@$, NULL);
@@ -3774,6 +3795,8 @@ tunnel_config		:	TYPE	string		stmt_separator	{	$<obj>0->tunnel.type = $2;	}
 			|	DPORT	NUM		stmt_separator	{	$<obj>0->tunnel.dport = $2;	}
 			|	DSCP	NUM		stmt_separator	{	$<obj>0->tunnel.tos = $2 << 2;	}
 			|	TTL	NUM		stmt_separator	{	$<obj>0->tunnel.ttl = $2;	}
+			|	FLAGS	NUM		stmt_separator	{	$<obj>0->tunnel.flags = $2;	}
+			|	OPTS	string		stmt_separator	{	$<obj>0->tunnel.opts = $2;	}
 			;
 
 tunnel_block		:	/* empty */	{ $$ = $<obj>-1; }
diff --git a/src/rule.c b/src/rule.c
index 3b52f63..a50bc85 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1921,10 +1921,31 @@ static void obj_print_data(const struct obj *obj,
 		nft_print(octx, "%s%sid %u%s",
 			  opts->tab, opts->tab, obj->tunnel.id, opts->nl);
 
+		if (obj->tunnel.src) {
+			nft_print(octx, "%s%sip saddr ",
+				  opts->tab, opts->tab);
+			expr_print(obj->tunnel.src, octx);
+			nft_print(octx, "%s", opts->nl);
+		}
 		if (obj->tunnel.dst) {
 			nft_print(octx, "%s%sip daddr ",
 				  opts->tab, opts->tab);
 			expr_print(obj->tunnel.dst, octx);
+			nft_print(octx, "%s", opts->nl);
+		}
+		nft_print(octx, "%s%ssport %d%s", opts->tab, opts->tab,
+			  obj->tunnel.sport, opts->nl);
+		nft_print(octx, "%s%sdport %d%s", opts->tab, opts->tab,
+			  obj->tunnel.dport, opts->nl);
+		nft_print(octx, "%s%stos %d%s", opts->tab, opts->tab,
+			  obj->tunnel.tos, opts->nl);
+		nft_print(octx, "%s%sttl %d%s", opts->tab, opts->tab,
+			  obj->tunnel.ttl, opts->nl);
+		nft_print(octx, "%s%sflags %x%s", opts->tab, opts->tab,
+			  obj->tunnel.flags, opts->nl);
+		if (obj->tunnel.opts) {
+			nft_print(octx, "%s%s%s opts %s%s", opts->tab, opts->tab,
+				  obj->tunnel.type, obj->tunnel.opts, opts->nl);
 		}
 		break;
 	default:
diff --git a/src/scanner.l b/src/scanner.l
index 71bf60e..72733a1 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -388,6 +388,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "saddr"			{ return SADDR; }
 "daddr"			{ return DADDR; }
 "type"			{ return TYPE; }
+"opts"			{ return OPTS; }
 
 "vlan"			{ return VLAN; }
 "id"			{ return ID; }
-- 
2.18.1

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Simon Horman]

Hi Xin,

On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> To keep consistent with ipgre_policy, it's better to parse
> ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> cls_flower and ip_tunnel_core.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/netfilter/nft_tunnel.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> index 3d4c2ae..f76cd7d 100644
> --- a/net/netfilter/nft_tunnel.c
> +++ b/net/netfilter/nft_tunnel.c
> @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
>  }
>  
>  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> +	[NFTA_TUNNEL_KEY_ERSPAN_VERSION]	= { .type = NLA_U8 },
>  	[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]	= { .type = NLA_U32 },
> -	[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]	= { .type = NLA_U8 },
> +	[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]		= { .type = NLA_U8 },
>  	[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]	= { .type = NLA_U8 },
>  };
>  
> @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
>  	if (err < 0)
>  		return err;
>  
> -	version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> +	version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);

I have concerns about this change and backwards-compatibility with existing
users of this UAPI. Likewise, with other changes to the encoding of existing
attributes elsewhere in this series.

>  	switch (version) {
>  	case ERSPAN_VERSION:
>  		if (!tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX])
> -- 
> 2.1.0
> 

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Xin Long]

On Tue, Dec 10, 2019 at 4:03 AM Simon Horman <simon.horman@netronome.com> wrote:
>
> Hi Xin,
>
> On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> > To keep consistent with ipgre_policy, it's better to parse
> > ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> > cls_flower and ip_tunnel_core.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  net/netfilter/nft_tunnel.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > index 3d4c2ae..f76cd7d 100644
> > --- a/net/netfilter/nft_tunnel.c
> > +++ b/net/netfilter/nft_tunnel.c
> > @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
> >  }
> >
> >  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> > +     [NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U8 },
> >       [NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]       = { .type = NLA_U32 },
> > -     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR] = { .type = NLA_U8 },
> > +     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]         = { .type = NLA_U8 },
> >       [NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]        = { .type = NLA_U8 },
> >  };
> >
> > @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
> >       if (err < 0)
> >               return err;
> >
> > -     version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> > +     version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
>
> I have concerns about this change and backwards-compatibility with existing
> users of this UAPI. Likewise, with other changes to the encoding of existing
> attributes elsewhere in this series.
userspace(nftables/libnftnl) is not ready for nft_tunnel, I don't
think there will be
any backwards-compatibility issue.

Pablo?

>
> >       switch (version) {
> >       case ERSPAN_VERSION:
> >               if (!tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX])
> > --
> > 2.1.0
> >

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Pablo Neira Ayuso]

Hi,

On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> To keep consistent with ipgre_policy, it's better to parse
> ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> cls_flower and ip_tunnel_core.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/netfilter/nft_tunnel.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> index 3d4c2ae..f76cd7d 100644
> --- a/net/netfilter/nft_tunnel.c
> +++ b/net/netfilter/nft_tunnel.c
> @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
>  }
>  
>  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> +	[NFTA_TUNNEL_KEY_ERSPAN_VERSION]	= { .type = NLA_U8 },
>  	[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]	= { .type = NLA_U32 },
> -	[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]	= { .type = NLA_U8 },
> +	[NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]		= { .type = NLA_U8 },
>  	[NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]	= { .type = NLA_U8 },
>  };
>  
> @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
>  	if (err < 0)
>  		return err;
>  
> -	version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> +	version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);

I think NFTA_TUNNEL_KEY_ERSPAN_VERSION as 32-bit is just fine.

Netlink will be adding the padding anyway for u8.

I would suggest you leave this as is.

Thanks.

Re: [PATCH nf-next 2/7] netfilter: nft_tunnel: parse VXLAN_GBP attr as u32 in nft_tunnel

[Pablo Neira Ayuso]

On Sun, Dec 08, 2019 at 12:41:32PM +0800, Xin Long wrote:
> Both user and kernel sides want VXLAN_GBP opt as u32, so there's no
> need to convert it on each side.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/netfilter/nft_tunnel.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> index f76cd7d..d9d6c0d 100644
> --- a/net/netfilter/nft_tunnel.c
> +++ b/net/netfilter/nft_tunnel.c
> @@ -239,7 +239,7 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
>  	if (!tb[NFTA_TUNNEL_KEY_VXLAN_GBP])
>  		return -EINVAL;
>  
> -	opts->u.vxlan.gbp = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]));
> +	opts->u.vxlan.gbp = nla_get_u32(tb[NFTA_TUNNEL_KEY_VXLAN_GBP]);

In netfilter, attributes go in network byte order to make it easier to
send them over the wire. The only remaining part that needs to be
converted to network byte order is the netlink header.

Please, leave this one as is.

Re: [PATCH nf-next 3/7] netfilter: nft_tunnel: no need to call htons() when dumping ports

[Pablo Neira Ayuso]

On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote:
> info->key.tp_src and tp_dst are __be16, when using nla_put_be16()
> to dump them, htons() is not needed, so remove it in this patch.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/netfilter/nft_tunnel.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> index d9d6c0d..e1184fa 100644
> --- a/net/netfilter/nft_tunnel.c
> +++ b/net/netfilter/nft_tunnel.c
> @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
>  static int nft_tunnel_ports_dump(struct sk_buff *skb,
>  				 struct ip_tunnel_info *info)
>  {
> -	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 ||
> -	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0)
> +	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 ||
> +	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0)

info->key.tp_src is already in __be16 as you describe. So I would take
this as a consistency fix. I would take this as a fix.

Re: [PATCH nf-next 4/7] netfilter: nft_tunnel: also dump ERSPAN_VERSION

[Pablo Neira Ayuso]

On Sun, Dec 08, 2019 at 12:41:34PM +0800, Xin Long wrote:
> This is not necessary, but it'll be easier to parse in userspace,
> also given that other places like act_tunnel_key, cls_flower and
> ip_tunnel_core are also doing so.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nf-next 5/7] netfilter: nft_tunnel: also dump OPTS_ERSPAN/VXLAN

[Pablo Neira Ayuso]

On Sun, Dec 08, 2019 at 12:41:35PM +0800, Xin Long wrote:
> This patch is to add the nest attr OPTS_ERSPAN/VXLAN when dumping
> KEY_OPTS, and it would be helpful when parsing in userpace. Also,
> this is needed for supporting multiple geneve opts in the future
> patches.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nf-next 6/7] netfilter: nft_tunnel: add the missing nla_nest_cancel()

[Pablo Neira Ayuso]

On Sun, Dec 08, 2019 at 12:41:36PM +0800, Xin Long wrote:
> When nla_put_xxx() fails under nla_nest_start_noflag(),
> nla_nest_cancel() should be called, so that the skb can
> be trimmed properly.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nf-next 3/7] netfilter: nft_tunnel: no need to call htons() when dumping ports

[Pablo Neira Ayuso]

On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote:
> info->key.tp_src and tp_dst are __be16, when using nla_put_be16()
> to dump them, htons() is not needed, so remove it in this patch.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nf-next 3/7] netfilter: nft_tunnel: no need to call htons() when dumping ports

[Pablo Neira Ayuso]

On Wed, Dec 11, 2019 at 10:53:12PM +0100, Pablo Neira Ayuso wrote:
> On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote:
> > info->key.tp_src and tp_dst are __be16, when using nla_put_be16()
> > to dump them, htons() is not needed, so remove it in this patch.
> > 
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  net/netfilter/nft_tunnel.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > index d9d6c0d..e1184fa 100644
> > --- a/net/netfilter/nft_tunnel.c
> > +++ b/net/netfilter/nft_tunnel.c
> > @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
> >  static int nft_tunnel_ports_dump(struct sk_buff *skb,
> >  				 struct ip_tunnel_info *info)
> >  {
> > -	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 ||
> > -	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0)
> > +	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 ||
> > +	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0)
> 
> info->key.tp_src is already in __be16 as you describe. So I would take
> this as a consistency fix. I would take this as a fix.

I mean, I think this is worth fixing it indeed.

Re: [PATCH nf-next 3/7] netfilter: nft_tunnel: no need to call htons() when dumping ports

[Pablo Neira Ayuso]

On Wed, Dec 11, 2019 at 11:06:37PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Dec 11, 2019 at 10:53:12PM +0100, Pablo Neira Ayuso wrote:
> > On Sun, Dec 08, 2019 at 12:41:33PM +0800, Xin Long wrote:
> > > info->key.tp_src and tp_dst are __be16, when using nla_put_be16()
> > > to dump them, htons() is not needed, so remove it in this patch.
> > > 
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  net/netfilter/nft_tunnel.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > > index d9d6c0d..e1184fa 100644
> > > --- a/net/netfilter/nft_tunnel.c
> > > +++ b/net/netfilter/nft_tunnel.c
> > > @@ -502,8 +502,8 @@ static int nft_tunnel_opts_dump(struct sk_buff *skb,
> > >  static int nft_tunnel_ports_dump(struct sk_buff *skb,
> > >  				 struct ip_tunnel_info *info)
> > >  {
> > > -	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, htons(info->key.tp_src)) < 0 ||
> > > -	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, htons(info->key.tp_dst)) < 0)
> > > +	if (nla_put_be16(skb, NFTA_TUNNEL_KEY_SPORT, info->key.tp_src) < 0 ||
> > > +	    nla_put_be16(skb, NFTA_TUNNEL_KEY_DPORT, info->key.tp_dst) < 0)
> > 
> > info->key.tp_src is already in __be16 as you describe. So I would take
> > this as a consistency fix. I would take this as a fix.
> 
> I mean, I think this is worth fixing it indeed.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nf-next 0/7] netfilter: nft_tunnel: reinforce key opts support

[Xin Long]

On Sun, Dec 8, 2019 at 12:41 PM Xin Long <lucien.xin@gmail.com> wrote:
>
> This patchset improves quite a few places to make vxlan/erspan
> opts in nft_tunnel work with userspace nftables/libnftnl, and
> also keep consistent with the support for vxlan/erspan opts in
> act_tunnel_key, cls_flower and ip_tunnel_core.
>
> Meanwhile, add support for geneve opts in nft_tunnel. One patch
> for nftables and one for libnftnl will be posted here for the
> testing. With them, nft_tunnel can be set and used by:
>
>   # nft add table ip filter
>   # nft add chain ip filter input { type filter hook input priority 0 \; }
>   # nft add tunnel filter vxlan_01 { type vxlan\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"ffff\"\; }
>   # nft add tunnel filter erspan_01 { type erspan\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"1:1:0:0\"\; }
>   # nft add tunnel filter erspan_02 { type erspan\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"2:0:1:1\"\; }
>   # nft add tunnel filter geneve_01 { type geneve\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"1:1:1212121234567890\"\; }
>   # nft add tunnel filter geneve_02 { type geneve\; id 2\; \
>     ip saddr 192.168.1.1\; ip daddr 192.168.1.2\; \
>     sport 9000\; dport 9001\; dscp 1234\; ttl 64\; flags 1\; \
>     opts \"1:1:34567890,2:2:12121212,3:3:1212121234567890\"\; }
>   # nft list tunnels table filter
>   # nft add rule filter input ip protocol udp tunnel name geneve_02
>   # nft add rule filter input meta l4proto udp tunnel id 2 drop
>   # nft add rule filter input meta l4proto udp tunnel path 0 drop
>   # nft list chain filter input -a

Hi, Pablo
as you commented on other patches, I will post v2 and
>
> Xin Long (7):
>   netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8
>   netfilter: nft_tunnel: parse VXLAN_GBP attr as u32 in nft_tunnel
drop these two patches
>   netfilter: nft_tunnel: no need to call htons() when dumping ports
move this one to nf.git
>   netfilter: nft_tunnel: also dump ERSPAN_VERSION
>   netfilter: nft_tunnel: also dump OPTS_ERSPAN/VXLAN
>   netfilter: nft_tunnel: add the missing nla_nest_cancel()
adjust these three for nf-next.git
>   netfilter: nft_tunnel: add support for geneve opts
will you also check this one before my posting v2?

Thanks.

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Xin Long]

On Thu, Dec 12, 2019 at 5:51 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> Hi,
>
> On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> > To keep consistent with ipgre_policy, it's better to parse
> > ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> > cls_flower and ip_tunnel_core.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  net/netfilter/nft_tunnel.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > index 3d4c2ae..f76cd7d 100644
> > --- a/net/netfilter/nft_tunnel.c
> > +++ b/net/netfilter/nft_tunnel.c
> > @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
> >  }
> >
> >  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> > +     [NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U8 },
> >       [NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]       = { .type = NLA_U32 },
> > -     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR] = { .type = NLA_U8 },
> > +     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]         = { .type = NLA_U8 },
> >       [NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]        = { .type = NLA_U8 },
> >  };
> >
> > @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
> >       if (err < 0)
> >               return err;
> >
> > -     version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> > +     version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
>
> I think NFTA_TUNNEL_KEY_ERSPAN_VERSION as 32-bit is just fine.
>
> Netlink will be adding the padding anyway for u8.
>
> I would suggest you leave this as is.
okay.

do you think I should prepare another patch/fix for the missing nla_policy part?
[NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U32 },

Thanks.

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Pablo Neira Ayuso]

On Thu, Dec 12, 2019 at 11:20:19AM +0800, Xin Long wrote:
> On Thu, Dec 12, 2019 at 5:51 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > Hi,
> >
> > On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> > > To keep consistent with ipgre_policy, it's better to parse
> > > ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> > > cls_flower and ip_tunnel_core.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  net/netfilter/nft_tunnel.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > > index 3d4c2ae..f76cd7d 100644
> > > --- a/net/netfilter/nft_tunnel.c
> > > +++ b/net/netfilter/nft_tunnel.c
> > > @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
> > >  }
> > >
> > >  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> > > +     [NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U8 },
> > >       [NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]       = { .type = NLA_U32 },
> > > -     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR] = { .type = NLA_U8 },
> > > +     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]         = { .type = NLA_U8 },
> > >       [NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]        = { .type = NLA_U8 },
> > >  };
> > >
> > > @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
> > >       if (err < 0)
> > >               return err;
> > >
> > > -     version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> > > +     version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
> >
> > I think NFTA_TUNNEL_KEY_ERSPAN_VERSION as 32-bit is just fine.
> >
> > Netlink will be adding the padding anyway for u8.
> >
> > I would suggest you leave this as is.
> okay.
> 
> do you think I should prepare another patch/fix for the missing nla_policy part?
> [NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U32 },

Yes, please, thanks.

Re: [PATCH nf-next 0/7] netfilter: nft_tunnel: reinforce key opts support

[Pablo Neira Ayuso]

On Thu, Dec 12, 2019 at 11:02:19AM +0800, Xin Long wrote:
[...]
> Hi, Pablo
> as you commented on other patches, I will post v2 and
> >
> > Xin Long (7):
> >   netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8
> >   netfilter: nft_tunnel: parse VXLAN_GBP attr as u32 in nft_tunnel
> drop these two patches

Yes, you will still need the netlink policy validation for
ERSPAN_VERSION which is missing, so at least one patch will be needed
for this one.

> >   netfilter: nft_tunnel: no need to call htons() when dumping ports
> move this one to nf.git

Given that nft_tunnel really needs care and that there is no upstream
userspace code using this extension, I think using nf-next.git in this
case is fine.

Thanks.

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Simon Horman]

On Tue, Dec 10, 2019 at 12:05:15PM +0800, Xin Long wrote:
> On Tue, Dec 10, 2019 at 4:03 AM Simon Horman <simon.horman@netronome.com> wrote:
> >
> > Hi Xin,
> >
> > On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> > > To keep consistent with ipgre_policy, it's better to parse
> > > ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> > > cls_flower and ip_tunnel_core.
> > >
> > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > ---
> > >  net/netfilter/nft_tunnel.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > > index 3d4c2ae..f76cd7d 100644
> > > --- a/net/netfilter/nft_tunnel.c
> > > +++ b/net/netfilter/nft_tunnel.c
> > > @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
> > >  }
> > >
> > >  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> > > +     [NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U8 },
> > >       [NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]       = { .type = NLA_U32 },
> > > -     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR] = { .type = NLA_U8 },
> > > +     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]         = { .type = NLA_U8 },
> > >       [NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]        = { .type = NLA_U8 },
> > >  };
> > >
> > > @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
> > >       if (err < 0)
> > >               return err;
> > >
> > > -     version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> > > +     version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
> >
> > I have concerns about this change and backwards-compatibility with existing
> > users of this UAPI. Likewise, with other changes to the encoding of existing
> > attributes elsewhere in this series.
> userspace(nftables/libnftnl) is not ready for nft_tunnel, I don't
> think there will be
> any backwards-compatibility issue.
> 
> Pablo?

Thanks, I'm happy to defer to Pablo on this question.

> 
> >
> > >       switch (version) {
> > >       case ERSPAN_VERSION:
> > >               if (!tb[NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX])
> > > --
> > > 2.1.0
> > >

Re: [PATCH nf-next 1/7] netfilter: nft_tunnel: parse ERSPAN_VERSION attr as u8

[Pablo Neira Ayuso]

Hi Simon,

On Fri, Dec 13, 2019 at 10:30:26AM +0100, Simon Horman wrote:
> On Tue, Dec 10, 2019 at 12:05:15PM +0800, Xin Long wrote:
> > On Tue, Dec 10, 2019 at 4:03 AM Simon Horman <simon.horman@netronome.com> wrote:
> > >
> > > Hi Xin,
> > >
> > > On Sun, Dec 08, 2019 at 12:41:31PM +0800, Xin Long wrote:
> > > > To keep consistent with ipgre_policy, it's better to parse
> > > > ERSPAN_VERSION attr as u8, as it does in act_tunnel_key,
> > > > cls_flower and ip_tunnel_core.
> > > >
> > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > ---
> > > >  net/netfilter/nft_tunnel.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
> > > > index 3d4c2ae..f76cd7d 100644
> > > > --- a/net/netfilter/nft_tunnel.c
> > > > +++ b/net/netfilter/nft_tunnel.c
> > > > @@ -248,8 +248,9 @@ static int nft_tunnel_obj_vxlan_init(const struct nlattr *attr,
> > > >  }
> > > >
> > > >  static const struct nla_policy nft_tunnel_opts_erspan_policy[NFTA_TUNNEL_KEY_ERSPAN_MAX + 1] = {
> > > > +     [NFTA_TUNNEL_KEY_ERSPAN_VERSION]        = { .type = NLA_U8 },
> > > >       [NFTA_TUNNEL_KEY_ERSPAN_V1_INDEX]       = { .type = NLA_U32 },
> > > > -     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR] = { .type = NLA_U8 },
> > > > +     [NFTA_TUNNEL_KEY_ERSPAN_V2_DIR]         = { .type = NLA_U8 },
> > > >       [NFTA_TUNNEL_KEY_ERSPAN_V2_HWID]        = { .type = NLA_U8 },
> > > >  };
> > > >
> > > > @@ -266,7 +267,7 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
> > > >       if (err < 0)
> > > >               return err;
> > > >
> > > > -     version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
> > > > +     version = nla_get_u8(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]);
> > >
> > > I have concerns about this change and backwards-compatibility with existing
> > > users of this UAPI. Likewise, with other changes to the encoding of existing
> > > attributes elsewhere in this series.
> >
> > userspace(nftables/libnftnl) is not ready for nft_tunnel, I don't
> > think there will be any backwards-compatibility issue.
> > 
> > Pablo?
> 
> Thanks, I'm happy to defer to Pablo on this question.

I agree with Xin. This uapi is not in good shape and there is no
upstream userspace code for this, no nftables support for this yet.
In this particular case I'm inclined to fix uapi, better sooner than
never.

Thanks.