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 1/6] ethdev: Add tunnel encap/decap actions
Date: Thu, 19 Apr 2018 15:03:07 +0200 [thread overview]
Message-ID: <20180419130307.GY4957@6wind.com> (raw)
In-Reply-To: <20180418210423.13847-2-declan.doherty@intel.com>
On Wed, Apr 18, 2018 at 10:04:18PM +0100, Declan Doherty wrote:
> Add new flow action types and associated action data structure to
> support the encapsulation and decapsulation of VXLAN/NVGRE tunnel
> endpoints.
>
> The RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP or
> RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP actions will cause the matching
> flow to be encapsulated in the tunnel endpoint overlay
> defined in the rte_flow_action_tunnel_encap action definition.
>
> The RTE_FLOW_ACTION_TYPE_VXLAN_DECAP and
> RTE_FLOW_ACTION_TYPE_NVGRE_DECAP actions will cause all headers
> associated with the outermost VXLAN/NVGRE tunnel overlay to be
> decapsulated from the matching flow.
>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
So far so good, more comments below.
> ---
> doc/guides/prog_guide/rte_flow.rst | 89 ++++++++++++++++++++++++++++++++++++++
> lib/librte_ether/rte_flow.h | 67 +++++++++++++++++++++++++++-
> 2 files changed, 155 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 961943dda..672473d33 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1486,6 +1486,95 @@ fields in the pattern items.
> | 1 | END |
> +-------+----------+
>
> +Action: ``VXLAN_ENCAP``
> +^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Performs a VXLAN encapsulation action by encapsulating the matched flow in the
> +VXLAN tunnel as defined in the``rte_flow_action_tunnel_encap`` flow items
> +definition.
Missing space before ``rte_flow_action_tunnel_encap``. However please define
a dedicated rte_flow_action_vxlan_encap and another for nvgre instead, even
if their contents are identical, in order to keep them synchronized with
their type name. There's not much overhead involved and it helps clarifying
the API.
> +
> +This action modifies the payload of matched flows. The flow definition specified
> +in the ``rte_flow_action_tunnel_encap`` action structure must defined a valid
defined => define
> +VLXAN network overlay which conforms with RFC 7348 (Virtual eXtensible Local Area
> +Network (VXLAN): A Framework for Overlaying Virtualized Layer 2 Networks over
> +Layer 3 Networks). The pattern must be terminated with
> +the RTE_FLOW_ITEM_TYPE_END item type.
> +
> +- Non-terminating by default.
> +
> +.. _table_rte_flow_action_tunnel_encap:
> +
> +.. table:: TUNNEL_ENCAP
> +
> + +----------------+-------------------------------------+
> + | Field | Value |
> + +================+=====================================+
> + | ``definition`` | Tunnel end-point overlay definition |
> + +----------------+-------------------------------------+
> +
> +.. _table_rte_flow_action_tunnel_encap_example:
> +
> +.. table:: IPv4 VxLAN flow pattern example.
> +
> + +-------+----------+
> + | Index | Item |
> + +=======+==========+
> + | 0 | Ethernet |
> + +-------+----------+
> + | 1 | IPv4 |
> + +-------+----------+
> + | 2 | UDP |
> + +-------+----------+
> + | 3 | VXLAN |
> + +-------+----------+
> + | 4 | END |
> + +-------+----------+
> +
> +Action: ``VXLAN_DECAP``
> +^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Performs a decapsulation action by stripping all headers of the VXLAN tunnel
> +network overlay from the matched flow.
> +
> +This action modifies the payload of matched flows.
You should document "undefined behavior" when this action faces traffic
which does not contain a recognized VXLAN header.
> +
> +Action: ``NVGRE_ENCAP``
> +^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Performs a NVGRE encapsulation action by encapsulating the matched flow in the
> +NVGRE tunnel as defined in the``rte_flow_action_tunnel_encap`` flow item
> +definition.
> +
> +This action modifies the payload of matched flows. The flow definition specified
> +in the ``rte_flow_action_tunnel_encap`` action structure must defined a valid
> +NVGRE network overlay which conforms with RFC 7637 (NVGRE: Network Virtualization
> +Using Generic Routing Encapsulation). The pattern must be terminated with
> +the RTE_FLOW_ITEM_TYPE_END item type.
Same comment as for VXLAN.
> +
> +.. _table_rte_flow_action_tunnel_encap_example:
> +
> +.. table:: IPv4 NVGRE flow pattern example.
> +
> + +-------+----------+
> + | Index | Item |
> + +=======+==========+
> + | 0 | Ethernet |
> + +-------+----------+
> + | 1 | IPv4 |
> + +-------+----------+
> + | 2 | NVGRE |
> + +-------+----------+
> + | 3 | END |
> + +-------+----------+
> +
> +Action: ``NVGRE_DECAP``
> +^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Performs a decapsulation action by stripping all headers of the NVGRE tunnel
> +network overlay from the matched flow.
> +
> +This action modifies the payload of matched flows.
> +
Ditto.
> Negative types
> ~~~~~~~~~~~~~~
>
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index 56c733451..537e1bfba 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -1010,7 +1010,35 @@ enum rte_flow_action_type {
> *
> * See struct rte_flow_action_security.
> */
> - RTE_FLOW_ACTION_TYPE_SECURITY
> + RTE_FLOW_ACTION_TYPE_SECURITY,
> +
> + /**
> + * Encapsulate flow in VXLAN tunnel as defined in RFC7348
> + * using headers defined in rte_flow_action_tunnel_encap
> + * structure.
> + *
> + * See struct rte_flow_action_tunnel_encap.
> + */
> + RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP,
> +
> + /**
> + * Decapsulate outer most VXLAN tunnel headers from flow
> + */
> + RTE_FLOW_ACTION_TYPE_VXLAN_DECAP,
> +
> + /**
> + * Encapsulate flow in NVGRE tunnel as defined in RFC7637
> + * using header defined in the rte_flow_action_tunnel_encap
> + * structure.
> + *
> + * See struct rte_flow_action_tunnel_encap.
> + */
> + RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP,
> +
> + /**
> + * Decapsulate outer most NVGRE tunnel headers from flow
> + */
> + RTE_FLOW_ACTION_TYPE_NVGRE_DECAP
Please add a trailing comma.
> };
>
> /**
> @@ -1148,6 +1176,43 @@ struct rte_flow_action_security {
> void *security_session; /**< Pointer to security session structure. */
> };
>
> +/**
> + * RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP
> + *
> + * Tunnel end-point encapsulation data definition
> + *
> + * The tunnel definition is provided through the flow item pattern pattern, the
> + * provided pattern must conform with the corresponding RFC for the
> + * tunnel encapsulation action specified by the action type. The flow
> + * definition must be provided in order from the RTE_FLOW_ITEM_TYPE_ETH
> + * definition up the end item which is specified by RTE_FLOW_ITEM_TYPE_END.
> + *
> + * The mask field allows user to specify which fields in the flow item
> + * definitions can be ignored and which have valid data and can be used
> + * verbatim.
> + *
> + * Note: the last field is not used in the definition of a tunnel and can be
> + * ignored.
> + *
> + * For example if the actions type was RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP
> + * then valid patterns would include:
> + *
> + * - ETH / IPV4 / UDP / VXLAN / END
> + * - ETH / VLAN / IPV4 / UDP / VXLAN / END
> + *
> + * some valid examples for RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP include:
> + *
> + * - ETH / IPV4 / NVGRE / END
> + * - ETH / VLAN / IPV4 / NVGRE / END
> + */
The above must be split between VXLAN and NVGRE, since they should come as
distinct structures. In any case, make sure Doxygen is synchronized with its
rte_flow.rst counterpart.
> +struct rte_flow_action_tunnel_encap {
> + struct rte_flow_item *definition;
> + /**<
> + * Encapsulating tunnel definition
> + * (list terminated by the END pattern item).
> + */
Please put this comment before the documented field by replacing the opening
"/**<" with "/**". End result is the same but I find it clearer and more
consistent that way ("/**<" is still fine for one-liners).
> +};
> +
> /**
> * Definition of a single action.
> *
> --
> 2.14.3
>
I'm still unsure about the definition involving a list of items. I imagined
these actions even less generic than that. How about tagging them
EXPERIMENTAL until sorted out through the first PMD implementation?
(We could even provide forward declarations only to prevent applications
from using them in the meantime.)
--
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 [this message]
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
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=20180419130307.GY4957@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).