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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170927110822.GD1944@nanopsycho.orion>
Date:   Wed, 27 Sep 2017 13:08:22 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Simon Horman <simon.horman@...ronome.com>
Cc:     David Miller <davem@...emloft.net>, Jiri Pirko <jiri@...lanox.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org,
        oss-drivers@...ronome.com
Subject: Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel
 options

Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@...ronome.com wrote:
>On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@...ronome.com wrote:
>> >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.
>> >
>> >e.g.
>> > # ip link add name geneve0 type geneve dstport 0 external
>> > # tc qdisc add 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@...ronome.com>
>> >Reviewed-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
>> >
>> >---
>> >v2
>> >* Correct example which was incorrectly described setting rather
>> >  than matching tunnel options
>> >---
>> > 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 */
>> 
>> I don't see the actual dissection implementation. Where is it?
>> Did you test the patchset?
>
>Yes, I did test it. But it is also possible something went astray along the
>way and I will retest.
>
>I think that the code you are looking for is in
>fl_classify() in this patch.

The dissection should be done in the flow_dissector. That's the whole
point in having it generic. You should move it there.