Re: [dpdk-dev] [PATCH v3 1/2] ethdev: introduce generic copy rte flow action

[Ori Kam <orika@nvidia.com>]

Hi Alexander,

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Wednesday, January 13, 2021 5:39 AM
> Subject: [PATCH v3 1/2] ethdev: introduce generic copy rte flow action
> 
> Implement a generic copy flow API to allow copying of an arbitrary
> header field (as well as mark, metadata or tag) to another item.
> 
> This generic copy mechanism removes the necessity to implement a
> separate RTE Flow action every time we need to modify a new packet
> field in the future. A user-provided value can be used from a
> specified tag/metadata/mark or directly copied from another packet field.
> 
> The item ID is used to specify the desired source/destination packet
> field in order to simplify the API for various encapsulation models.
> Specifying the packet field ID with the needed encapsulation level
> is able to quickly get a packet field for any inner packet header.
> 
> Alternatively, the special ID (ITEM_START) can be used to point to the
> very beginning of a packet. This ID in conjunction with the offset
> parameter provides great flexibility to copy/modify any part of a packet
>  as needed.
> 
> The number of bits to copy as well as the offset to start from can
> be specified to allow a partial copy or dividing a big packet field
> into multiple small items (let's say copying 128 bits of IPv6 to 4 tags).
> 
> RFC:
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.d
> pdk.org%2Fpatch%2F85384%2F&amp;data=04%7C01%7Corika%40nvidia.com%
> 7C57d1832b57bf4d6852e808d8b774c4b9%7C43083d15727340c1b7db39efd9cc
> c17a%7C0%7C0%7C637461059443472688%7CUnknown%7CTWFpbGZsb3d8eyJ
> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> 1000&amp;sdata=7g5wYtlhsN9z2m%2FGMMMGt6rV2bx2z6oFLW1pzGJVGqA%3
> D&amp;reserved=0
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst     | 50 ++++++++++++++++++++++
>  doc/guides/rel_notes/release_21_02.rst |  5 +++
>  lib/librte_ethdev/rte_flow.c           |  1 +
>  lib/librte_ethdev/rte_flow.h           | 59 ++++++++++++++++++++++++++
>  4 files changed, 115 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 86b3444803..d6420edc83 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2766,6 +2766,56 @@ The behaviour of the shared action defined by
> ``action`` argument of type
>     | no properties |
>     +---------------+
> 
> +Action: ``COPY_ITEM``
> +^^^^^^^^^^^^^^^^^^^^^
> +
> +Copy ``width`` bits from ``src`` item to ``dst`` item.
> +
> +Any arbitrary header field (as well as mark, metadata or tag values)
> +can be used as both source and destination items as set by ``item``.
> +``RTE_FLOW_ITEM_START`` is used to point to the beginning of a packet.
> +The copy is ignored in case the item specified is not present in a packet.
> +
> + ``level`` is used to access any packet field on any encapsulation level
> + as well as any tag element in the tag array.
> +- ``0`` means the default behaviour. Depending on the packet type, it can
> +  mean outermost, innermost or anything in between.
> +- ``1`` requests access to the outermost packet encapsulation level.
> +- ``2`` and subsequent values requests access to the specified packet
> +  encapsulation level, from outermost to innermost (lower to higher values).
> +  For the tag array ``level`` translates directly into the array index.
> +
> +``offset`` specifies the number of bits to skip from an item start.
> +That allows performing a partial copy of the needed part or to divide a big
> +packet field into multiple smaller items. Alternatively, ``offset`` allows
> +going past the specified packet field boundary to copy an item to an
> +arbitrary place in a packet essentially providing a way to copy any part of
> +a packet to any other part of it if supported by a underlying PMD driver.
> +
> +.. _table_rte_flow_action_copy_item:
> +
> +.. table:: COPY_ITEM
> +
> +   +-----------------------------------------+
> +   | Field         | Value                   |
> +   +===============+=========================+
> +   | ``dst``       | destination item        |
> +   | ``src``       | source item             |
> +   | ``width``     | number of bits to copy  |
> +   +---------------+-------------------------+
> +
> +.. _table_rte_flow_action_copy_data:
> +
> +.. table:: destination/source item definition
> +
> +   +--------------------------------------------------------------------------+
> +   | Field         | Value                                                    |
> +
> +===============+===============================================
> ===========+
> +   | ``item``      | item ID of a packet field or mark/metadata/tag           |
> +   | ``level``     | encapsulation level of a packet field or tag array index |
> +   | ``offset``    | number of bits to skip at the beginning during the copy  |
> +   +---------------+----------------------------------------------------------+
> +
>  Negative types
>  ~~~~~~~~~~~~~~
> 
> diff --git a/doc/guides/rel_notes/release_21_02.rst
> b/doc/guides/rel_notes/release_21_02.rst
> index 706cbf8f0c..6bc482a6bf 100644
> --- a/doc/guides/rel_notes/release_21_02.rst
> +++ b/doc/guides/rel_notes/release_21_02.rst
> @@ -55,6 +55,11 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
> 
> +* **Added support of copy action in the flow API.**
> +
> +  Added copy action support to copy any arbitrary header field
> +  (as well as mark, metadata or tag values) to another one:
> +  ``RTE_FLOW_ACTION_TYPE_COPY_ITEM``.
> 
>  Removed Items
>  -------------
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index a06f64c271..fdbabefc47 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -176,6 +176,7 @@ static const struct rte_flow_desc_data
> rte_flow_desc_action[] = {
>  	MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct
> rte_flow_action_set_dscp)),
>  	MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
>  	MK_FLOW_ACTION(SAMPLE, sizeof(struct rte_flow_action_sample)),
> +	MK_FLOW_ACTION(COPY_ITEM, sizeof(struct
> rte_flow_action_copy_item)),
>  	/**
>  	 * Shared action represented as handle of type
>  	 * (struct rte_flow_shared action *) stored in conf field (see
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 0977a78270..794e59f99b 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -2198,6 +2198,16 @@ enum rte_flow_action_type {
>  	 * struct rte_flow_shared_action).
>  	 */
>  	RTE_FLOW_ACTION_TYPE_SHARED,
> +
> +	/**
> +	 * Copy a packet header field, tag, mark or metadata.
> +	 *
> +	 * Allow saving an arbitrary header field by copying its value
> +	 * to a tag/mark/metadata or copy it into another header field.
> +	 *
> +	 * See struct rte_flow_action_copy_item.
> +	 */
> +	RTE_FLOW_ACTION_TYPE_COPY_ITEM,
>  };
> 
>  /**
> @@ -2791,6 +2801,55 @@ struct rte_flow_action_set_dscp {
>   */
>  struct rte_flow_shared_action;
> 
> +enum rte_flow_item_id {
> +	RTE_FLOW_ITEM_START = 0,
> +	RTE_FLOW_ITEM_MAC_DST,
> +	RTE_FLOW_ITEM_MAC_SRC,
> +	RTE_FLOW_ITEM_VLAN_TYPE,
> +	RTE_FLOW_ITEM_VLAN_ID,
> +	RTE_FLOW_ITEM_MAC_TYPE,
> +	RTE_FLOW_ITEM_IPV4_DSCP,
> +	RTE_FLOW_ITEM_IPV4_TTL,
> +	RTE_FLOW_ITEM_IPV4_SRC,
> +	RTE_FLOW_ITEM_IPV4_DST,
> +	RTE_FLOW_ITEM_IPV6_HOPLIMIT,
> +	RTE_FLOW_ITEM_IPV6_SRC,
> +	RTE_FLOW_ITEM_IPV6_DST,
> +	RTE_FLOW_ITEM_TCP_PORT_SRC,
> +	RTE_FLOW_ITEM_TCP_PORT_DST,
> +	RTE_FLOW_ITEM_TCP_SEQ_NUM,
> +	RTE_FLOW_ITEM_TCP_ACK_NUM,
> +	RTE_FLOW_ITEM_TCP_FLAGS,
> +	RTE_FLOW_ITEM_UDP_PORT_SRC,
> +	RTE_FLOW_ITEM_UDP_PORT_DST,
> +	RTE_FLOW_ITEM_VXLAN_VNI,
> +	RTE_FLOW_ITEM_GENEVE_VNI,
> +	RTE_FLOW_ITEM_GTP_TEID,
> +	RTE_FLOW_ITEM_TAG,
> +	RTE_FLOW_ITEM_MARK,
> +	RTE_FLOW_ITEM_META,
> +};
> +
Please change the names, RTE_FLOW_ITEM_XXX means that it can be
used as rte_flow item this is not the case here.
Maybe RTE_FLOW_FIELD_XXX?


> +struct rte_flow_action_copy_data {
> +	enum rte_flow_item_id item;
> +	uint32_t level;
> +	uint32_t offset;
> +};
> +
> +/**
> + * RTE_FLOW_ACTION_TYPE_COPY_ITEM
> + *
> + * Copies a specified number of bits from a source header field
> + * to a destination header field. Tag, mark or metadata can also
> + * be used as a source/destination to allow saving/overwriting
> + * an arbitrary header field with a user-specified value.
> + */
> +struct rte_flow_action_copy_item {
> +	struct rte_flow_action_copy_data dst;
> +	struct rte_flow_action_copy_data src;
> +	uint32_t width;
> +};
> +
>  /* Mbuf dynamic field offset for metadata. */
>  extern int32_t rte_flow_dynf_metadata_offs;
> 
> --
> 2.24.1