From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Declan Doherty <declan.doherty@intel.com>
Cc: dev@dpdk.org, Alex Rosenbaum <alexr@mellanox.com>,
Ferruh Yigit <ferruh.yigit@intel.com>,
Thomas Monjalon <thomas@monjalon.net>,
Shahaf Shuler <shahafs@mellanox.com>,
Qi Zhang <qi.z.zhang@intel.com>,
Alejandro Lucero <alejandro.lucero@netronome.com>,
Andrew Rybchenko <arybchenko@solarflare.com>,
Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>,
Remy Horton <remy.horton@intel.com>,
John McNamara <john.mcnamara@intel.com>,
Rony Efraim <ronye@mellanox.com>,
Jingjing Wu <jingjing.wu@intel.com>,
Wenzhuo Lu <wenzhuo.lu@intel.com>,
Vincent Jardin <vincent.jardin@6wind.com>,
Yuanhan Liu <yliu@fridaylinux.org>,
Bruce Richardson <bruce.richardson@intel.com>,
Konstantin Ananyev <konstantin.ananyev@intel.com>,
Zhihong Wang <zhihong.wang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v4 2/6] ethdev: Add jump action type to rte_flow
Date: Thu, 19 Apr 2018 15:03:19 +0200 [thread overview]
Message-ID: <20180419130319.GZ4957@6wind.com> (raw)
In-Reply-To: <20180418210423.13847-3-declan.doherty@intel.com>
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 <declan.doherty@intel.com>
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
next prev parent reply other threads:[~2018-04-19 13:03 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-06 12:23 [dpdk-dev] [PATCH v3 0/4] ethdev: Additions to support tunnel encap/decap offload Declan Doherty
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add group counter support to rte_flow Declan Doherty
2018-04-06 20:26 ` Adrien Mazarguil
2018-04-09 14:22 ` Mohammad Abdul Awal
2018-04-09 15:23 ` Adrien Mazarguil
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 2/4] ethdev: Add tunnel encap/decap actions Declan Doherty
2018-04-06 20:26 ` Adrien Mazarguil
2018-04-09 16:10 ` Mohammad Abdul Awal
2018-04-10 10:19 ` Adrien Mazarguil
2018-04-10 11:06 ` Shahaf Shuler
2018-04-17 14:58 ` Doherty, Declan
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 3/4] ethdev: Add group action type to rte_flow Declan Doherty
2018-04-06 20:26 ` Adrien Mazarguil
2018-04-17 14:40 ` Doherty, Declan
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 4/4] ethdev: Add metadata flow and action items support Declan Doherty
2018-04-06 20:27 ` Adrien Mazarguil
2018-04-17 14:40 ` Doherty, Declan
2018-04-27 7:41 ` Xueming(Steven) Li
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 0/6] additions to support tunnel encap/decap Declan Doherty
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 1/6] ethdev: Add tunnel encap/decap actions Declan Doherty
2018-04-19 13:03 ` Adrien Mazarguil
2018-04-23 11:00 ` Shahaf Shuler
2018-04-23 11:17 ` Doherty, Declan
2018-04-23 11:49 ` Adrien Mazarguil
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 2/6] ethdev: Add jump action type to rte_flow Declan Doherty
2018-04-19 13:03 ` Adrien Mazarguil [this message]
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 3/6] testpmd: add jump action Declan Doherty
2018-04-19 13:03 ` Adrien Mazarguil
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 4/6] ethdev: add mark flow item to flow item types Declan Doherty
2018-04-19 13:03 ` Adrien Mazarguil
2018-04-23 11:10 ` Shahaf Shuler
2018-04-23 11:49 ` Adrien Mazarguil
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 5/6] testpmd: add support for MARK flow item Declan Doherty
2018-04-19 13:03 ` Adrien Mazarguil
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 6/6] ethdev: add shared counter support to rte_flow Declan Doherty
2018-04-19 13:03 ` Adrien Mazarguil
2018-04-23 11:11 ` [dpdk-dev] [PATCH v4 0/6] additions to support tunnel encap/decap Shahaf Shuler
2018-04-23 11:13 ` Doherty, Declan
2018-04-23 15:56 ` [dpdk-dev] [PATCH v5 0/4] ethdev " Declan Doherty
2018-04-23 15:56 ` [dpdk-dev] [PATCH v5 1/4] ethdev: Add tunnel encap/decap actions Declan Doherty
2018-04-23 15:56 ` [dpdk-dev] [PATCH v5 2/4] ethdev: Add group JUMP action Declan Doherty
2018-04-23 15:56 ` [dpdk-dev] [PATCH v5 3/4] ethdev: add mark flow item to rte_flow_item_types Declan Doherty
2018-04-23 15:56 ` [dpdk-dev] [PATCH v5 4/4] ethdev: add shared counter support to rte_flow Declan Doherty
2018-04-24 16:26 ` [dpdk-dev] [PATCH v5 0/4] ethdev additions to support tunnel encap/decap Thomas Monjalon
2018-04-30 13:54 ` Thomas Monjalon
2018-04-25 22:05 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180419130319.GZ4957@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=alejandro.lucero@netronome.com \
--cc=alexr@mellanox.com \
--cc=arybchenko@solarflare.com \
--cc=bruce.richardson@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=jingjing.wu@intel.com \
--cc=john.mcnamara@intel.com \
--cc=konstantin.ananyev@intel.com \
--cc=mohammad.abdul.awal@intel.com \
--cc=qi.z.zhang@intel.com \
--cc=remy.horton@intel.com \
--cc=ronye@mellanox.com \
--cc=shahafs@mellanox.com \
--cc=thomas@monjalon.net \
--cc=vincent.jardin@6wind.com \
--cc=wenzhuo.lu@intel.com \
--cc=yliu@fridaylinux.org \
--cc=zhihong.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).