[PATCH/RFC net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key

[PATCH/RFC net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key

[Simon Horman]

Allow the flower classifier to match on tunnel options and the
tunnel key action to set them.

Tunnel options are a bytestring of up to 256 bytes.
The flower classifier matching with an optional bitwise mask.
Tunnel implementations may support more or less options,
or none at all.

Simon Horman (2):
  net/sched: add tunnel option support to act_tunnel_key
  net/sched: allow flower to match tunnel options

 include/net/flow_dissector.h              | 13 ++++++++++++
 include/uapi/linux/pkt_cls.h              |  3 +++
 include/uapi/linux/tc_act/tc_tunnel_key.h |  1 +
 net/sched/act_tunnel_key.c                | 26 ++++++++++++++++++-----
 net/sched/cls_flower.c                    | 35 ++++++++++++++++++++++++++++++-
 5 files changed, 72 insertions(+), 6 deletions(-)

-- 
2.1.4

[PATCH/RFC net-next 1/2] net/sched: add tunnel option support to act_tunnel_key

[Simon Horman]

Allow setting tunnel options using the act_tunnel_key action.

Options are a bitwise maskable bytestring of up to 256 bytes.
Tunnel implementations may support less or more options,
or no options at all.

e.g.
 # ip link add name geneve0 type geneve dstport 0 external
 # tc qdisc del dev geneve0 ingress
 # tc filter add dev geneve0 protocol ip parent ffff: \
     flower \
       enc_src_ip 10.0.99.192 \
       enc_dst_ip 10.0.99.193 \
       enc_key_id 11 \
       enc_opts 0102800100800020/fffffffffffffff0 \
       ip_proto udp \
       action mirred egress redirect dev eth1

Signed-off-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/uapi/linux/tc_act/tc_tunnel_key.h |  1 +
 net/sched/act_tunnel_key.c                | 26 +++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/include/uapi/linux/tc_act/tc_tunnel_key.h b/include/uapi/linux/tc_act/tc_tunnel_key.h
index afcd4be953e2..e0cb1121d132 100644
--- a/include/uapi/linux/tc_act/tc_tunnel_key.h
+++ b/include/uapi/linux/tc_act/tc_tunnel_key.h
@@ -35,6 +35,7 @@ enum {
 	TCA_TUNNEL_KEY_PAD,
 	TCA_TUNNEL_KEY_ENC_DST_PORT,	/* be16 */
 	TCA_TUNNEL_KEY_NO_CSUM,		/* u8 */
+	TCA_TUNNEL_KEY_ENC_OPTS,
 	__TCA_TUNNEL_KEY_MAX,
 };
 
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 30c96274c638..77b5890a48b9 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -66,6 +66,7 @@ static const struct nla_policy tunnel_key_policy[TCA_TUNNEL_KEY_MAX + 1] = {
 	[TCA_TUNNEL_KEY_ENC_KEY_ID]   = { .type = NLA_U32 },
 	[TCA_TUNNEL_KEY_ENC_DST_PORT] = {.type = NLA_U16},
 	[TCA_TUNNEL_KEY_NO_CSUM]      = { .type = NLA_U8 },
+	[TCA_TUNNEL_KEY_ENC_OPTS]     = { .type = NLA_BINARY },
 };
 
 static int tunnel_key_init(struct net *net, struct nlattr *nla,
@@ -81,9 +82,11 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 	struct tcf_tunnel_key *t;
 	bool exists = false;
 	__be16 dst_port = 0;
+	int opts_len = 0;
 	__be64 key_id;
 	__be16 flags;
 	int ret = 0;
+	u8 *opts;
 	int err;
 
 	if (!nla)
@@ -121,6 +124,11 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 		if (tb[TCA_TUNNEL_KEY_ENC_DST_PORT])
 			dst_port = nla_get_be16(tb[TCA_TUNNEL_KEY_ENC_DST_PORT]);
 
+		if (tb[TCA_TUNNEL_KEY_ENC_OPTS]) {
+			opts = nla_data(tb[TCA_TUNNEL_KEY_ENC_OPTS]);
+			opts_len = nla_len(tb[TCA_TUNNEL_KEY_ENC_OPTS]);
+		}
+
 		if (tb[TCA_TUNNEL_KEY_ENC_IPV4_SRC] &&
 		    tb[TCA_TUNNEL_KEY_ENC_IPV4_DST]) {
 			__be32 saddr;
@@ -131,7 +139,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 
 			metadata = __ip_tun_set_dst(saddr, daddr, 0, 0,
 						    dst_port, flags,
-						    key_id, 0);
+						    key_id, opts_len);
 		} else if (tb[TCA_TUNNEL_KEY_ENC_IPV6_SRC] &&
 			   tb[TCA_TUNNEL_KEY_ENC_IPV6_DST]) {
 			struct in6_addr saddr;
@@ -142,9 +150,13 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
 
 			metadata = __ipv6_tun_set_dst(&saddr, &daddr, 0, 0, dst_port,
 						      0, flags,
-						      key_id, 0);
+						      key_id, opts_len);
 		}
 
+		if (opts_len)
+			ip_tunnel_info_opts_set(&metadata->u.tun_info,
+						opts, opts_len);
+
 		if (!metadata) {
 			ret = -EINVAL;
 			goto err_out;
@@ -264,8 +276,9 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
 		goto nla_put_failure;
 
 	if (params->tcft_action == TCA_TUNNEL_KEY_ACT_SET) {
-		struct ip_tunnel_key *key =
-			&params->tcft_enc_metadata->u.tun_info.key;
+		struct ip_tunnel_info *info =
+			&params->tcft_enc_metadata->u.tun_info;
+		struct ip_tunnel_key *key = &info->key;
 		__be32 key_id = tunnel_id_to_key32(key->tun_id);
 
 		if (nla_put_be32(skb, TCA_TUNNEL_KEY_ENC_KEY_ID, key_id) ||
@@ -273,7 +286,10 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
 					      &params->tcft_enc_metadata->u.tun_info) ||
 		    nla_put_be16(skb, TCA_TUNNEL_KEY_ENC_DST_PORT, key->tp_dst) ||
 		    nla_put_u8(skb, TCA_TUNNEL_KEY_NO_CSUM,
-			       !(key->tun_flags & TUNNEL_CSUM)))
+			       !(key->tun_flags & TUNNEL_CSUM)) ||
+		    (info->options_len &&
+		     nla_put(skb, TCA_TUNNEL_KEY_ENC_OPTS, info->options_len,
+			     info + 1)))
 			goto nla_put_failure;
 	}
 
-- 
2.1.4

[PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Simon Horman]

Allow matching on options in tunnel headers.
This makes use of existing tunnel metadata support.

Options are a bytestring of up to 256 bytes.
Tunnel implementations may support less or more options,
or no options at all.

 # ip link add name geneve0 type geneve dstport 0 external
 # tc qdisc add dev eth0 ingress
 # tc qdisc del dev eth0 ingress; 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 4789 \
            id 11 \
            opts 0102800100800022 \
    action mirred egress redirect dev geneve0

Signed-off-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 include/net/flow_dissector.h | 13 +++++++++++++
 include/uapi/linux/pkt_cls.h |  3 +++
 net/sched/cls_flower.c       | 35 ++++++++++++++++++++++++++++++++++-
 3 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index fc3dce730a6b..43f98bf0b349 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {
 	__u8	ttl;
 };
 
+/**
+ * struct flow_dissector_key_enc_opts:
+ * @data: data
+ * @len: len
+ */
+struct flow_dissector_key_enc_opts {
+	u8 data[256];	/* Using IP_TUNNEL_OPTS_MAX is desired here
+			 * but seems difficult to #include
+			 */
+	u8 len;
+};
+
 enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
 	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
@@ -205,6 +217,7 @@ enum flow_dissector_key_id {
 	FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */
 	FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */
 	FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */
+	FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */
 
 	FLOW_DISSECTOR_KEY_MAX,
 };
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index d5e2bf68d0d4..7a09a28f21e0 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -467,6 +467,9 @@ enum {
 	TCA_FLOWER_KEY_IP_TTL,		/* u8 */
 	TCA_FLOWER_KEY_IP_TTL_MASK,	/* u8 */
 
+	TCA_FLOWER_KEY_ENC_OPTS,
+	TCA_FLOWER_KEY_ENC_OPTS_MASK,
+
 	__TCA_FLOWER_MAX,
 };
 
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 1a267e77c6de..2a8364ef4fd5 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -51,6 +51,7 @@ struct fl_flow_key {
 	struct flow_dissector_key_mpls mpls;
 	struct flow_dissector_key_tcp tcp;
 	struct flow_dissector_key_ip ip;
+	struct flow_dissector_key_enc_opts enc_opts;
 } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
 
 struct fl_flow_mask_range {
@@ -181,6 +182,11 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 		skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);
 		skb_key.enc_tp.src = key->tp_src;
 		skb_key.enc_tp.dst = key->tp_dst;
+
+		if (info->options_len) {
+			skb_key.enc_opts.len = info->options_len;
+			ip_tunnel_info_opts_get(skb_key.enc_opts.data, info);
+		}
 	}
 
 	skb_key.indev_ifindex = skb->skb_iif;
@@ -421,6 +427,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_KEY_IP_TOS_MASK]	= { .type = NLA_U8 },
 	[TCA_FLOWER_KEY_IP_TTL]		= { .type = NLA_U8 },
 	[TCA_FLOWER_KEY_IP_TTL_MASK]	= { .type = NLA_U8 },
+	[TCA_FLOWER_KEY_ENC_OPTS]	= { .type = NLA_BINARY },
+	[TCA_FLOWER_KEY_ENC_OPTS_MASK]	= { .type = NLA_BINARY },
 };
 
 static void fl_set_key_val(struct nlattr **tb,
@@ -712,6 +720,26 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 		       &mask->enc_tp.dst, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,
 		       sizeof(key->enc_tp.dst));
 
+	if (tb[TCA_FLOWER_KEY_ENC_OPTS]) {
+		key->enc_opts.len = nla_len(tb[TCA_FLOWER_KEY_ENC_OPTS]);
+
+		if (key->enc_opts.len > sizeof(key->enc_opts.data))
+			return -EINVAL;
+
+		/* enc_opts is variable length.
+		 * If present ensure the value and mask are the same length.
+		 */
+		if (tb[TCA_FLOWER_KEY_ENC_OPTS_MASK] &&
+		    nla_len(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]) != key->enc_opts.len)
+			return -EINVAL;
+
+		mask->enc_opts.len = key->enc_opts.len;
+		fl_set_key_val(tb, key->enc_opts.data, TCA_FLOWER_KEY_ENC_OPTS,
+			       mask->enc_opts.data,
+			       TCA_FLOWER_KEY_ENC_OPTS_MASK,
+			       key->enc_opts.len);
+	}
+
 	if (tb[TCA_FLOWER_KEY_FLAGS])
 		ret = fl_set_key_flags(tb, &key->control.flags, &mask->control.flags);
 
@@ -804,6 +832,8 @@ static void fl_init_dissector(struct cls_fl_head *head,
 			   enc_control);
 	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
 			     FLOW_DISSECTOR_KEY_ENC_PORTS, enc_tp);
+	FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt,
+			     FLOW_DISSECTOR_KEY_ENC_OPTS, enc_opts);
 
 	skb_flow_dissector_init(&head->dissector, keys, cnt);
 }
@@ -1327,7 +1357,10 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 			    TCA_FLOWER_KEY_ENC_UDP_DST_PORT,
 			    &mask->enc_tp.dst,
 			    TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,
-			    sizeof(key->enc_tp.dst)))
+			    sizeof(key->enc_tp.dst)) ||
+	    fl_dump_key_val(skb, key->enc_opts.data, TCA_FLOWER_KEY_ENC_OPTS,
+			    mask->enc_opts.data, TCA_FLOWER_KEY_ENC_OPTS_MASK,
+			    key->enc_opts.len))
 		goto nla_put_failure;
 
 	if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
-- 
2.1.4

Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Or Gerlitz]

On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
<simon.horman@netronome.com> wrote:
> Allow matching on options in tunnel headers.
> This makes use of existing tunnel metadata support.

Simon,

This patch is about matching on tunnel options, right? but

> Options are a bytestring of up to 256 bytes.
> Tunnel implementations may support less or more options,
> or no options at all.
>
>  # ip link add name geneve0 type geneve dstport 0 external
>  # tc qdisc add dev eth0 ingress
>  # tc qdisc del dev eth0 ingress; 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 4789 \
>             id 11 \
>             opts 0102800100800022 \
>     action mirred egress redirect dev geneve0

the example here is on how to use tunnel options in the tunnel set key actions..

And the other way around in the other patch... the patch is about the
tunnel key set action and the example shows how to match that in
flower... I guess you want to swap the relevant of the change log.

Anyway, is there any human readable/understandable representation of
these options? e.g what does 0102800100800022 means for geneve?

Re: [oss-drivers] Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Simon Horman]

On Tue, Sep 12, 2017 at 11:23:55PM +0300, Or Gerlitz wrote:
> On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > Allow matching on options in tunnel headers.
> > This makes use of existing tunnel metadata support.
> 
> Simon,
> 
> This patch is about matching on tunnel options, right? but
> 
> > Options are a bytestring of up to 256 bytes.
> > Tunnel implementations may support less or more options,
> > or no options at all.
> >
> >  # ip link add name geneve0 type geneve dstport 0 external
> >  # tc qdisc add dev eth0 ingress
> >  # tc qdisc del dev eth0 ingress; 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 4789 \
> >             id 11 \
> >             opts 0102800100800022 \
> >     action mirred egress redirect dev geneve0
> 
> the example here is on how to use tunnel options in the tunnel set key actions..
> 
> And the other way around in the other patch... the patch is about the
> tunnel key set action and the example shows how to match that in
> flower... I guess you want to swap the relevant of the change log.

Yes, it seems so. Sorry about that.

> Anyway, is there any human readable/understandable representation of
> these options? e.g what does 0102800100800022 means for geneve?

Thanks, I had not thought of that. My feeling is that could
be added to the tc tool as follow-up work.

Re: [oss-drivers] Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Or Gerlitz]

On Wed, Sep 13, 2017 at 12:25 PM, Simon Horman
<simon.horman@netronome.com> wrote:
> On Tue, Sep 12, 2017 at 11:23:55PM +0300, Or Gerlitz wrote:
>> On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
>> <simon.horman@netronome.com> wrote:
>> > Allow matching on options in tunnel headers.
>> > This makes use of existing tunnel metadata support.
>>
>> Simon,
>>
>> This patch is about matching on tunnel options, right? but
>>
>> > Options are a bytestring of up to 256 bytes.
>> > Tunnel implementations may support less or more options,
>> > or no options at all.
>> >
>> >  # ip link add name geneve0 type geneve dstport 0 external
>> >  # tc qdisc add dev eth0 ingress
>> >  # tc qdisc del dev eth0 ingress; 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 4789 \
>> >             id 11 \
>> >             opts 0102800100800022 \
>> >     action mirred egress redirect dev geneve0
>>
>> the example here is on how to use tunnel options in the tunnel set key actions..
>>
>> And the other way around in the other patch... the patch is about the
>> tunnel key set action and the example shows how to match that in
>> flower... I guess you want to swap the relevant of the change log.
>
> Yes, it seems so. Sorry about that.

no worries, you can do the swap, but before that

>> Anyway, is there any human readable/understandable representation of
>> these options? e.g what does 0102800100800022 means for geneve?

> Thanks, I had not thought of that. My feeling is that could
> be added to the tc tool as follow-up work.

could you spend few words on the nature of these options now when we review
the kernel patches? I guess this is somehow related to protocols such
as geneve and vxlan-gpe  -- it would be good if you elaborate on that
a bit, does
the kernel does any usage with these options beyond matching on them or stiching
them to packet headers?

Or.

Re: [oss-drivers] Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Simon Horman]

On Wed, Sep 13, 2017 at 01:03:43PM +0300, Or Gerlitz wrote:
> On Wed, Sep 13, 2017 at 12:25 PM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > On Tue, Sep 12, 2017 at 11:23:55PM +0300, Or Gerlitz wrote:
> >> On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
> >> <simon.horman@netronome.com> wrote:
> >> > Allow matching on options in tunnel headers.
> >> > This makes use of existing tunnel metadata support.
> >>
> >> Simon,
> >>
> >> This patch is about matching on tunnel options, right? but
> >>
> >> > Options are a bytestring of up to 256 bytes.
> >> > Tunnel implementations may support less or more options,
> >> > or no options at all.
> >> >
> >> >  # ip link add name geneve0 type geneve dstport 0 external
> >> >  # tc qdisc add dev eth0 ingress
> >> >  # tc qdisc del dev eth0 ingress; 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 4789 \
> >> >             id 11 \
> >> >             opts 0102800100800022 \
> >> >     action mirred egress redirect dev geneve0
> >>
> >> the example here is on how to use tunnel options in the tunnel set key actions..
> >>
> >> And the other way around in the other patch... the patch is about the
> >> tunnel key set action and the example shows how to match that in
> >> flower... I guess you want to swap the relevant of the change log.
> >
> > Yes, it seems so. Sorry about that.
> 
> no worries, you can do the swap, but before that
> 
> >> Anyway, is there any human readable/understandable representation of
> >> these options? e.g what does 0102800100800022 means for geneve?
> 
> > Thanks, I had not thought of that. My feeling is that could
> > be added to the tc tool as follow-up work.
> 
> could you spend few words on the nature of these options now when we review
> the kernel patches? I guess this is somehow related to protocols such
> as geneve and vxlan-gpe  -- it would be good if you elaborate on that
> a bit, does
> the kernel does any usage with these options beyond matching on them or stiching
> them to packet headers?

This feature is to be used in conjunction with tunnels in collect metadata
(external) mode. As I understand it there are three tunnel netdevs that use
options metadata in the kernel at this time.

* Geneve

  In the case of Geneve options are TLVs[1]. My reading is that in collect
  metadata mode the kernel does not appear to do anything other than pass
  them around as a bytestring.

  [1] https://tools.ietf.org/html/draft-ietf-nvo3-geneve-05#section-3.5

* VXLAN-GBP

  In the case of VXLAN-GBP on RX in collect metadata mode options are used
  to carry information parsed in vxlan_parse_gbp_hdr() from the VXLAN Group
  Based Policy Extension[2]. On RX the options data is used to create an
  extension (header) by vxlan_build_gbp_hdr().

  [2] https://tools.ietf.org/html/draft-smith-vxlan-group-policy-03#section-2.1

* ERSPAN (GRE)

  In the case of ERSPAN, which is a variant of GRE, on RX in collect
  metadata mode options are used to carry the index parsed from the ERSPAN
  Type II feature header[3] in erspan_rcv().  The converse is true on TX
  and is handled by erspan_fb_xmit().

  [3] https://tools.ietf.org/html/draft-foschiano-erspan-03#section-4.2

Users of options:

* There are eBPF hooks to allow getting on and setting tunnel metadata:
  bpf_skb_set_tunnel_opt, bpf_skb_get_tunnel_opt.

* Open vSwitch is able to match and set Geneve and VXLAN-GBP options.

Neither of the above appear to assume any structure for the data.

I hope the above is the kind of information you are after.

Re: [oss-drivers] Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Or Gerlitz]

On Wed, Sep 13, 2017 at 2:59 PM, Simon Horman
<simon.horman@netronome.com> wrote:
> On Wed, Sep 13, 2017 at 01:03:43PM +0300, Or Gerlitz wrote:
>> On Wed, Sep 13, 2017 at 12:25 PM, Simon Horman
>> <simon.horman@netronome.com> wrote:
>> > On Tue, Sep 12, 2017 at 11:23:55PM +0300, Or Gerlitz wrote:
>> >> On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
>> >> <simon.horman@netronome.com> wrote:
>> >> > Allow matching on options in tunnel headers.
>> >> > This makes use of existing tunnel metadata support.
>> >>
>> >> Simon,
>> >>
>> >> This patch is about matching on tunnel options, right? but
>> >>
>> >> > Options are a bytestring of up to 256 bytes.
>> >> > Tunnel implementations may support less or more options,
>> >> > or no options at all.
>> >> >
>> >> >  # ip link add name geneve0 type geneve dstport 0 external
>> >> >  # tc qdisc add dev eth0 ingress
>> >> >  # tc qdisc del dev eth0 ingress; 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 4789 \
>> >> >             id 11 \
>> >> >             opts 0102800100800022 \
>> >> >     action mirred egress redirect dev geneve0
>> >>
>> >> the example here is on how to use tunnel options in the tunnel set key actions..
>> >>
>> >> And the other way around in the other patch... the patch is about the
>> >> tunnel key set action and the example shows how to match that in
>> >> flower... I guess you want to swap the relevant of the change log.
>> >
>> > Yes, it seems so. Sorry about that.
>>
>> no worries, you can do the swap, but before that
>>
>> >> Anyway, is there any human readable/understandable representation of
>> >> these options? e.g what does 0102800100800022 means for geneve?
>>
>> > Thanks, I had not thought of that. My feeling is that could
>> > be added to the tc tool as follow-up work.
>>
>> could you spend few words on the nature of these options now when we review
>> the kernel patches? I guess this is somehow related to protocols such
>> as geneve and vxlan-gpe  -- it would be good if you elaborate on that
>> a bit, does
>> the kernel does any usage with these options beyond matching on them or stiching
>> them to packet headers?
>
> This feature is to be used in conjunction with tunnels in collect metadata
> (external) mode. As I understand it there are three tunnel netdevs that use
> options metadata in the kernel at this time.
>
> * Geneve
>
>   In the case of Geneve options are TLVs[1]. My reading is that in collect
>   metadata mode the kernel does not appear to do anything other than pass
>   them around as a bytestring.
>
>   [1] https://tools.ietf.org/html/draft-ietf-nvo3-geneve-05#section-3.5
>
> * VXLAN-GBP
>
>   In the case of VXLAN-GBP on RX in collect metadata mode options are used
>   to carry information parsed in vxlan_parse_gbp_hdr() from the VXLAN Group
>   Based Policy Extension[2]. On RX the options data is used to create an
>   extension (header) by vxlan_build_gbp_hdr().
>
>   [2] https://tools.ietf.org/html/draft-smith-vxlan-group-policy-03#section-2.1
>
> * ERSPAN (GRE)
>
>   In the case of ERSPAN, which is a variant of GRE, on RX in collect
>   metadata mode options are used to carry the index parsed from the ERSPAN
>   Type II feature header[3] in erspan_rcv().  The converse is true on TX
>   and is handled by erspan_fb_xmit().
>
>   [3] https://tools.ietf.org/html/draft-foschiano-erspan-03#section-4.2
>
> Users of options:
>
> * There are eBPF hooks to allow getting on and setting tunnel metadata:
>   bpf_skb_set_tunnel_opt, bpf_skb_get_tunnel_opt.
>
> * Open vSwitch is able to match and set Geneve and VXLAN-GBP options.
>
> Neither of the above appear to assume any structure for the data.
>
> I hope the above is the kind of information you are after.

yeah, this helps, do we expect HW offloads to be able to work with
these options? e.g place/match on packets
or even do something beyond that?

Re: [oss-drivers] Re: [PATCH/RFC net-next 2/2] net/sched: allow flower to match tunnel options

[Simon Horman]

On Wed, Sep 13, 2017 at 03:38:01PM +0300, Or Gerlitz wrote:
> On Wed, Sep 13, 2017 at 2:59 PM, Simon Horman
> <simon.horman@netronome.com> wrote:
> > On Wed, Sep 13, 2017 at 01:03:43PM +0300, Or Gerlitz wrote:
> >> On Wed, Sep 13, 2017 at 12:25 PM, Simon Horman
> >> <simon.horman@netronome.com> wrote:
> >> > On Tue, Sep 12, 2017 at 11:23:55PM +0300, Or Gerlitz wrote:
> >> >> On Tue, Sep 12, 2017 at 5:20 PM, Simon Horman
> >> >> <simon.horman@netronome.com> wrote:
> >> >> > Allow matching on options in tunnel headers.
> >> >> > This makes use of existing tunnel metadata support.
> >> >>
> >> >> Simon,
> >> >>
> >> >> This patch is about matching on tunnel options, right? but
> >> >>
> >> >> > Options are a bytestring of up to 256 bytes.
> >> >> > Tunnel implementations may support less or more options,
> >> >> > or no options at all.
> >> >> >
> >> >> >  # ip link add name geneve0 type geneve dstport 0 external
> >> >> >  # tc qdisc add dev eth0 ingress
> >> >> >  # tc qdisc del dev eth0 ingress; 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 4789 \
> >> >> >             id 11 \
> >> >> >             opts 0102800100800022 \
> >> >> >     action mirred egress redirect dev geneve0
> >> >>
> >> >> the example here is on how to use tunnel options in the tunnel set key actions..
> >> >>
> >> >> And the other way around in the other patch... the patch is about the
> >> >> tunnel key set action and the example shows how to match that in
> >> >> flower... I guess you want to swap the relevant of the change log.
> >> >
> >> > Yes, it seems so. Sorry about that.
> >>
> >> no worries, you can do the swap, but before that
> >>
> >> >> Anyway, is there any human readable/understandable representation of
> >> >> these options? e.g what does 0102800100800022 means for geneve?
> >>
> >> > Thanks, I had not thought of that. My feeling is that could
> >> > be added to the tc tool as follow-up work.
> >>
> >> could you spend few words on the nature of these options now when we review
> >> the kernel patches? I guess this is somehow related to protocols such
> >> as geneve and vxlan-gpe  -- it would be good if you elaborate on that
> >> a bit, does
> >> the kernel does any usage with these options beyond matching on them or stiching
> >> them to packet headers?
> >
> > This feature is to be used in conjunction with tunnels in collect metadata
> > (external) mode. As I understand it there are three tunnel netdevs that use
> > options metadata in the kernel at this time.
> >
> > * Geneve
> >
> >   In the case of Geneve options are TLVs[1]. My reading is that in collect
> >   metadata mode the kernel does not appear to do anything other than pass
> >   them around as a bytestring.
> >
> >   [1] https://tools.ietf.org/html/draft-ietf-nvo3-geneve-05#section-3.5
> >
> > * VXLAN-GBP
> >
> >   In the case of VXLAN-GBP on RX in collect metadata mode options are used
> >   to carry information parsed in vxlan_parse_gbp_hdr() from the VXLAN Group
> >   Based Policy Extension[2]. On RX the options data is used to create an
> >   extension (header) by vxlan_build_gbp_hdr().
> >
> >   [2] https://tools.ietf.org/html/draft-smith-vxlan-group-policy-03#section-2.1
> >
> > * ERSPAN (GRE)
> >
> >   In the case of ERSPAN, which is a variant of GRE, on RX in collect
> >   metadata mode options are used to carry the index parsed from the ERSPAN
> >   Type II feature header[3] in erspan_rcv().  The converse is true on TX
> >   and is handled by erspan_fb_xmit().
> >
> >   [3] https://tools.ietf.org/html/draft-foschiano-erspan-03#section-4.2
> >
> > Users of options:
> >
> > * There are eBPF hooks to allow getting on and setting tunnel metadata:
> >   bpf_skb_set_tunnel_opt, bpf_skb_get_tunnel_opt.
> >
> > * Open vSwitch is able to match and set Geneve and VXLAN-GBP options.
> >
> > Neither of the above appear to assume any structure for the data.
> >
> > I hope the above is the kind of information you are after.
> 
> yeah, this helps, do we expect HW offloads to be able to work with
> these options? e.g place/match on packets
> or even do something beyond that?

Perhaps unsurprisingly given my posting I would like to offload
"place/match" of options. I do not currently have any plans beyond that
and my crystal ball broke some time ago.