Re: [PATCH v2 net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key
On Mon, 2 Oct 2017 09:50:15 +0200, Simon Horman wrote: > I believe that in order to avoid per-packet overhead and at the same time > code complexity the TLVs should be described in-order. So matching on > TLV-A,TLV-B,TLV-C would be a different match to TLV-C,TLV-A,TLV-B. An > order-independent match could be added if desired in future. Although better than the binary format, I doubt that it would be useful. I can't imagine a real use case where you would want such match. Instead, what you want is a match on a particular TLV, wherever it is in the data. For start, we can support just a single TLV. I.e. when matching on TLV-A, all of these would match: TLV-A,TLV-B,TLV-C; TLV-B,TLV-A,TLV-C; TLV-B,TLV-C,TLV-A. And this one won't match: TLV-B,TLV-C,TLV-D. Jiri
Re: [PATCH v2 net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key
On Fri, Sep 29, 2017 at 05:54:23AM +0100, David Miller wrote: > From: Simon Horman > Date: Wed, 27 Sep 2017 10:16:32 +0200 > > > 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 really worry about this. > > These metadata option blobs are internal kernel datastructure which we > could change at any point in time. They are not exported to > userspace as a UAPI. > > It's kinda OK for eBPF programs to access this stuff since they are > expected to cope with changes to internal data-structures. > > But for anything user facing, this really doesn't work. Hi Dave, Hi Jiri, the feedback I got from Jiri is that there needs to be some exposure of TLVs. What I have in mind is to describe Geneve option TLVs in the UAPI and for the kernel - most likely cls_flower, possibly using helpers, to translate between that encoding and the one used internally by the kernel - which currently happens to be the on-the-wire format. I believe that in order to avoid per-packet overhead and at the same time code complexity the TLVs should be described in-order. So matching on TLV-A,TLV-B,TLV-C would be a different match to TLV-C,TLV-A,TLV-B. An order-independent match could be added if desired in future. This would mean the feature is initially restricted to Geneve but could be expended to offer a similar feature for other encapsulation protocols as the need arises. Would this address your concerns?
Re: [PATCH v2 net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key
From: Simon Horman Date: Wed, 27 Sep 2017 10:16:32 +0200 > 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 really worry about this. These metadata option blobs are internal kernel datastructure which we could change at any point in time. They are not exported to userspace as a UAPI. It's kinda OK for eBPF programs to access this stuff since they are expected to cope with changes to internal data-structures. But for anything user facing, this really doesn't work.
Re: [PATCH v2 net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key
On Wed, 27 Sep 2017 10:16:32 +0200, Simon Horman wrote: > * 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 [...] > > Neither of the above appear to assume any structure for the data. But that's not true. Geneve uses TLVs, you even mentioned that yourself. Matching on a block of TLVs as a bytestring doesn't make sense. The TLV fields may be in any order. We need better matching here. Bytestring is useless for Geneve. NACK for this direction of option matching. We'd need to introduce matching on TLVs sooner or later anyway and this would be just a never used compat cruft that we need to keep around forever. Jiri
[PATCH v2 net-next 0/2] net/sched: support tunnel options in cls_flower and act_tunnel_key
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. Discussion stemming from review of RFC: 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. Changes since RFC: * Drop RFC prefix * Correct changelogs and enhance cover letter. 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
