[{"id":1776165,"web_url":"http://patchwork.ozlabs.org/comment/1776165/","msgid":"<20170927091005.GB1944@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-27T09:10:05","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n>Allow matching on options in tunnel headers.\n>This makes use of existing tunnel metadata support.\n>\n>Options are a bytestring of up to 256 bytes.\n>Tunnel implementations may support less or more options,\n>or no options at all.\n>\n>e.g.\n> # ip link add name geneve0 type geneve dstport 0 external\n> # tc qdisc add dev geneve0 ingress\n> # tc filter add dev geneve0 protocol ip parent ffff: \\\n>     flower \\\n>       enc_src_ip 10.0.99.192 \\\n>       enc_dst_ip 10.0.99.193 \\\n>       enc_key_id 11 \\\n>       enc_opts 0102800100800020/fffffffffffffff0 \\\n>       ip_proto udp \\\n>       action mirred egress redirect dev eth1\n>\n>Signed-off-by: Simon Horman <simon.horman@netronome.com>\n>Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>\n>\n>---\n>v2\n>* Correct example which was incorrectly described setting rather\n>  than matching tunnel options\n>---\n> include/net/flow_dissector.h | 13 +++++++++++++\n> include/uapi/linux/pkt_cls.h |  3 +++\n> net/sched/cls_flower.c       | 35 ++++++++++++++++++++++++++++++++++-\n> 3 files changed, 50 insertions(+), 1 deletion(-)\n>\n>diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h\n>index fc3dce730a6b..43f98bf0b349 100644\n>--- a/include/net/flow_dissector.h\n>+++ b/include/net/flow_dissector.h\n>@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {\n> \t__u8\tttl;\n> };\n> \n>+/**\n>+ * struct flow_dissector_key_enc_opts:\n>+ * @data: data\n>+ * @len: len\n>+ */\n>+struct flow_dissector_key_enc_opts {\n>+\tu8 data[256];\t/* Using IP_TUNNEL_OPTS_MAX is desired here\n>+\t\t\t * but seems difficult to #include\n>+\t\t\t */\n>+\tu8 len;\n>+};\n>+\n> enum flow_dissector_key_id {\n> \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n>@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n>+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n\nI don't see the actual dissection implementation. Where is it?\nDid you test the patchset?","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"xKFzi8m2\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2BqT2vTPz9tXf\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 19:10:13 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752686AbdI0JKJ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 05:10:09 -0400","from mail-wm0-f66.google.com ([74.125.82.66]:43228 \"EHLO\n\tmail-wm0-f66.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752611AbdI0JKH (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 05:10:07 -0400","by mail-wm0-f66.google.com with SMTP id m72so19281265wmc.0\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 02:10:07 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\t81sm4017913wmi.17.2017.09.27.02.10.05\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 27 Sep 2017 02:10:05 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=4Xju5ovKV8QPg6CQay7cKaVeIVpFy8LMG2Jyk68q7gs=;\n\tb=xKFzi8m2PFz3nZwGkKa/k1UGo6+P2A7L78tk4Pn5wvr0nz9Gj0ncEDkPtksvXdQFkA\n\t5YETCCXfDrxqpgeraLM4LHdJpBc7KBakz9hYQCshf4c6ZVWBFtIHOhnfVVNvMbMz++N+\n\tfK954YIw00kY+SKZnSW6VR8PXYr1NiQuCr87zd/KzhE7QktV/o7kbpTvOBxMksg4i8xm\n\tVgwbi3Ec59ck+aHjHdZDLhPEFviqqImGgmfVrdsIwnyMkDB2tz5RjfpT2NQLhrfuAaQR\n\t//Au5ARyzuIZFsl6KdNTyZujkowZJ35Uj57iMbQYPLcKGSsdZ64XjNQFrjKUwa0BlbnC\n\toJ0g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=4Xju5ovKV8QPg6CQay7cKaVeIVpFy8LMG2Jyk68q7gs=;\n\tb=rNQy5/NnUgRpLAW4Vb9ZstsbtS/qUS4o9ArcwNxN5zSBMXBnghKgwpgCWoHHLBLH/9\n\tselMPfXmKAUHH6bo7mM+pXKAKpbMzni9sZHlLdUr2iT8ry9s36f9HMS76neIkI5c0zaQ\n\tjyDOZCYoF0kjuNYJPlPiEydTIXIjYL/J3PSvDcB+8J3+lORF3bMfDqtulDppRZPu/DMK\n\twbj8854mfVpmJlIyuXV4K/HW2GVmYV7TNz8ye202w6bDM4hTxnMLipG1bLVKuxgaDmiL\n\tSZNit6IbsRID+mTjnvltV/u/neo1xDWpsB8T89SP5fCLPz/K2BgJxYejkxTkSVmyE8m9\n\t1Mtw==","X-Gm-Message-State":"AHPjjUhUQXmYDrE6hCq8wzz2SQxgqGF/XTiNtYfxzV5um5n4JCcHi7dO\n\t9azf8o5PqMtrdkBJxgkheGHydg==","X-Google-Smtp-Source":"AOwi7QACMgs7jHIrQ5SlgAN6Os4FBkZVQJ7IagjKrdm5bVAyFQswNc6KYKOYzGxrJJHZf8vR3Ydk/w==","X-Received":"by 10.28.62.65 with SMTP id l62mr891564wma.47.1506503406422;\n\tWed, 27 Sep 2017 02:10:06 -0700 (PDT)","Date":"Wed, 27 Sep 2017 11:10:05 +0200","From":"Jiri Pirko <jiri@resnulli.us>","To":"Simon Horman <simon.horman@netronome.com>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927091005.GB1944@nanopsycho.orion>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1506500194-17637-3-git-send-email-simon.horman@netronome.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776179,"web_url":"http://patchwork.ozlabs.org/comment/1776179/","msgid":"<20170927092732.GC25449@vergenet.net>","list_archive_url":null,"date":"2017-09-27T09:27:33","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n> >Allow matching on options in tunnel headers.\n> >This makes use of existing tunnel metadata support.\n> >\n> >Options are a bytestring of up to 256 bytes.\n> >Tunnel implementations may support less or more options,\n> >or no options at all.\n> >\n> >e.g.\n> > # ip link add name geneve0 type geneve dstport 0 external\n> > # tc qdisc add dev geneve0 ingress\n> > # tc filter add dev geneve0 protocol ip parent ffff: \\\n> >     flower \\\n> >       enc_src_ip 10.0.99.192 \\\n> >       enc_dst_ip 10.0.99.193 \\\n> >       enc_key_id 11 \\\n> >       enc_opts 0102800100800020/fffffffffffffff0 \\\n> >       ip_proto udp \\\n> >       action mirred egress redirect dev eth1\n> >\n> >Signed-off-by: Simon Horman <simon.horman@netronome.com>\n> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>\n> >\n> >---\n> >v2\n> >* Correct example which was incorrectly described setting rather\n> >  than matching tunnel options\n> >---\n> > include/net/flow_dissector.h | 13 +++++++++++++\n> > include/uapi/linux/pkt_cls.h |  3 +++\n> > net/sched/cls_flower.c       | 35 ++++++++++++++++++++++++++++++++++-\n> > 3 files changed, 50 insertions(+), 1 deletion(-)\n> >\n> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h\n> >index fc3dce730a6b..43f98bf0b349 100644\n> >--- a/include/net/flow_dissector.h\n> >+++ b/include/net/flow_dissector.h\n> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {\n> > \t__u8\tttl;\n> > };\n> > \n> >+/**\n> >+ * struct flow_dissector_key_enc_opts:\n> >+ * @data: data\n> >+ * @len: len\n> >+ */\n> >+struct flow_dissector_key_enc_opts {\n> >+\tu8 data[256];\t/* Using IP_TUNNEL_OPTS_MAX is desired here\n> >+\t\t\t * but seems difficult to #include\n> >+\t\t\t */\n> >+\tu8 len;\n> >+};\n> >+\n> > enum flow_dissector_key_id {\n> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n> \n> I don't see the actual dissection implementation. Where is it?\n> Did you test the patchset?\n\nYes, I did test it. But it is also possible something went astray along the\nway and I will retest.\n\nI think that the code you are looking for is in\nfl_classify() in this patch.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"b86ROQaI\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2CCk12yLz9tXT\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 19:27:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752638AbdI0J1m (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 05:27:42 -0400","from mail-wr0-f177.google.com ([209.85.128.177]:53905 \"EHLO\n\tmail-wr0-f177.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752281AbdI0J1j (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 05:27:39 -0400","by mail-wr0-f177.google.com with SMTP id 54so710510wrz.10\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 02:27:39 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id\n\tq19sm14395331wrb.17.2017.09.27.02.27.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 27 Sep 2017 02:27:37 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=NmeslxYPlbdr7rHQv94dFYsRJQp//DP6QDNKJ+OXKT8=;\n\tb=b86ROQaIadRVmmi9/tU/WVI369N/8+wkmrdDnp2bCdE5kry7QMyUPfy+T2RszHcBHL\n\t/UkAT78/3RVr7haYzVBOILFMZmgz9+1twExc/O6FXMqjTo1F9K30LAjPYd7NyiaJ5a5e\n\t7JbbkT6ZxMvgttD0HT8PanS+UMbO7S2L6UvSW3XAEDmMs9jinwJRlFR1J9ysQPHLTU1n\n\tQMNzh3sHjLg2SSo23pT+NNkORsE+InRW39M/O7q7PCSQRp3A2ZsLznt7nMPULEPX3oyV\n\tEE3Rr5jQ/krIrxeBMs0iHtqmPc1lTB4hlpsQgyYicneqrZEnDaJj7UZUyEUYDBN8Xbos\n\tD/BA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=NmeslxYPlbdr7rHQv94dFYsRJQp//DP6QDNKJ+OXKT8=;\n\tb=ZKvMiBLLdYxNIMB4FNwBMa7d7aPBW5EYOeSfh6L3s7OSJT87q9pdLsaSs6WXh0cAVM\n\tzgzNY7F5bedEUFKMjfYKWLoRnnmqFURH8pikh1Aymxr2b9b/l/fkeJVhjrKuarFGqpVG\n\t4R2XHlWhEsDJ+jGb7B4LKSmy0pAIboKD8no48yNt5MaCuZWjOkOPboUKDHt3fcouVOg+\n\tC/+wicQm3t6rQMv4OzXhLCtUmBhklFPhvKZHmwdo2c/BoM4FGY/NrB1hX3IeyRiLzxvd\n\t55Xkzv365Mo1Z1YBZCriareRiswORwUm26HGO1cxjR4MQSzm4q7xJfYKdg1sYJ4iO7Yi\n\tMvSA==","X-Gm-Message-State":"AHPjjUhB8rvzUzFXX6jDrCxF8i4WY59/8nMXvUbEcWNnGySA2Rn9vlpV\n\ttrP5RdZpNydDEOQO2XM+cWsJpw==","X-Google-Smtp-Source":"AOwi7QBRVIWUrWwbEVv3YWKhoIMFDYIx3qh1VpCM7RyLtz3CAqyEUPe6YIePTU+gLbgrLN9wz0/1eQ==","X-Received":"by 10.223.198.130 with SMTP id j2mr867584wrg.52.1506504458363;\n\tWed, 27 Sep 2017 02:27:38 -0700 (PDT)","Date":"Wed, 27 Sep 2017 11:27:33 +0200","From":"Simon Horman <simon.horman@netronome.com>","To":"Jiri Pirko <jiri@resnulli.us>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927092732.GC25449@vergenet.net>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927091005.GB1944@nanopsycho.orion>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776252,"web_url":"http://patchwork.ozlabs.org/comment/1776252/","msgid":"<20170927110822.GD1944@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-27T11:08:22","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n>On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n>> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n>> >Allow matching on options in tunnel headers.\n>> >This makes use of existing tunnel metadata support.\n>> >\n>> >Options are a bytestring of up to 256 bytes.\n>> >Tunnel implementations may support less or more options,\n>> >or no options at all.\n>> >\n>> >e.g.\n>> > # ip link add name geneve0 type geneve dstport 0 external\n>> > # tc qdisc add dev geneve0 ingress\n>> > # tc filter add dev geneve0 protocol ip parent ffff: \\\n>> >     flower \\\n>> >       enc_src_ip 10.0.99.192 \\\n>> >       enc_dst_ip 10.0.99.193 \\\n>> >       enc_key_id 11 \\\n>> >       enc_opts 0102800100800020/fffffffffffffff0 \\\n>> >       ip_proto udp \\\n>> >       action mirred egress redirect dev eth1\n>> >\n>> >Signed-off-by: Simon Horman <simon.horman@netronome.com>\n>> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>\n>> >\n>> >---\n>> >v2\n>> >* Correct example which was incorrectly described setting rather\n>> >  than matching tunnel options\n>> >---\n>> > include/net/flow_dissector.h | 13 +++++++++++++\n>> > include/uapi/linux/pkt_cls.h |  3 +++\n>> > net/sched/cls_flower.c       | 35 ++++++++++++++++++++++++++++++++++-\n>> > 3 files changed, 50 insertions(+), 1 deletion(-)\n>> >\n>> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h\n>> >index fc3dce730a6b..43f98bf0b349 100644\n>> >--- a/include/net/flow_dissector.h\n>> >+++ b/include/net/flow_dissector.h\n>> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {\n>> > \t__u8\tttl;\n>> > };\n>> > \n>> >+/**\n>> >+ * struct flow_dissector_key_enc_opts:\n>> >+ * @data: data\n>> >+ * @len: len\n>> >+ */\n>> >+struct flow_dissector_key_enc_opts {\n>> >+\tu8 data[256];\t/* Using IP_TUNNEL_OPTS_MAX is desired here\n>> >+\t\t\t * but seems difficult to #include\n>> >+\t\t\t */\n>> >+\tu8 len;\n>> >+};\n>> >+\n>> > enum flow_dissector_key_id {\n>> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n>> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n>> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n>> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n>> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n>> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n>> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n>> \n>> I don't see the actual dissection implementation. Where is it?\n>> Did you test the patchset?\n>\n>Yes, I did test it. But it is also possible something went astray along the\n>way and I will retest.\n>\n>I think that the code you are looking for is in\n>fl_classify() in this patch.\n\nThe dissection should be done in the flow_dissector. That's the whole\npoint in having it generic. You should move it there.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"vtZa6eXG\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2FSV5y5bz9sNr\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 21:08:58 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751973AbdI0LI0 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 07:08:26 -0400","from mail-wr0-f193.google.com ([209.85.128.193]:32918 \"EHLO\n\tmail-wr0-f193.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750703AbdI0LIZ (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 07:08:25 -0400","by mail-wr0-f193.google.com with SMTP id b9so4327958wra.0\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 04:08:24 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\tm28sm1788358wmi.20.2017.09.27.04.08.22\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 27 Sep 2017 04:08:23 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=97cUErP0p3HqvDqnV4BzaXQNE7HMIJ26W94iGRDUEEM=;\n\tb=vtZa6eXG0Cno41mAZeG8WvCJIEN7F8tEOOCkoOvh8JRBrP5+iGGIhQvmi44TGjrJ7O\n\tdVrYlstUiyMryFLM0xqs6lhaJR/QprQ+ImwnV9Sje8OhOVKI7Ye7+cCWwyogjB9Ln+ZD\n\trGGhqj9p/VF26UGO0Qix+YYehCb7lw442tEcaOHYUGNB7okv8tb+2Es1X2lRE97/9Kyv\n\t42/k656IK2Kxe+lHyl03u2jSpVj7W1MbLYQKvmBenx2tJrgvdOtc6DYXNfobhMCnLJuq\n\t6lZsuxumpuRiW/0yZb4GUW9SmizIcepLnDelbFJxCYdMfnmaKPcuC8MEmO7KuaWp2/NE\n\t80Gw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=97cUErP0p3HqvDqnV4BzaXQNE7HMIJ26W94iGRDUEEM=;\n\tb=lNPSyaKPzE/i5KWhgUAgyNSF7nqV/OWwmnkeMBZI4xOH9GvkCwSNA/nrBDyXRtzv6Q\n\tPnaGWWBt9UqeaiB9wT03MJsooNVz2gXLWniRLfv1z55pZlHJy/AhH3GYJv3Xgq/CSQ61\n\tsMiCW8m3wdX91rpISUwRv+2NOu0mrFTPNaf2xzXt03EKIvDonMtksJBv4MmxkOm/caUU\n\t+8QjWx8icqTEXx1PPrcdoCrdZ7GudgaC1vkiRi+0aN78rJo16iufQtzY6HiN38jJTQFU\n\t0dNWNjbDqAmAwT8IfMWm0ew44DvRNO85DcJGtt3XT2lcV0gfiiI1wFOxCdnJANwbB9RX\n\tUgvw==","X-Gm-Message-State":"AHPjjUiR31iwP8ctUHGgKkdo97nY2LLDx5rYHzmWFb0ZR1yojGK6hHrS\n\tRKh0hA1l/3GUbAirJqLVKr0T8A==","X-Google-Smtp-Source":"AOwi7QCG2UAvEtLEXQamRzWMUEQW9SIUj1Zg/QYmXKBx3W0Xc3uDHsRjQEQ/ewlet3NWwEXaLaaZSQ==","X-Received":"by 10.223.171.21 with SMTP id q21mr1089252wrc.125.1506510503991; \n\tWed, 27 Sep 2017 04:08:23 -0700 (PDT)","Date":"Wed, 27 Sep 2017 13:08:22 +0200","From":"Jiri Pirko <jiri@resnulli.us>","To":"Simon Horman <simon.horman@netronome.com>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927110822.GD1944@nanopsycho.orion>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927092732.GC25449@vergenet.net>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776279,"web_url":"http://patchwork.ozlabs.org/comment/1776279/","msgid":"<20170927115412.GA16020@vergenet.net>","list_archive_url":null,"date":"2017-09-27T11:54:14","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n> >> >Allow matching on options in tunnel headers.\n> >> >This makes use of existing tunnel metadata support.\n> >> >\n> >> >Options are a bytestring of up to 256 bytes.\n> >> >Tunnel implementations may support less or more options,\n> >> >or no options at all.\n> >> >\n> >> >e.g.\n> >> > # ip link add name geneve0 type geneve dstport 0 external\n> >> > # tc qdisc add dev geneve0 ingress\n> >> > # tc filter add dev geneve0 protocol ip parent ffff: \\\n> >> >     flower \\\n> >> >       enc_src_ip 10.0.99.192 \\\n> >> >       enc_dst_ip 10.0.99.193 \\\n> >> >       enc_key_id 11 \\\n> >> >       enc_opts 0102800100800020/fffffffffffffff0 \\\n> >> >       ip_proto udp \\\n> >> >       action mirred egress redirect dev eth1\n> >> >\n> >> >Signed-off-by: Simon Horman <simon.horman@netronome.com>\n> >> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>\n> >> >\n> >> >---\n> >> >v2\n> >> >* Correct example which was incorrectly described setting rather\n> >> >  than matching tunnel options\n> >> >---\n> >> > include/net/flow_dissector.h | 13 +++++++++++++\n> >> > include/uapi/linux/pkt_cls.h |  3 +++\n> >> > net/sched/cls_flower.c       | 35 ++++++++++++++++++++++++++++++++++-\n> >> > 3 files changed, 50 insertions(+), 1 deletion(-)\n> >> >\n> >> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h\n> >> >index fc3dce730a6b..43f98bf0b349 100644\n> >> >--- a/include/net/flow_dissector.h\n> >> >+++ b/include/net/flow_dissector.h\n> >> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip {\n> >> > \t__u8\tttl;\n> >> > };\n> >> > \n> >> >+/**\n> >> >+ * struct flow_dissector_key_enc_opts:\n> >> >+ * @data: data\n> >> >+ * @len: len\n> >> >+ */\n> >> >+struct flow_dissector_key_enc_opts {\n> >> >+\tu8 data[256];\t/* Using IP_TUNNEL_OPTS_MAX is desired here\n> >> >+\t\t\t * but seems difficult to #include\n> >> >+\t\t\t */\n> >> >+\tu8 len;\n> >> >+};\n> >> >+\n> >> > enum flow_dissector_key_id {\n> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n> >> \n> >> I don't see the actual dissection implementation. Where is it?\n> >> Did you test the patchset?\n> >\n> >Yes, I did test it. But it is also possible something went astray along the\n> >way and I will retest.\n> >\n> >I think that the code you are looking for is in\n> >fl_classify() in this patch.\n> \n> The dissection should be done in the flow_dissector. That's the whole\n> point in having it generic. You should move it there.\n> \n\nThanks, will do.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"D37qmd9g\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2GSz6M2Mz9sNV\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 21:54:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752674AbdI0LyZ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 07:54:25 -0400","from mail-qk0-f181.google.com ([209.85.220.181]:54611 \"EHLO\n\tmail-qk0-f181.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1750854AbdI0LyX (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 07:54:23 -0400","by mail-qk0-f181.google.com with SMTP id d70so12953091qkc.11\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 04:54:23 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id\n\tt184sm8337743qkc.74.2017.09.27.04.54.19\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 27 Sep 2017 04:54:22 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=9vADiTKGhtkHxy7lcsBdVcMA1qKvoNPN+lUd1BQtc4A=;\n\tb=D37qmd9gSEn8OWlkD7EzKzg114FBhcCdU6kMGv5vSqcoSq01Mb4505zNcG3k1oTiF1\n\tQSWPHj1gbrAYNba6b/WkOuVNBINreD6bubYjDkj49aY5gljZ9Wpv/FTxyNvTsC3xpmmn\n\tDi0VR79LfC3VAFOjnYsAXYR2EdF4NkuLjWAGj7+l+roNHwZar4e5N7n9me3XD/8Uud8C\n\tASlIw0L7NRnENQys76uf/RRvfXKbvvtwriTKeNZgT+EGHCClIE+IsrKqH0RdZn7/lVNk\n\tpchRybjC4w9C//0UPPvkAUNKJ7tJmuNW/t6+pnP6KC9ifajxMxADt6hyavYtQAYnkbEq\n\tpzDA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=9vADiTKGhtkHxy7lcsBdVcMA1qKvoNPN+lUd1BQtc4A=;\n\tb=VKy8/VO7IwbowK2k94y4YBLLlbxuxndgtlz4JtBB9FEleWBPy0CAP2NviMmD2dJ8rU\n\tp4JnHdfIx3vuw5mCjM+kVU30xZmCOSKa/ejG26lkyHWKa9ZLogzHEihWvYapaCZru/yX\n\tncPA80nK1/alOLPUoBq+QvGIb0RMkuDHWUD0IKYzVrMkOhMzq7CxCnL6VqghB98WNm+O\n\tgUlre3zeO9YTd+OZRIfymkcy+RlsncgMH2sDXwKvelwJQMTxf2GO7mOpHBE5bLmSoZku\n\tkmCfpC8ear+yqzmiq0PZ7kyMvk3ReM+mFFXXS9Log35C5n0G7Ke8NV0cjy1cwgpx4Zu1\n\tuNCg==","X-Gm-Message-State":"AMCzsaUvl3KZixZqjjGB18lHW8j61OehgKqNwirld9X4C8XLchnIw3sI\n\txpsaEOQfcBEJV58Y38NNSh4P8w==","X-Google-Smtp-Source":"AOwi7QC0Nj/Wy5DgNYhWJ6s59Eu3JCy1grQGrAsoHDCpcGOEJWVmNgWnh7KiI9NbIh5slvQX6iAd8w==","X-Received":"by 10.55.198.202 with SMTP id s71mr1678329qkl.143.1506513262764; \n\tWed, 27 Sep 2017 04:54:22 -0700 (PDT)","Date":"Wed, 27 Sep 2017 13:54:14 +0200","From":"Simon Horman <simon.horman@netronome.com>","To":"Jiri Pirko <jiri@resnulli.us>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927115412.GA16020@vergenet.net>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927110822.GD1944@nanopsycho.orion>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776318,"web_url":"http://patchwork.ozlabs.org/comment/1776318/","msgid":"<20170927125205.GA30000@vergenet.net>","list_archive_url":null,"date":"2017-09-27T12:52:06","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n\n...\n\n> >> > enum flow_dissector_key_id {\n> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n> >> \n> >> I don't see the actual dissection implementation. Where is it?\n> >> Did you test the patchset?\n> >\n> >Yes, I did test it. But it is also possible something went astray along the\n> >way and I will retest.\n> >\n> >I think that the code you are looking for is in\n> >fl_classify() in this patch.\n> \n> The dissection should be done in the flow_dissector. That's the whole\n> point in having it generic. You should move it there.\n\nComing back to this after lunch, I believe what I have done in this patch\nis consistent with handling of other enc fields, which are set in\nfl_classify() rather than the dissector. In particular the ip_tunnel_info,\nwhich is used by this patch, is already used in fl_classify().\n\nWithout this patch I see:\n\n\nstatic int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,\n                       struct tcf_result *res)\n{\n        ...\n        struct ip_tunnel_info *info;\n\n        ...\n\n        info = skb_tunnel_info(skb);\n        if (info) {\n                struct ip_tunnel_key *key = &info->key;\n\n                switch (ip_tunnel_info_af(info)) {\n                case AF_INET:\n                        skb_key.enc_control.addr_type =\n                                FLOW_DISSECTOR_KEY_IPV4_ADDRS;\n                        skb_key.enc_ipv4.src = key->u.ipv4.src;\n                        skb_key.enc_ipv4.dst = key->u.ipv4.dst;\n                        break;\n                case AF_INET6:\n                        skb_key.enc_control.addr_type =\n                                FLOW_DISSECTOR_KEY_IPV6_ADDRS;\n                        skb_key.enc_ipv6.src = key->u.ipv6.src;\n                        skb_key.enc_ipv6.dst = key->u.ipv6.dst;\n                        break;\n                }\n\n                skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);\n                skb_key.enc_tp.src = key->tp_src;\n                skb_key.enc_tp.dst = key->tp_dst;\n        }\n\n\t...\n}\n\nThis patch adds the following inside the if() clause above:\n\n\tif (info->options_len) {\n\t\tskb_key.enc_opts.len = info->options_len;\n\t\tip_tunnel_info_opts_get(skb_key.enc_opts.data, info);\n\t}","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"n1hIPx3t\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2Hlj5LL9z9tXn\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 22:52:17 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752406AbdI0MwP (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 08:52:15 -0400","from mail-qk0-f174.google.com ([209.85.220.174]:56331 \"EHLO\n\tmail-qk0-f174.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752104AbdI0MwN (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 08:52:13 -0400","by mail-qk0-f174.google.com with SMTP id g128so13126395qke.13\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 05:52:13 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id\n\ta13sm8583884qkj.35.2017.09.27.05.52.10\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 27 Sep 2017 05:52:12 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=MWLIbGfVnpOUUR3IxHt7E9/PHkMvatoQqWXBgH1LwC0=;\n\tb=n1hIPx3tg0/njx1l6cuimsta+orrwPGLtfv/XuXjuqGsji439LZnmncd264z6fy1Gg\n\tF6WbWhxk7omOy6EurGLDo3jKhET+JYUXiGcLybOaHT23zaz4O3XKrmhQu/h8v8Y7Cacw\n\tuLJTulwHp4QMyql/KWDn7WZxfbAQKTGwbZGvM/bt7OcblDbONJzLC0CnDPdu5MeaoAr6\n\torbfu4PubtoUZxUBRK2TB3IL2FRf4gvkwG+MGmFMwAu0in7Zj3QUHdGfsfYIEL9yiRYg\n\tvOh6zKlTQ3rt6vpqiKwLqzgYOL8MEkrBU3Uc/f7TqyMA1omJM32I/KEzv5tM4wlUV5UA\n\tRJEg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=MWLIbGfVnpOUUR3IxHt7E9/PHkMvatoQqWXBgH1LwC0=;\n\tb=J2ZY9hfKk3PGV+0gYS7Q0wKW2FCDgNDVIasW0kJE2SP5p0hLVj7KZWtPtpJnZl/gez\n\tbNHPSHuf53kKTldSN9RC/jOJ/BKDoY+8lUeMuzi6W1sWWL6EalX2eGJchj4sxbrUqCw5\n\tFX+D00OiQpmySL4gKk2M+hEXM13eigXQ0akY8LRV3K0AXZ6Y8qwZErDih4u0S42P0YpM\n\tSk5FKCJtJibABENxcJT4Ahdti3W+7uf7XzswUFhextwYO3cIL04X4dqZy5zrazDakiCE\n\t4gXJ5M0wXDHhlltnpu+ydY/8CJiar1fyTqZAG6OdYq5frKsynsn+9NqS7uMXXvuBFiMK\n\tc/pg==","X-Gm-Message-State":"AMCzsaWfwwjr/qjvXEzb/a7/lpuFZMrFRrncandZH/G7ps0v1bwKE4Ni\n\tyQ7GBYM/oP5CN8IH8PoCtIRd/Q==","X-Google-Smtp-Source":"AOwi7QBPeZ+OjNt1QKai8d+hpUip3gDWcwErccoHRbf/KhTbpx7Zx7Ib+iRMt9Q/LsAVWt+LTg95bA==","X-Received":"by 10.55.214.208 with SMTP id p77mr248846qkl.112.1506516732896; \n\tWed, 27 Sep 2017 05:52:12 -0700 (PDT)","Date":"Wed, 27 Sep 2017 14:52:06 +0200","From":"Simon Horman <simon.horman@netronome.com>","To":"Jiri Pirko <jiri@resnulli.us>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927125205.GA30000@vergenet.net>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927110822.GD1944@nanopsycho.orion>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776320,"web_url":"http://patchwork.ozlabs.org/comment/1776320/","msgid":"<20170927125603.GH1944@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-27T12:56:03","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n>On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n>> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n>> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n>> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n>\n>...\n>\n>> >> > enum flow_dissector_key_id {\n>> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n>> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n>> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n>> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n>> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n>> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n>> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n>> >> \n>> >> I don't see the actual dissection implementation. Where is it?\n>> >> Did you test the patchset?\n>> >\n>> >Yes, I did test it. But it is also possible something went astray along the\n>> >way and I will retest.\n>> >\n>> >I think that the code you are looking for is in\n>> >fl_classify() in this patch.\n>> \n>> The dissection should be done in the flow_dissector. That's the whole\n>> point in having it generic. You should move it there.\n>\n>Coming back to this after lunch, I believe what I have done in this patch\n>is consistent with handling of other enc fields, which are set in\n>fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n>which is used by this patch, is already used in fl_classify().\n\nThat means the current code is wrong. The dissection should be done in\nflow_dissector, not in fl_classify.\n\n\n\n>\n>Without this patch I see:\n>\n>\n>static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,\n>                       struct tcf_result *res)\n>{\n>        ...\n>        struct ip_tunnel_info *info;\n>\n>        ...\n>\n>        info = skb_tunnel_info(skb);\n>        if (info) {\n>                struct ip_tunnel_key *key = &info->key;\n>\n>                switch (ip_tunnel_info_af(info)) {\n>                case AF_INET:\n>                        skb_key.enc_control.addr_type =\n>                                FLOW_DISSECTOR_KEY_IPV4_ADDRS;\n>                        skb_key.enc_ipv4.src = key->u.ipv4.src;\n>                        skb_key.enc_ipv4.dst = key->u.ipv4.dst;\n>                        break;\n>                case AF_INET6:\n>                        skb_key.enc_control.addr_type =\n>                                FLOW_DISSECTOR_KEY_IPV6_ADDRS;\n>                        skb_key.enc_ipv6.src = key->u.ipv6.src;\n>                        skb_key.enc_ipv6.dst = key->u.ipv6.dst;\n>                        break;\n>                }\n>\n>                skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);\n>                skb_key.enc_tp.src = key->tp_src;\n>                skb_key.enc_tp.dst = key->tp_dst;\n>        }\n>\n>\t...\n>}\n>\n>This patch adds the following inside the if() clause above:\n>\n>\tif (info->options_len) {\n>\t\tskb_key.enc_opts.len = info->options_len;\n>\t\tip_tunnel_info_opts_get(skb_key.enc_opts.data, info);\n>\t}\n>\n>","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"JqcO48oZ\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2HrB4PTjz9tXf\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 22:56:10 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752796AbdI0M4H (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 08:56:07 -0400","from mail-wm0-f68.google.com ([74.125.82.68]:48578 \"EHLO\n\tmail-wm0-f68.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752104AbdI0M4G (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 08:56:06 -0400","by mail-wm0-f68.google.com with SMTP id m127so17916484wmm.3\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 05:56:05 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\tl15sm5748949wmd.23.2017.09.27.05.56.04\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 27 Sep 2017 05:56:04 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=jzMojgzMJ3K1abR+uzUcD2dhBLRD9jKgOCSRDy+KUaU=;\n\tb=JqcO48oZF198pFWpXQiCGJwo1TiR51pEVArtm7UDx3w12d6SOb7TVbTCBTVhUslCKR\n\tjE7XXgSMXy4+7sgHvmzNE27DXFoKsKpjR+O8Aaeypi/55+AKe3TRPB9EPow76UBH/zxR\n\tOt15yTQCoscCaPP4qyAtYhLHtm3T+19lSkMaRKwm3OpMqH2s77XYxCEXLilH+JEPusgD\n\tV33xRxiksrBi273SlaKIrtw9MiSbUaBslqCYmTukd5LzetpracqOaR7V8v7jpDUx7Hlq\n\telQfMt+iIvZ62T4XZ1QHTMHOwthLfJSxCmiyPTznUS7LEqE0lDKI8eyFsr7NXDNSX2Th\n\t4/tg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=jzMojgzMJ3K1abR+uzUcD2dhBLRD9jKgOCSRDy+KUaU=;\n\tb=pVj3FjMMYPspcub+FPTvFDQWOXcecW0W3bhBy34rksWsDNxdSQos1LLNu2qHCwd75F\n\trvBG/EWN+TqJv5Ah8lDA8YDQRsFZL9nXuyl6sgsrw69QT4VBczTh7Qpd7APYEYNHq16o\n\tl5aEXlYmGIc1h7PsE7fVa4Nscuh9++tDovkAw0TjB03GS8q88+gy1AEVVKEGYzXiBo4H\n\tcD5a4sCe3CrRghwC6D0rzrMiTvIuIYM+iTd0XDQA4Fnr3Gewnr/wPNNXjMfTMXAgRlaO\n\tSEMg5N/aHA6k5RvCCkfojzaY/wlgwFE3S0sEmsdL9vttKGFAe7Aubi9YzS5dcyScj1BL\n\tI6TQ==","X-Gm-Message-State":"AHPjjUhdDXjhWomFeOj2CqnDcDPwSGHa84unA0pLqrB48MWBzde+DTjq\n\tIE97ZCeLEs7Gy0/jQjM9kz1Vug==","X-Google-Smtp-Source":"AOwi7QA1F3y2NffDa1rvOZHvjAIXRTzCNeeMfIFTIpIiOyKF7g46x9B3yw11pgg93f9g4PnbFx1jpQ==","X-Received":"by 10.28.29.71 with SMTP id d68mr243598wmd.114.1506516964709;\n\tWed, 27 Sep 2017 05:56:04 -0700 (PDT)","Date":"Wed, 27 Sep 2017 14:56:03 +0200","From":"Jiri Pirko <jiri@resnulli.us>","To":"Simon Horman <simon.horman@netronome.com>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927125603.GH1944@nanopsycho.orion>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927125205.GA30000@vergenet.net>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776349,"web_url":"http://patchwork.ozlabs.org/comment/1776349/","msgid":"<20170927133731.GA14183@vergenet.net>","list_archive_url":null,"date":"2017-09-27T13:37:33","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote:\n> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n> >\n> >...\n> >\n> >> >> > enum flow_dissector_key_id {\n> >> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> >> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> >> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> >> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> >> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n> >> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n> >> >> \n> >> >> I don't see the actual dissection implementation. Where is it?\n> >> >> Did you test the patchset?\n> >> >\n> >> >Yes, I did test it. But it is also possible something went astray along the\n> >> >way and I will retest.\n> >> >\n> >> >I think that the code you are looking for is in\n> >> >fl_classify() in this patch.\n> >> \n> >> The dissection should be done in the flow_dissector. That's the whole\n> >> point in having it generic. You should move it there.\n> >\n> >Coming back to this after lunch, I believe what I have done in this patch\n> >is consistent with handling of other enc fields, which are set in\n> >fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n> >which is used by this patch, is already used in fl_classify().\n> \n> That means the current code is wrong. The dissection should be done in\n> flow_dissector, not in fl_classify.\n\nWould an better approach be to move the fl_classify() below into, say,\nskb_flow_dissect_tunnel_info() and call that from fl_classify().\n\nThe reason I suggest this rather than moving the code into\n__skb_flow_dissect() is that currently flower assumes that tunnel_info\nis used if present. While I assume other users of () assume tunnel_info\nis not used even if present.\n\n> >Without this patch I see:\n> >\n> >\n> >static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,\n> >                       struct tcf_result *res)\n> >{\n> >        ...\n> >        struct ip_tunnel_info *info;\n> >\n> >        ...\n> >\n> >        info = skb_tunnel_info(skb);\n> >        if (info) {\n> >                struct ip_tunnel_key *key = &info->key;\n> >\n> >                switch (ip_tunnel_info_af(info)) {\n> >                case AF_INET:\n> >                        skb_key.enc_control.addr_type =\n> >                                FLOW_DISSECTOR_KEY_IPV4_ADDRS;\n> >                        skb_key.enc_ipv4.src = key->u.ipv4.src;\n> >                        skb_key.enc_ipv4.dst = key->u.ipv4.dst;\n> >                        break;\n> >                case AF_INET6:\n> >                        skb_key.enc_control.addr_type =\n> >                                FLOW_DISSECTOR_KEY_IPV6_ADDRS;\n> >                        skb_key.enc_ipv6.src = key->u.ipv6.src;\n> >                        skb_key.enc_ipv6.dst = key->u.ipv6.dst;\n> >                        break;\n> >                }\n> >\n> >                skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);\n> >                skb_key.enc_tp.src = key->tp_src;\n> >                skb_key.enc_tp.dst = key->tp_dst;\n> >        }\n> >\n> >\t...\n> >}\n> >\n> >This patch adds the following inside the if() clause above:\n> >\n> >\tif (info->options_len) {\n> >\t\tskb_key.enc_opts.len = info->options_len;\n> >\t\tip_tunnel_info_opts_get(skb_key.enc_opts.data, info);\n> >\t}\n> >\n> >","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"1lJi0ocR\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2Jm600hPz9sPr\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 23:37:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752835AbdI0Nhk (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 09:37:40 -0400","from mail-wr0-f174.google.com ([209.85.128.174]:50075 \"EHLO\n\tmail-wr0-f174.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752734AbdI0Nhj (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 09:37:39 -0400","by mail-wr0-f174.google.com with SMTP id h16so2297666wrf.6\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 06:37:38 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id\n\tz10sm1049933wre.6.2017.09.27.06.37.36\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 27 Sep 2017 06:37:37 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=NEbQR/KFJxMOE1ZmhG+H2ewZJopKf3n2UKe6DE23eKU=;\n\tb=1lJi0ocRAR0hFPvYVN0ptukUPJyt4WSpR1NKLflbOtrMsAL77uzP3Z85eg7AOVzgfs\n\tgJ1aqYboJfJQvcjAqIOfRZWK0Lnt0j3klMdbr2osBW2ghQMn3dWd0ZHRd0FPLgJW7va1\n\tO9I9yBlwF49CI54Ye+z+xFnNjSwdG/vYQTtNIhy4N3LWDgP25S0UXtqUU8GV0aFUVvmy\n\tZxxn0s+1ODewpINB+/D+L/s0rNehdnLACFmvPFNLko2MImvhiCIWKKB+w23n+yjltnxS\n\tFj3O8cKPlIS67a5ZC3F4t080akrx1f7q4CWMj4VYLXwEDYeMrAHqt4F7zE8NPZA22xJj\n\t0NLw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=NEbQR/KFJxMOE1ZmhG+H2ewZJopKf3n2UKe6DE23eKU=;\n\tb=d3sr7wSmyiIjicoQ/O6dapqK0pmDOAk4EEZKFwLgs00L4Vx3ilsPCn84EesREMHd+E\n\trrh/9IEoUbvZGLeM0NFIDMxnS79hM9WdDDoZo4jlWIYkkaRQWxaQp0/ygRnGalV5ss4Y\n\tsAMzM0zMFm4IJVWAaFKLtIX/Acmp4SwnwemIWxGmRwpFvUPuDUEmlB72pgcZu5619J/S\n\tf8dvao3i9g1yxXEss7mkw/fscGMtG3bf8d+Ga2900L60kOEppKvh7GTYP2ekGcEjaQUF\n\t6iT1Qm7qqgcZb3J7ZmnUFshpgi4wL29SuvRYEXhMJ/Pxm7e4MYQ9j4WkVZgxEcIEk9FP\n\tuLVw==","X-Gm-Message-State":"AHPjjUh8GQjEVXtmjzJ4TKncV7C5aPI4gyeS6q0V7sbnMNBK4sAW+2oZ\n\t6QB/nKYfnBsSFPCegmI/4hS7BA==","X-Google-Smtp-Source":"AOwi7QB07ExhtEmT+yHOSg6FPT+sO03JAOeI1/FoTkSlSxH8I/QwVcU4WnCF4+JZAcELygFWpkrHZA==","X-Received":"by 10.223.169.247 with SMTP id b110mr1433110wrd.31.1506519457826;\n\tWed, 27 Sep 2017 06:37:37 -0700 (PDT)","Date":"Wed, 27 Sep 2017 15:37:33 +0200","From":"Simon Horman <simon.horman@netronome.com>","To":"Jiri Pirko <jiri@resnulli.us>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927133731.GA14183@vergenet.net>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>\n\t<20170927125603.GH1944@nanopsycho.orion>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927125603.GH1944@nanopsycho.orion>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776366,"web_url":"http://patchwork.ozlabs.org/comment/1776366/","msgid":"<20170927134750.GI1944@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-27T13:47:50","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote:\n>On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote:\n>> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n>> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n>> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n>> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n>> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n>> >\n>> >...\n>> >\n>> >> >> > enum flow_dissector_key_id {\n>> >> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n>> >> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n>> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n>> >> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n>> >> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n>> >> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n>> >> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n>> >> >> \n>> >> >> I don't see the actual dissection implementation. Where is it?\n>> >> >> Did you test the patchset?\n>> >> >\n>> >> >Yes, I did test it. But it is also possible something went astray along the\n>> >> >way and I will retest.\n>> >> >\n>> >> >I think that the code you are looking for is in\n>> >> >fl_classify() in this patch.\n>> >> \n>> >> The dissection should be done in the flow_dissector. That's the whole\n>> >> point in having it generic. You should move it there.\n>> >\n>> >Coming back to this after lunch, I believe what I have done in this patch\n>> >is consistent with handling of other enc fields, which are set in\n>> >fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n>> >which is used by this patch, is already used in fl_classify().\n>> \n>> That means the current code is wrong. The dissection should be done in\n>> flow_dissector, not in fl_classify.\n>\n>Would an better approach be to move the fl_classify() below into, say,\n>skb_flow_dissect_tunnel_info() and call that from fl_classify().\n\nNo. There is one flow dissection function and you just set it up in a\nway you need it. Makes no sense to me to split it up in any way.\n\n\n>\n>The reason I suggest this rather than moving the code into\n>__skb_flow_dissect() is that currently flower assumes that tunnel_info\n>is used if present. While I assume other users of () assume tunnel_info\n>is not used even if present.\n\n__skb_flow_dissect should look at what caller wants, then check skb_tunnel_info\nonly in case it is needed.\n\n\n>\n>> >Without this patch I see:\n>> >\n>> >\n>> >static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,\n>> >                       struct tcf_result *res)\n>> >{\n>> >        ...\n>> >        struct ip_tunnel_info *info;\n>> >\n>> >        ...\n>> >\n>> >        info = skb_tunnel_info(skb);\n>> >        if (info) {\n>> >                struct ip_tunnel_key *key = &info->key;\n>> >\n>> >                switch (ip_tunnel_info_af(info)) {\n>> >                case AF_INET:\n>> >                        skb_key.enc_control.addr_type =\n>> >                                FLOW_DISSECTOR_KEY_IPV4_ADDRS;\n>> >                        skb_key.enc_ipv4.src = key->u.ipv4.src;\n>> >                        skb_key.enc_ipv4.dst = key->u.ipv4.dst;\n>> >                        break;\n>> >                case AF_INET6:\n>> >                        skb_key.enc_control.addr_type =\n>> >                                FLOW_DISSECTOR_KEY_IPV6_ADDRS;\n>> >                        skb_key.enc_ipv6.src = key->u.ipv6.src;\n>> >                        skb_key.enc_ipv6.dst = key->u.ipv6.dst;\n>> >                        break;\n>> >                }\n>> >\n>> >                skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id);\n>> >                skb_key.enc_tp.src = key->tp_src;\n>> >                skb_key.enc_tp.dst = key->tp_dst;\n>> >        }\n>> >\n>> >\t...\n>> >}\n>> >\n>> >This patch adds the following inside the if() clause above:\n>> >\n>> >\tif (info->options_len) {\n>> >\t\tskb_key.enc_opts.len = info->options_len;\n>> >\t\tip_tunnel_info_opts_get(skb_key.enc_opts.data, info);\n>> >\t}\n>> >\n>> >","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"ie+xFxJ6\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2Jzx6G6Xz9sRm\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 23:47:57 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753299AbdI0Nrz (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 09:47:55 -0400","from mail-wm0-f67.google.com ([74.125.82.67]:51033 \"EHLO\n\tmail-wm0-f67.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752581AbdI0Nrw (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 09:47:52 -0400","by mail-wm0-f67.google.com with SMTP id b195so18505756wmb.5\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 06:47:52 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\tx11sm8750983wrb.26.2017.09.27.06.47.50\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 27 Sep 2017 06:47:51 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=eKlQRhtIeO+lxXGnEnX4rFeY9eJSy9ZqDSznsap08Ic=;\n\tb=ie+xFxJ6DovvQZr0eLyav4Oc+qDMkk2OUfOnUmnhJkPFNfJiMAEVNh9ohU41v0T3wZ\n\tutKfKQqY+TWa99hW2MXGGkIHqOwCmhh/oM4uwwCO/vMCXg7amgoHwTZ2AvRMD5z3c+se\n\tfz4zzZnfp0yXxj9Fi48GiOm8h7yfx/IcowcBxYclPjBETQe/9nKCMwhtWj2zmAqYapsU\n\tEOs3Ps2xtz8D60PQyx5z8H9XS0qBDsSE2m/Pj9oTSTPbwnx2g0PFF4X9s9+eayhoaMLM\n\tPvkOy9XqJACG3rGiTppW5NUzxVZHDR9agDHK085hEG4zPgSKfMWQJJmMPVcT5zUeIUBn\n\tmqjg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=eKlQRhtIeO+lxXGnEnX4rFeY9eJSy9ZqDSznsap08Ic=;\n\tb=gXLEdaWCHOoZpjXXCQfjQi24V+4xntiv6yK8FMc42lVoH+AM4KuRGRqzRRPJCp/hVW\n\tjnIFFJplozWL/Pfj1n90fyzW/a1MyBzPrlfFnJUUCj+JP0xsovFT9HHrLdylyEQBAOME\n\t9BGn9Uz9aMJ8jgI+qY8COuzr7hEOwlaFW8aZk6aow6KuXoqqRcariSly36b7p9XYLSgo\n\trkRnI21oDrKmH0H2FOiYedzF9Spr037MuFScCtk4hhbG5i8vpQudCnhMLWx/anEKOWGS\n\t2w5Lc0E9oWTIrdDFVvN4x8BFUOIynVowaBkNjQAxU7V2FP2HU+7CotQi8n/Gczc8yYIo\n\ts3ZQ==","X-Gm-Message-State":"AHPjjUhrOA/VBqBflmsT3Iwa9nmyDIIq12g81+BctN4yjvocUMcOaEum\n\tLxVX3ehtl9Ub67m50HXM0YEKQg==","X-Google-Smtp-Source":"AOwi7QAIgoc9Wdi/xbQ2OWEuSC/q6ZIvAjnRs1p+my8eM2z9HOZlVV7rIh6JfyVhnCVPjMP0lYdgjA==","X-Received":"by 10.28.8.75 with SMTP id 72mr331287wmi.43.1506520071469;\n\tWed, 27 Sep 2017 06:47:51 -0700 (PDT)","Date":"Wed, 27 Sep 2017 15:47:50 +0200","From":"Jiri Pirko <jiri@resnulli.us>","To":"Simon Horman <simon.horman@netronome.com>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927134750.GI1944@nanopsycho.orion>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>\n\t<20170927125603.GH1944@nanopsycho.orion>\n\t<20170927133731.GA14183@vergenet.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927133731.GA14183@vergenet.net>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776372,"web_url":"http://patchwork.ozlabs.org/comment/1776372/","msgid":"<20170927135042.GB14183@vergenet.net>","list_archive_url":null,"date":"2017-09-27T13:50:44","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote:\n> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote:\n> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote:\n> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n> >> >\n> >> >...\n> >> >\n> >> >> >> > enum flow_dissector_key_id {\n> >> >> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> >> >> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> >> >> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> >> >> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> >> >> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n> >> >> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n> >> >> >> \n> >> >> >> I don't see the actual dissection implementation. Where is it?\n> >> >> >> Did you test the patchset?\n> >> >> >\n> >> >> >Yes, I did test it. But it is also possible something went astray along the\n> >> >> >way and I will retest.\n> >> >> >\n> >> >> >I think that the code you are looking for is in\n> >> >> >fl_classify() in this patch.\n> >> >> \n> >> >> The dissection should be done in the flow_dissector. That's the whole\n> >> >> point in having it generic. You should move it there.\n> >> >\n> >> >Coming back to this after lunch, I believe what I have done in this patch\n> >> >is consistent with handling of other enc fields, which are set in\n> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n> >> >which is used by this patch, is already used in fl_classify().\n> >> \n> >> That means the current code is wrong. The dissection should be done in\n> >> flow_dissector, not in fl_classify.\n> >\n> >Would an better approach be to move the fl_classify() below into, say,\n> >skb_flow_dissect_tunnel_info() and call that from fl_classify().\n> \n> No. There is one flow dissection function and you just set it up in a\n> way you need it. Makes no sense to me to split it up in any way.\n> \n> \n> >\n> >The reason I suggest this rather than moving the code into\n> >__skb_flow_dissect() is that currently flower assumes that tunnel_info\n> >is used if present. While I assume other users of () assume tunnel_info\n> >is not used even if present.\n> \n> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info\n> only in case it is needed.\n\nOk, do you think it is sufficient for __skb_flow_dissect to look at the\ntunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may\nbreak flower as it look at the tunnel info unconditionally.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"FnNkjHjp\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2K3K37rWz9tXT\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 23:50:53 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753120AbdI0Nuv (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 09:50:51 -0400","from mail-wm0-f51.google.com ([74.125.82.51]:45832 \"EHLO\n\tmail-wm0-f51.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752891AbdI0Nut (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 09:50:49 -0400","by mail-wm0-f51.google.com with SMTP id q124so18542918wmb.0\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 06:50:49 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id\n\tp80sm4478032wmf.42.2017.09.27.06.50.46\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 27 Sep 2017 06:50:47 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=xTd1mmLEotYTpB96/p9mXWQx5WxZru9MOWHlLI81XdA=;\n\tb=FnNkjHjpVvHsMmMMl+4thzj7EgJhGXD8Xyu89PI05j04oIycLC+PWiGUdnRS+SvGoC\n\twfJg+l9FshXzNYBQ6VqkitYILqzzMoRT6orOR971ryrF00SMdKjwraqT45DMzhEwqvKR\n\t3mLgjyzz2yS3hxk9opA9WOHc6x2fuI4IXTZfdsUezLQl0a+Byxn0gVBoCu7ljWWJzNSW\n\tVeDA3OBIpbAEe/FQsN8IVGMamKJ8DkXTTIdgBnxEk8MKiqPNpeNYXGb5wUjkMvO+5PVl\n\t0e3K1AgSow2D6ZaW8vTkjjoouvhi61f6+Y0Kc7KIDYgeJOVWjA/whIc5uvOBcOVdQ9tV\n\t5w/g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=xTd1mmLEotYTpB96/p9mXWQx5WxZru9MOWHlLI81XdA=;\n\tb=egq8i5z+XVBeeUaKYRDrveHh35hvxn/ofpZ+go2SBlEhWfUaP/U8eUHvdbLyKAvpwg\n\t0a0IHZlRSLDMWB86cnFeJJE8PC6Cr+HsM3pfoP3uu8YUpxN0ZNCfuXVhbIlA4KBncYxI\n\tpt0FfYjD+RAyqhZXHBqfwTaN/o0qcZ95D3AqZUGicGk409s4oGrVhXsGAMsGHujy/pKt\n\tFqcfzUqfjeZNCXdIkO/fIrN4YvDllN/ICwdvlEu3AFtR9rKix26bWYIuVxWqpkclZydv\n\tFesjKVwtqocFKOhN79jYhspI7enlLWnKqJouahj4y7gMdUJiy2/MTASQ4l7ROk1gyvPV\n\tSDtA==","X-Gm-Message-State":"AHPjjUil3c/bYFAe8+LWVt5qvB3lrcmaQuXqUKg1GCIKY+FkydtXsCr8\n\tZlCvOPeWTxUt5fEOzPM54cB0E6l7zNI=","X-Google-Smtp-Source":"AOwi7QCbgJRXSml3IDiksO61M/GmfoUFqF5PrGwzFUG9XRlK/hzbHn0JdQR39T/H2T9aNvG8ltZcVA==","X-Received":"by 10.28.32.22 with SMTP id g22mr337412wmg.38.1506520248440;\n\tWed, 27 Sep 2017 06:50:48 -0700 (PDT)","Date":"Wed, 27 Sep 2017 15:50:44 +0200","From":"Simon Horman <simon.horman@netronome.com>","To":"Jiri Pirko <jiri@resnulli.us>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927135042.GB14183@vergenet.net>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>\n\t<20170927125603.GH1944@nanopsycho.orion>\n\t<20170927133731.GA14183@vergenet.net>\n\t<20170927134750.GI1944@nanopsycho.orion>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927134750.GI1944@nanopsycho.orion>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776378,"web_url":"http://patchwork.ozlabs.org/comment/1776378/","msgid":"<20170927140011.GJ1944@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-27T14:00:11","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Wed, Sep 27, 2017 at 03:50:44PM CEST, simon.horman@netronome.com wrote:\n>On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote:\n>> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote:\n>> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote:\n>> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n>> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n>> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n>> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n>> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n>> >> >\n>> >> >...\n>> >> >\n>> >> >> >> > enum flow_dissector_key_id {\n>> >> >> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n>> >> >> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n>> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n>> >> >> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n>> >> >> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n>> >> >> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n>> >> >> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n>> >> >> >> \n>> >> >> >> I don't see the actual dissection implementation. Where is it?\n>> >> >> >> Did you test the patchset?\n>> >> >> >\n>> >> >> >Yes, I did test it. But it is also possible something went astray along the\n>> >> >> >way and I will retest.\n>> >> >> >\n>> >> >> >I think that the code you are looking for is in\n>> >> >> >fl_classify() in this patch.\n>> >> >> \n>> >> >> The dissection should be done in the flow_dissector. That's the whole\n>> >> >> point in having it generic. You should move it there.\n>> >> >\n>> >> >Coming back to this after lunch, I believe what I have done in this patch\n>> >> >is consistent with handling of other enc fields, which are set in\n>> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n>> >> >which is used by this patch, is already used in fl_classify().\n>> >> \n>> >> That means the current code is wrong. The dissection should be done in\n>> >> flow_dissector, not in fl_classify.\n>> >\n>> >Would an better approach be to move the fl_classify() below into, say,\n>> >skb_flow_dissect_tunnel_info() and call that from fl_classify().\n>> \n>> No. There is one flow dissection function and you just set it up in a\n>> way you need it. Makes no sense to me to split it up in any way.\n>> \n>> \n>> >\n>> >The reason I suggest this rather than moving the code into\n>> >__skb_flow_dissect() is that currently flower assumes that tunnel_info\n>> >is used if present. While I assume other users of () assume tunnel_info\n>> >is not used even if present.\n>> \n>> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info\n>> only in case it is needed.\n>\n>Ok, do you think it is sufficient for __skb_flow_dissect to look at the\n>tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may\n>break flower as it look at the tunnel info unconditionally.\n\nyeah. When flower needs that, it will get that from the flow dissector.\nI don't see why it would break anything. Again, existing code is wrong:\ncommit bc3103f1ed405de587fa43d8b0671e615505a700\nAuthor: Amir Vadai <amir@vadai.me>\nDate:   Thu Sep 8 16:23:47 2016 +0300\n\n    net/sched: cls_flower: Classify packet in ip tunnels\n\nThe dissection has to be moved to flow dissector.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"VbIqWEwj\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2KGB3xWwz9tY0\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 00:00:18 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753357AbdI0OAQ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 10:00:16 -0400","from mail-wm0-f54.google.com ([74.125.82.54]:44147 \"EHLO\n\tmail-wm0-f54.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753050AbdI0OAN (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 10:00:13 -0400","by mail-wm0-f54.google.com with SMTP id m127so20547107wmm.1\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 07:00:13 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\tz51sm16181923wrz.80.2017.09.27.07.00.12\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 27 Sep 2017 07:00:12 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=ZRjAPr7WmWx0MHNGre4XLNMtgnJS2zV6a0Jf3MqFnUk=;\n\tb=VbIqWEwjgQaSmnbnwR4QuiK0JEZDyE/8wkVtZOPcHlIae/qfXME1keiLATuNZkvfzW\n\toA9mAB+bwMZF8yqyS6hBeCzNZPQfcX+v9GycdjaxeH/uHIEMSMoXeokmgyKnuIJtO8aR\n\twDtZVV+wVvglid5GTCvTvOUdCQtuQJTNnbxAqcXyiUmlpFYtgEsemJrqXzR67CpghX80\n\tHShjqURKczqRV0sxnWNtJeU8XowMkQ9YsOPgUkexWmgVi7PtFQcAF/L8+XBY4+h7QAqt\n\tv/W6f8uaF4srCh7a/0lsY7xQQ3xzbjJMlMXRbFiguIA9eafBNZ6XXVtlqmkrehT0WCbX\n\tk4nA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=ZRjAPr7WmWx0MHNGre4XLNMtgnJS2zV6a0Jf3MqFnUk=;\n\tb=uaQUBngugSTsYiasyOlrxZ+rZQwb/5EskWJ6/Ce/ZOg9LhetAb1yJI8XpCkuLNf8Rg\n\tpTuhn9O9EzvECDMi5Nd2vACUprpjPvNDBIBOOzSKQnquQIU03CbZjnU61TJ77gIQvaml\n\thWCKLWyGv8Sbi2wq8C+MCAotKa/3VYSProdOb2NOsDb7ilB/2Aw0ZzQjIpLBEcfyCsB/\n\tbKi/utesJsYB7bSO8LhMXZnCgA9k2geVfjFKzr5ZjHCjr4lm1dgTPShumVvEWdFZdqn2\n\tvaA4wKT1bW1mFiP/BITEkLcwYz3Q+k81oskjB4t/EAotRS3JkH6WpL9AwX3uGcuB2+aT\n\t7Oiw==","X-Gm-Message-State":"AHPjjUgWokRY24UuesdUFGzCKC0OPtWFj2PKf4YWHTavyuQokB+mehY9\n\tzWvHaBxqmJgv61ehVP19OJjCKg==","X-Google-Smtp-Source":"AOwi7QAimkSsBbmVHX8LbynI4jFkDgiskfdrYzbLWyKtfcF+qHpuZjRHxnegBv2CAlg+kBwIsAMKzQ==","X-Received":"by 10.28.74.211 with SMTP id n80mr436353wmi.94.1506520812523;\n\tWed, 27 Sep 2017 07:00:12 -0700 (PDT)","Date":"Wed, 27 Sep 2017 16:00:11 +0200","From":"Jiri Pirko <jiri@resnulli.us>","To":"Simon Horman <simon.horman@netronome.com>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com, amir@vadai.me","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927140011.GJ1944@nanopsycho.orion>","References":"<1506500194-17637-1-git-send-email-simon.horman@netronome.com>\n\t<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>\n\t<20170927125603.GH1944@nanopsycho.orion>\n\t<20170927133731.GA14183@vergenet.net>\n\t<20170927134750.GI1944@nanopsycho.orion>\n\t<20170927135042.GB14183@vergenet.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927135042.GB14183@vergenet.net>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776383,"web_url":"http://patchwork.ozlabs.org/comment/1776383/","msgid":"<20170927140952.GC14183@vergenet.net>","list_archive_url":null,"date":"2017-09-27T14:09:54","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":64714,"url":"http://patchwork.ozlabs.org/api/people/64714/","name":"Simon Horman","email":"simon.horman@netronome.com"},"content":"On Wed, Sep 27, 2017 at 04:00:11PM +0200, Jiri Pirko wrote:\n> Wed, Sep 27, 2017 at 03:50:44PM CEST, simon.horman@netronome.com wrote:\n> >On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote:\n> >> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote:\n> >> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote:\n> >> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n> >> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n> >> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n> >> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n> >> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n> >> >> >\n> >> >> >...\n> >> >> >\n> >> >> >> >> > enum flow_dissector_key_id {\n> >> >> >> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n> >> >> >> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n> >> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n> >> >> >> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n> >> >> >> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n> >> >> >> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n> >> >> >> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n> >> >> >> >> \n> >> >> >> >> I don't see the actual dissection implementation. Where is it?\n> >> >> >> >> Did you test the patchset?\n> >> >> >> >\n> >> >> >> >Yes, I did test it. But it is also possible something went astray along the\n> >> >> >> >way and I will retest.\n> >> >> >> >\n> >> >> >> >I think that the code you are looking for is in\n> >> >> >> >fl_classify() in this patch.\n> >> >> >> \n> >> >> >> The dissection should be done in the flow_dissector. That's the whole\n> >> >> >> point in having it generic. You should move it there.\n> >> >> >\n> >> >> >Coming back to this after lunch, I believe what I have done in this patch\n> >> >> >is consistent with handling of other enc fields, which are set in\n> >> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n> >> >> >which is used by this patch, is already used in fl_classify().\n> >> >> \n> >> >> That means the current code is wrong. The dissection should be done in\n> >> >> flow_dissector, not in fl_classify.\n> >> >\n> >> >Would an better approach be to move the fl_classify() below into, say,\n> >> >skb_flow_dissect_tunnel_info() and call that from fl_classify().\n> >> \n> >> No. There is one flow dissection function and you just set it up in a\n> >> way you need it. Makes no sense to me to split it up in any way.\n> >> \n> >> \n> >> >\n> >> >The reason I suggest this rather than moving the code into\n> >> >__skb_flow_dissect() is that currently flower assumes that tunnel_info\n> >> >is used if present. While I assume other users of () assume tunnel_info\n> >> >is not used even if present.\n> >> \n> >> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info\n> >> only in case it is needed.\n> >\n> >Ok, do you think it is sufficient for __skb_flow_dissect to look at the\n> >tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may\n> >break flower as it look at the tunnel info unconditionally.\n> \n> yeah. When flower needs that, it will get that from the flow dissector.\n> I don't see why it would break anything. Again, existing code is wrong:\n\nI understand that you think the existing code is wrong.\nBut I also want to try not to add new bugs.\n\nI am concerned about the case where none of FLOW_DISSECTOR_KEY_ENC_* are\nset but flower currently dissects the tunnel info anyway. If I make\ndissection of tunnel info dependent on FLOW_DISSECTOR_KEY_ENC_*\nthat may change things.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=netronome-com.20150623.gappssmtp.com\n\theader.i=@netronome-com.20150623.gappssmtp.com\n\theader.b=\"YYwBUeId\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2KTR0JmMz9t3m\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 00:10:03 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753247AbdI0OKA (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 10:10:00 -0400","from mail-wm0-f42.google.com ([74.125.82.42]:50350 \"EHLO\n\tmail-wm0-f42.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752231AbdI0OJ7 (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 10:09:59 -0400","by mail-wm0-f42.google.com with SMTP id b195so18772842wmb.5\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 07:09:59 -0700 (PDT)","from vergenet.net ([217.111.208.18])\n\tby smtp.gmail.com with ESMTPSA id\n\ty99sm27303736wmh.1.2017.09.27.07.09.56\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tWed, 27 Sep 2017 07:09:57 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=netronome-com.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=aLOkoRYZOgg+gju3zNoQpYlrEc0+5zs8S89SEhiO9To=;\n\tb=YYwBUeIdVvRfqKsP1P1fzrlL5GkgDe160frhxWJYQa/lrcaMIHLCMTNLm4TYbI877m\n\t5ecdIfu/Y8uDC6lEnKpQ5y0N2YmelTNzTJmGreLvnojRsLB0Z1P3TRJlBsXKfRu60mBP\n\t/gwC7geS9F2Dh6OrK+BPuWkkas+5T3gEMPgL/nUR4pPu0TX+3mml35uO079GrdhVst7C\n\tDAnfysmH6WWcXq9JQqDBSnld7lNhe65hQQOnkKeybhnojxrfiUw+oHDbhy9Vx6XywbVS\n\tsW0+JQ0NxvDfnlU/Cvtsk2YrAf5A5NhTz/f4T2/Hij7ovRgjCp5Mj8JgVAtUnnMTH+kS\n\tA4Ag==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=aLOkoRYZOgg+gju3zNoQpYlrEc0+5zs8S89SEhiO9To=;\n\tb=E+h9Rm+xW1fTg74+NuapBQe2mSilL6iB29olu+uI1u5x4GrUKPVuOZf8Bg1IorKbLG\n\t6sEWCXzeDafTz4ihtrJuyydG6sUTcSeuIQARgzrY+Sng+RynpibDpza6JLhHY7w1LxPX\n\tNSHk0COQpXdVwaH8uBknzoGS/rXUUZ2tHIGXOnuNzfMeumPBwqG/HYUJmcAZnXMjTLij\n\tMZmoe1BJQDgfixmA0cQpqjSm83+yjEA48Uhy4EBlzNwZ43yjapaipG7qF/Z8RsfBgqGy\n\tkWjE/kZ7BiveXbImJiChN0w6vgo5LWV/CWdyrHGVTPSvnIBJ18FXr9L50fP6FZooSWXF\n\twS6Q==","X-Gm-Message-State":"AHPjjUh5+z5BtRFUgAPm1YQZbNP2SBow7FIF9mx2phfJqYkaWdw/NBmU\n\t/clFVj6o+xTQdlSq0LOav/pK+w==","X-Google-Smtp-Source":"AOwi7QAeQ4J3a9rGXKqeQ1Wz6AWN/oTtWsaQiRarPhNczpd0hXzfEko6xllmIWToqbuSwC+XQAcE8g==","X-Received":"by 10.28.165.1 with SMTP id o1mr466715wme.31.1506521398274;\n\tWed, 27 Sep 2017 07:09:58 -0700 (PDT)","Date":"Wed, 27 Sep 2017 16:09:54 +0200","From":"Simon Horman <simon.horman@netronome.com>","To":"Jiri Pirko <jiri@resnulli.us>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com, amir@vadai.me","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927140952.GC14183@vergenet.net>","References":"<1506500194-17637-3-git-send-email-simon.horman@netronome.com>\n\t<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>\n\t<20170927125603.GH1944@nanopsycho.orion>\n\t<20170927133731.GA14183@vergenet.net>\n\t<20170927134750.GI1944@nanopsycho.orion>\n\t<20170927135042.GB14183@vergenet.net>\n\t<20170927140011.GJ1944@nanopsycho.orion>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927140011.GJ1944@nanopsycho.orion>","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776391,"web_url":"http://patchwork.ozlabs.org/comment/1776391/","msgid":"<20170927141911.GK1944@nanopsycho.orion>","list_archive_url":null,"date":"2017-09-27T14:19:11","subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","submitter":{"id":15321,"url":"http://patchwork.ozlabs.org/api/people/15321/","name":"Jiri Pirko","email":"jiri@resnulli.us"},"content":"Wed, Sep 27, 2017 at 04:09:54PM CEST, simon.horman@netronome.com wrote:\n>On Wed, Sep 27, 2017 at 04:00:11PM +0200, Jiri Pirko wrote:\n>> Wed, Sep 27, 2017 at 03:50:44PM CEST, simon.horman@netronome.com wrote:\n>> >On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote:\n>> >> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote:\n>> >> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote:\n>> >> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote:\n>> >> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote:\n>> >> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote:\n>> >> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote:\n>> >> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote:\n>> >> >> >\n>> >> >> >...\n>> >> >> >\n>> >> >> >> >> > enum flow_dissector_key_id {\n>> >> >> >> >> > \tFLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */\n>> >> >> >> >> > \tFLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */\n>> >> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id {\n>> >> >> >> >> > \tFLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */\n>> >> >> >> >> > \tFLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */\n>> >> >> >> >> > \tFLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */\n>> >> >> >> >> >+\tFLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */\n>> >> >> >> >> \n>> >> >> >> >> I don't see the actual dissection implementation. Where is it?\n>> >> >> >> >> Did you test the patchset?\n>> >> >> >> >\n>> >> >> >> >Yes, I did test it. But it is also possible something went astray along the\n>> >> >> >> >way and I will retest.\n>> >> >> >> >\n>> >> >> >> >I think that the code you are looking for is in\n>> >> >> >> >fl_classify() in this patch.\n>> >> >> >> \n>> >> >> >> The dissection should be done in the flow_dissector. That's the whole\n>> >> >> >> point in having it generic. You should move it there.\n>> >> >> >\n>> >> >> >Coming back to this after lunch, I believe what I have done in this patch\n>> >> >> >is consistent with handling of other enc fields, which are set in\n>> >> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info,\n>> >> >> >which is used by this patch, is already used in fl_classify().\n>> >> >> \n>> >> >> That means the current code is wrong. The dissection should be done in\n>> >> >> flow_dissector, not in fl_classify.\n>> >> >\n>> >> >Would an better approach be to move the fl_classify() below into, say,\n>> >> >skb_flow_dissect_tunnel_info() and call that from fl_classify().\n>> >> \n>> >> No. There is one flow dissection function and you just set it up in a\n>> >> way you need it. Makes no sense to me to split it up in any way.\n>> >> \n>> >> \n>> >> >\n>> >> >The reason I suggest this rather than moving the code into\n>> >> >__skb_flow_dissect() is that currently flower assumes that tunnel_info\n>> >> >is used if present. While I assume other users of () assume tunnel_info\n>> >> >is not used even if present.\n>> >> \n>> >> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info\n>> >> only in case it is needed.\n>> >\n>> >Ok, do you think it is sufficient for __skb_flow_dissect to look at the\n>> >tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may\n>> >break flower as it look at the tunnel info unconditionally.\n>> \n>> yeah. When flower needs that, it will get that from the flow dissector.\n>> I don't see why it would break anything. Again, existing code is wrong:\n>\n>I understand that you think the existing code is wrong.\n>But I also want to try not to add new bugs.\n>\n>I am concerned about the case where none of FLOW_DISSECTOR_KEY_ENC_* are\n>set but flower currently dissects the tunnel info anyway. If I make\n>dissection of tunnel info dependent on FLOW_DISSECTOR_KEY_ENC_*\n>that may change things.\n\nIf none of FLOW_DISSECTOR_KEY_ENC_* are set, flower does not care about\nthe fields and therefore they are masked out by fl_set_masked_key.\n\nOtherwise it would be a bug is flower would match on something user did\nnot specify.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=resnulli-us.20150623.gappssmtp.com\n\theader.i=@resnulli-us.20150623.gappssmtp.com\n\theader.b=\"u0JpD+wF\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2Kh608Ydz9t3m\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 00:19:18 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753253AbdI0OTQ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 10:19:16 -0400","from mail-wm0-f46.google.com ([74.125.82.46]:48298 \"EHLO\n\tmail-wm0-f46.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752439AbdI0OTN (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 10:19:13 -0400","by mail-wm0-f46.google.com with SMTP id m127so18896789wmm.3\n\tfor <netdev@vger.kernel.org>; Wed, 27 Sep 2017 07:19:13 -0700 (PDT)","from localhost (jirka.pirko.cz. [84.16.102.26])\n\tby smtp.gmail.com with ESMTPSA id\n\te77sm4509417wmi.29.2017.09.27.07.19.11\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tWed, 27 Sep 2017 07:19:11 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=resnulli-us.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=pe/LCLSMTjttrRhnYL1RRrkKjIXaATMsxIyMf6FmtiM=;\n\tb=u0JpD+wFu8JkK/u1szH+oM/jF/o7U0Yz7TW/NXDUSoRAdqyMV1ZSd8THQ7TIDTSyQT\n\tbHSaMiOqtItaRcCIdKj3YwDvOGZDt+untNbn4cKD5lQuTvZ8+gFdRiSIXjmjWJfjIvax\n\t65wS8oww+C1KJT5kdg3YGbqeV9aCCZuroQ0diVOR3rIyfVjDQZhfye9FPnLUwquHaBZn\n\t2SOpiTa5wfG1/YZfP1G5cVKvXvXia/2zBliAtowMM1fpy6jOC1atdOGoPR0Yw7g2G27N\n\t0jnkBiyKCm58Gp5KKYt6xh/bSKghBfraPFrqVo7g/KhD0qD+k9qbEDSOABzsld/EzGIZ\n\tNnnQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=pe/LCLSMTjttrRhnYL1RRrkKjIXaATMsxIyMf6FmtiM=;\n\tb=SgdLg9wXJu8oic8ZH/5YjNKH6u6gKDaQtfY4E09fx0lAIYqQNxVosv9omjuWWnDUFM\n\tHHlTcK6bE6hZyuyLB895zAmnwuS8mCfC4JDFeCD6S+uXvWombQuO73HltDsB/dSDwYkB\n\tkeLzyY/ZPSPG1GeZWu0Lynxq7Xv+ooC+x5LGB5KUj6Xg6tWaf6PumXvDssQ+uEXjxo4L\n\tx72f0zBT9/nNY/KenuDJGPtry1I6QqXbH/Yo1bqVDyoz5/UMVMy1E+WCSqgBk6/xOrcY\n\tY6Wo00c96X4wVCLAQDZE2ht8hnlFvZXDCg/rIOSe4C6w14Q9huW3u7g7PyxQ7BWky+6s\n\tw+Og==","X-Gm-Message-State":"AHPjjUiCJmv3PjMfH6NAedCC7B8dTkFVJkRAGxacffVl2ev7Rw5wj5fm\n\tLDAH2E50heNAGfIP8AC4/xnU0Q==","X-Google-Smtp-Source":"AOwi7QA20ZN4jdeL+MYzyrXvPuTxtLsCV+93oKVCgPz58B9tHPYAVr1URs4UpX1O4U5y1Ow6oIyQhQ==","X-Received":"by 10.28.165.136 with SMTP id o130mr449838wme.107.1506521952297; \n\tWed, 27 Sep 2017 07:19:12 -0700 (PDT)","Date":"Wed, 27 Sep 2017 16:19:11 +0200","From":"Jiri Pirko <jiri@resnulli.us>","To":"Simon Horman <simon.horman@netronome.com>","Cc":"David Miller <davem@davemloft.net>, Jiri Pirko <jiri@mellanox.com>,\n\tJamal Hadi Salim <jhs@mojatatu.com>,\n\tCong Wang <xiyou.wangcong@gmail.com>, netdev@vger.kernel.org,\n\toss-drivers@netronome.com, amir@vadai.me","Subject":"Re: [PATCH v2 net-next 2/2] net/sched: allow flower to match tunnel\n\toptions","Message-ID":"<20170927141911.GK1944@nanopsycho.orion>","References":"<20170927091005.GB1944@nanopsycho.orion>\n\t<20170927092732.GC25449@vergenet.net>\n\t<20170927110822.GD1944@nanopsycho.orion>\n\t<20170927125205.GA30000@vergenet.net>\n\t<20170927125603.GH1944@nanopsycho.orion>\n\t<20170927133731.GA14183@vergenet.net>\n\t<20170927134750.GI1944@nanopsycho.orion>\n\t<20170927135042.GB14183@vergenet.net>\n\t<20170927140011.GJ1944@nanopsycho.orion>\n\t<20170927140952.GC14183@vergenet.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170927140952.GC14183@vergenet.net>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}}]