From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id D87901B019 for ; Thu, 19 Apr 2018 15:03:33 +0200 (CEST) Received: by mail-wr0-f195.google.com with SMTP id u4-v6so13824857wrg.10 for ; Thu, 19 Apr 2018 06:03:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=elpN8lzMwDZ1Ggkq3VE02GjDyVPGydhZyRNbnysODGk=; b=Ma1aMHa7JZxmNdxwAex1RAEup2dVAOoZIJ6CHUGEvoG0deqCrZymBxXN8eNY3BikL3 G2VeMgxxui3wuud+DicG+1hlW67zEDhtpb+r4K/WokFWQ3k2rGWU+0tiGvpY6HoZNPfG lIFQuAUnpwXhvguWep5pA8S+6gV5vS1qs/sz2da1AvmNCh+3aJlsYiUCc8wtemR+6xXF NpNne8zdwKixNzysQ5NoC0ZlvD8m7Uc5OnNBvHn0BDeyT227D08eQaX6jsvTAb85IKQa Apk3rsPHDoNqiczSRjfN5KcTPXr2nWiocQNd4680MTCkeXU87uwG2fRLM5LjbaLVtn7g V5Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=elpN8lzMwDZ1Ggkq3VE02GjDyVPGydhZyRNbnysODGk=; b=gpSTuUm/TjbBVs53YftXqz0i1PItdZNY/ZLcK7D5RPxjeR1wZKo18s6lUjrUBcMPPb 4VAGNPKFfnmxEwNBX9HDAc+AyJ7QUZSzXL03Fx4uXj+WJrYA2ZziNDhNgaxZ0pmpMfie WvN03m6h1QlSQbo0Hs+dqbewVuugd1nqK2RNkoN3lQNhHpvkwQZ7jE9VMOphPU/zkhNg aZih4KgLoph3gG5Ku05yqxWKE8z+CCTmnQevuzMaY62t1J+4CfkifL8lCpMabHBpPtmg 4ymMAMZwUjk7EU1tx7V2snP9K3fj17wuAtCsPHk5aRgdQrNNvEaSNek/1Pzo0cjn7jAV 5D3Q== X-Gm-Message-State: ALQs6tCE2CunVO/es6+MdB5j6M9ti65nQz2wjxiWYlf06RcPmM7e5lE5 IuA4c8XKSBXxGPEHC7mkhjvnUQ== X-Google-Smtp-Source: AIpwx49/yLZtrLM5HvM8dOtqF/o9sCcijsgrWJxO0fpxjbsawspm6DGp84RtURACPH++D2S1sEimLA== X-Received: by 2002:adf:e092:: with SMTP id c18-v6mr3005460wri.70.1524143013549; Thu, 19 Apr 2018 06:03:33 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id d28-v6sm3427740wra.37.2018.04.19.06.03.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Apr 2018 06:03:32 -0700 (PDT) Date: Thu, 19 Apr 2018 15:03:19 +0200 From: Adrien Mazarguil To: Declan Doherty Cc: dev@dpdk.org, Alex Rosenbaum , Ferruh Yigit , Thomas Monjalon , Shahaf Shuler , Qi Zhang , Alejandro Lucero , Andrew Rybchenko , Mohammad Abdul Awal , Remy Horton , John McNamara , Rony Efraim , Jingjing Wu , Wenzhuo Lu , Vincent Jardin , Yuanhan Liu , Bruce Richardson , Konstantin Ananyev , Zhihong Wang Message-ID: <20180419130319.GZ4957@6wind.com> References: <1523017443-12414-1-git-send-email-declan.doherty@intel.com> <20180418210423.13847-1-declan.doherty@intel.com> <20180418210423.13847-3-declan.doherty@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180418210423.13847-3-declan.doherty@intel.com> Subject: Re: [dpdk-dev] [PATCH v4 2/6] ethdev: Add jump action type to rte_flow X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Apr 2018 13:03:34 -0000 On Wed, Apr 18, 2018 at 10:04:19PM +0100, Declan Doherty wrote: > Add jump action type which defines an action which allows a matched > flow to be redirect to the specified group. This allows physical and > logical flow table/group hierarchies to be managed through rte_flow. > > Signed-off-by: Declan Doherty You should have rebased this series including this patch on top of mine ([1] followed by [2]) to avoid some mistakes such as documenting "terminating actions". This patch could also update documentation for flow rules [3] groups [4] and priorities [5] to make clear that groups aren't linked by default, their order doesn't matter and explicit JUMP actions are needed to reach them (actually look for every instance of the word "group" in this documentation). Also the addition of an action in the middle of the enum has an ABI impact, please put a reminder in the commit log as in [6]. More below. [1] "Bunch of flow API-related fixes" http://dpdk.org/ml/archives/dev/2018-April/098035.html [2] "Flow API overhaul for switch offloads" http://dpdk.org/ml/archives/dev/2018-April/098047.html [3] "9.2.1. Description" https://dpdk.org/doc/guides/prog_guide/rte_flow.html#description [3] "9.2.2.1. Attribute: Group" https://dpdk.org/doc/guides/prog_guide/rte_flow.html#attribute-group [5] "9.2.2.2. Attribute: Priority" https://dpdk.org/doc/guides/prog_guide/rte_flow.html#attribute-priority [6] "ethdev: add physical port action to flow API" http://dpdk.org/ml/archives/dev/2018-April/098062.html > --- > doc/guides/prog_guide/rte_flow.rst | 26 ++++++++++++++++++++++++-- > lib/librte_ether/rte_flow.h | 27 +++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst > index 672473d33..325010544 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -1188,6 +1188,28 @@ flow rules: > | 2 | END | > +-------+----------------------------+ > > + Extraneous empty line. > +Action: ``JUMP`` > +^^^^^^^^^^^^^^^^ > + > +Redirects packets to a group on the current device. > + > +In a hierarchy of groups, which can be used to represent physical or logical > +flow group/tables on the device, this action allows the terminating action to > +be a group on that device. > + > +- Terminating by default. Since there are no more "terminating" actions, you should document that it will cause traffic to be processed by flow rules found in the target group. Also I think you should put emphasis on undefined behavior: - Targeting a group that doesn't include any flow rule. - Triggering loops. > + > +.. _table_rte_flow_action_jump: > + > +.. table:: JUMP > + > + +-----------+---------------------------------+ > + | Field | Value | > + +===========+=================================+ > + | ``group`` | Group ID to redirect packets to | > + +-----------+---------------------------------+ Nit: "Group ID" => "group ID" to keep the style. > + > Action: ``MARK`` > ^^^^^^^^^^^^^^^^ > > @@ -1512,7 +1534,7 @@ the RTE_FLOW_ITEM_TYPE_END item type. > | ``definition`` | Tunnel end-point overlay definition | > +----------------+-------------------------------------+ > > -.. _table_rte_flow_action_tunnel_encap_example: > +.. _table_rte_flow_action_tunnel_encap_vxlan_example: > > .. table:: IPv4 VxLAN flow pattern example. > > @@ -1551,7 +1573,7 @@ NVGRE network overlay which conforms with RFC 7637 (NVGRE: Network Virtualizatio > Using Generic Routing Encapsulation). The pattern must be terminated with > the RTE_FLOW_ITEM_TYPE_END item type. > > -.. _table_rte_flow_action_tunnel_encap_example: > +.. _table_rte_flow_action_tunnel_encap_nvgre_example: The above two hunks do not seem relevant. > > .. table:: IPv4 NVGRE flow pattern example. > > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h > index 537e1bfba..91544f62b 100644 > --- a/lib/librte_ether/rte_flow.h > +++ b/lib/librte_ether/rte_flow.h > @@ -913,6 +913,20 @@ enum rte_flow_action_type { > */ > RTE_FLOW_ACTION_TYPE_PASSTHRU, > > + /** > + * RTE_FLOW_ACTION_TYPE_JUMP > + * > + * Redirects packets to a group on the current device. > + * > + * In a hierarchy of groups, which can be used to represent > + * physical or logical flow groups/tables on a device, this > + * action allows the terminating action to be a group on > + * that device. Since struct rte_flow_action_jump provides complete documentation, no need to repeat this paragraph here. A single line is enough (see other actions). > + * > + * See struct rte_flow_action_jump Nit: missing "." > + */ > + RTE_FLOW_ACTION_TYPE_JUMP, > + > /** > * [META] > * > @@ -1213,6 +1227,19 @@ struct rte_flow_action_tunnel_encap { > */ > }; > > +/** > + * RTE_FLOW_ACTION_TYPE_JUMP > + * > + * Redirects packets to a group on the current device. > + * > + * In a hierarchy of groups, which can be used to represent physical or logical > + * flow tables on the device, this action allows the action to be a redirect to > + * a group on that device. Remember to synchronize this paragraph after making changes to rte_flow.rst. Here you can also provide more info regarding undefined behavior (useful in Doxygen format for application writers). > + */ > +struct rte_flow_action_jump { > + uint32_t group; > +}; > + You should move this definition before "struct rte_flow_action_mark" to keep the same definition order as in the enum. > /** > * Definition of a single action. > * > -- > 2.14.3 > -- Adrien Mazarguil 6WIND