DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
@ 2020-12-18  1:31 Alexander Kozyrev
  2021-01-05 22:12 ` Alexander Kozyrev
  2021-01-05 22:16 ` Thomas Monjalon
  0 siblings, 2 replies; 16+ messages in thread
From: Alexander Kozyrev @ 2020-12-18  1:31 UTC (permalink / raw)
  To: dev; +Cc: viacheslavo, orika, thomas, ferruh.yigit, andrew.rybchenko

RTE Flows API lacks the ability to save an arbitrary header field in
order to use it later for advanced packet manipulations. Examples
include the usage of VxLAN ID after the packet is decapsulated or
storing this ID inside the packet payload itself or swapping an
arbitrary inner and outer packet fields.

The idea is to allow a copy of a specified number of bits form any
packet header field into another header field:
RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.

struct rte_flow_action_copy_field {
	struct rte_flow_action_copy_data dest;
	struct rte_flow_action_copy_data src;
	uint16_t width;
};

Arbitrary header field (as well as mark, metadata or tag values) can be
used as both source and destination fields. This way we can save an
arbitrary header field by copying its value to a tag/mark/metadata or
copy it into another header field directly. tag/mark/metadata can also
be used as a value to be stored in an arbitrary packet header field.

struct rte_flow_action_copy_data {
	enum rte_flow_field_id field;
	uint16_t index;
	uint16_t offset;
};

The rte_flow_field_id specifies the particular packet field (or
tag/mark/metadata) to be used as a copy source or destination.
The index gives access to inner packet headers or elements in the tags
array. The offset allows to copy a packet field value into the payload.

It is proposed to implement the "set copy_field" command to store all
the required parameters and then to use this template by specifying the
index of the needed copy action. For example, to modify the GTP tunnel
ID after the packet is encapsulated following testpmd rules are used:

      set copy_field width 32 src field tag index 1 offset 0
        dst field teid index 0 offset 0
      flow create 0 ingress pattern ... / end
        raw_decap index 1 / raw_encap index 2 /
        copy_field index 1 / end

A more generic mechanism to overwrite an arbitrary header field may be
introduced to the RTE flows implementation later:
RTE_FLOW_ACTION_TYPE_SET_FIELD with the structure defined below.

struct rte_flow_action_copy_field {
	struct rte_flow_action_copy_data dest;
	uint8_t *data;
	uint16_t width;
};

This way we can have the generic way to specify an immediate value and
use it as data for any packet header field instead of having separate
RTE Flow action for each of the packet fields. Deprecation notice may
be issued to RTE_FLOW_ACTION_TYPE_SET_XXX actions after the unified
method of setting a value to any packet field is implemented.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 lib/librte_ethdev/rte_flow.c |  1 +
 lib/librte_ethdev/rte_flow.h | 59 ++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index a06f64c271..c3154a29e2 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_FIELD, sizeof(struct rte_flow_action_copy_field)),
 	/**
 	 * 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..8f1798487c 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_field.
+	 */
+	RTE_FLOW_ACTION_TYPE_COPY_FIELD,
 };
 
 /**
@@ -2791,6 +2801,55 @@ struct rte_flow_action_set_dscp {
  */
 struct rte_flow_shared_action;
 
+enum rte_flow_field_id {
+	RTE_FLOW_FIELD_NONE = 0,
+	RTE_FLOW_FIELD_MAC_DST,
+	RTE_FLOW_FIELD_MAC_SRC,
+	RTE_FLOW_FIELD_VLAN_TYPE,
+	RTE_FLOW_FIELD_VLAN_ID,
+	RTE_FLOW_FIELD_MAC_TYPE,
+	RTE_FLOW_FIELD_IPV4_DSCP,
+	RTE_FLOW_FIELD_IPV4_TTL,
+	RTE_FLOW_FIELD_IPV4_SRC,
+	RTE_FLOW_FIELD_IPV4_DST,
+	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
+	RTE_FLOW_FIELD_IPV6_SRC,
+	RTE_FLOW_FIELD_IPV6_DST,
+	RTE_FLOW_FIELD_TCP_PORT_SRC,
+	RTE_FLOW_FIELD_TCP_PORT_DST,
+	RTE_FLOW_FIELD_TCP_SEQ_NUM,
+	RTE_FLOW_FIELD_TCP_ACK_NUM,
+	RTE_FLOW_FIELD_TCP_FLAGS,
+	RTE_FLOW_FIELD_UDP_PORT_SRC,
+	RTE_FLOW_FIELD_UDP_PORT_DST,
+	RTE_FLOW_FIELD_VXLAN_VNI,
+	RTE_FLOW_FIELD_GENEVE_VNI,
+	RTE_FLOW_FIELD_GTP_TEID,
+	RTE_FLOW_FIELD_TAG,
+	RTE_FLOW_FIELD_MARK,
+	RTE_FLOW_FIELD_META,
+};
+
+struct rte_flow_action_copy_data {
+	enum rte_flow_field_id field;
+	uint16_t index;
+	uint16_t offset;
+};
+
+/**
+ * RTE_FLOW_ACTION_TYPE_COPY_FIELD
+ *
+ * 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 arbituary header field with a user-specified value.
+ */
+struct rte_flow_action_copy_field {
+	struct rte_flow_action_copy_data dest;
+	struct rte_flow_action_copy_data src;
+	uint16_t width;
+};
+
 /* Mbuf dynamic field offset for metadata. */
 extern int32_t rte_flow_dynf_metadata_offs;
 
-- 
2.24.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2020-12-18  1:31 [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action Alexander Kozyrev
@ 2021-01-05 22:12 ` Alexander Kozyrev
  2021-01-05 22:18   ` Thomas Monjalon
  2021-01-05 22:16 ` Thomas Monjalon
  1 sibling, 1 reply; 16+ messages in thread
From: Alexander Kozyrev @ 2021-01-05 22:12 UTC (permalink / raw)
  To: Alexander Kozyrev, dev
  Cc: Slava Ovsiienko, Ori Kam, NBU-Contact-Thomas Monjalon,
	ferruh.yigit, andrew.rybchenko

Happy New Year Gentlemen, would you mind to share your thought on the proposed changes?
I was thinking about renaming it to more general "copy item" to cover all packet fields, tags  and metadata.
Other than that it is ready for a formal patch integration in my opinion if there are no objections. Please advise.

Regards,
Alex

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Alexander Kozyrev
> Sent: Thursday, December 17, 2020 20:31
> To: dev@dpdk.org
> Cc: Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>;
> NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com; andrew.rybchenko@oktetlabs.ru
> Subject: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
> 
> RTE Flows API lacks the ability to save an arbitrary header field in
> order to use it later for advanced packet manipulations. Examples
> include the usage of VxLAN ID after the packet is decapsulated or
> storing this ID inside the packet payload itself or swapping an
> arbitrary inner and outer packet fields.
> 
> The idea is to allow a copy of a specified number of bits form any
> packet header field into another header field:
> RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.
> 
> struct rte_flow_action_copy_field {
> 	struct rte_flow_action_copy_data dest;
> 	struct rte_flow_action_copy_data src;
> 	uint16_t width;
> };
> 
> Arbitrary header field (as well as mark, metadata or tag values) can be
> used as both source and destination fields. This way we can save an
> arbitrary header field by copying its value to a tag/mark/metadata or
> copy it into another header field directly. tag/mark/metadata can also
> be used as a value to be stored in an arbitrary packet header field.
> 
> struct rte_flow_action_copy_data {
> 	enum rte_flow_field_id field;
> 	uint16_t index;
> 	uint16_t offset;
> };
> 
> The rte_flow_field_id specifies the particular packet field (or
> tag/mark/metadata) to be used as a copy source or destination.
> The index gives access to inner packet headers or elements in the tags
> array. The offset allows to copy a packet field value into the payload.
> 
> It is proposed to implement the "set copy_field" command to store all
> the required parameters and then to use this template by specifying the
> index of the needed copy action. For example, to modify the GTP tunnel
> ID after the packet is encapsulated following testpmd rules are used:
> 
>       set copy_field width 32 src field tag index 1 offset 0
>         dst field teid index 0 offset 0
>       flow create 0 ingress pattern ... / end
>         raw_decap index 1 / raw_encap index 2 /
>         copy_field index 1 / end
> 
> A more generic mechanism to overwrite an arbitrary header field may be
> introduced to the RTE flows implementation later:
> RTE_FLOW_ACTION_TYPE_SET_FIELD with the structure defined below.
> 
> struct rte_flow_action_copy_field {
> 	struct rte_flow_action_copy_data dest;
> 	uint8_t *data;
> 	uint16_t width;
> };
> 
> This way we can have the generic way to specify an immediate value and
> use it as data for any packet header field instead of having separate
> RTE Flow action for each of the packet fields. Deprecation notice may
> be issued to RTE_FLOW_ACTION_TYPE_SET_XXX actions after the unified
> method of setting a value to any packet field is implemented.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  lib/librte_ethdev/rte_flow.c |  1 +
>  lib/librte_ethdev/rte_flow.h | 59 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index a06f64c271..c3154a29e2 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_FIELD, sizeof(struct
> rte_flow_action_copy_field)),
>  	/**
>  	 * 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..8f1798487c 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_field.
> +	 */
> +	RTE_FLOW_ACTION_TYPE_COPY_FIELD,
>  };
> 
>  /**
> @@ -2791,6 +2801,55 @@ struct rte_flow_action_set_dscp {
>   */
>  struct rte_flow_shared_action;
> 
> +enum rte_flow_field_id {
> +	RTE_FLOW_FIELD_NONE = 0,
> +	RTE_FLOW_FIELD_MAC_DST,
> +	RTE_FLOW_FIELD_MAC_SRC,
> +	RTE_FLOW_FIELD_VLAN_TYPE,
> +	RTE_FLOW_FIELD_VLAN_ID,
> +	RTE_FLOW_FIELD_MAC_TYPE,
> +	RTE_FLOW_FIELD_IPV4_DSCP,
> +	RTE_FLOW_FIELD_IPV4_TTL,
> +	RTE_FLOW_FIELD_IPV4_SRC,
> +	RTE_FLOW_FIELD_IPV4_DST,
> +	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
> +	RTE_FLOW_FIELD_IPV6_SRC,
> +	RTE_FLOW_FIELD_IPV6_DST,
> +	RTE_FLOW_FIELD_TCP_PORT_SRC,
> +	RTE_FLOW_FIELD_TCP_PORT_DST,
> +	RTE_FLOW_FIELD_TCP_SEQ_NUM,
> +	RTE_FLOW_FIELD_TCP_ACK_NUM,
> +	RTE_FLOW_FIELD_TCP_FLAGS,
> +	RTE_FLOW_FIELD_UDP_PORT_SRC,
> +	RTE_FLOW_FIELD_UDP_PORT_DST,
> +	RTE_FLOW_FIELD_VXLAN_VNI,
> +	RTE_FLOW_FIELD_GENEVE_VNI,
> +	RTE_FLOW_FIELD_GTP_TEID,
> +	RTE_FLOW_FIELD_TAG,
> +	RTE_FLOW_FIELD_MARK,
> +	RTE_FLOW_FIELD_META,
> +};
> +
> +struct rte_flow_action_copy_data {
> +	enum rte_flow_field_id field;
> +	uint16_t index;
> +	uint16_t offset;
> +};
> +
> +/**
> + * RTE_FLOW_ACTION_TYPE_COPY_FIELD
> + *
> + * 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 arbituary header field with a user-specified value.
> + */
> +struct rte_flow_action_copy_field {
> +	struct rte_flow_action_copy_data dest;
> +	struct rte_flow_action_copy_data src;
> +	uint16_t width;
> +};
> +
>  /* Mbuf dynamic field offset for metadata. */
>  extern int32_t rte_flow_dynf_metadata_offs;
> 
> --
> 2.24.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2020-12-18  1:31 [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action Alexander Kozyrev
  2021-01-05 22:12 ` Alexander Kozyrev
@ 2021-01-05 22:16 ` Thomas Monjalon
  2021-01-07 14:17   ` Alexander Kozyrev
  1 sibling, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-05 22:16 UTC (permalink / raw)
  To: Alexander Kozyrev; +Cc: dev, viacheslavo, orika, ferruh.yigit, andrew.rybchenko

18/12/2020 02:31, Alexander Kozyrev:
> RTE Flows API lacks the ability to save an arbitrary header field in
> order to use it later for advanced packet manipulations. Examples
> include the usage of VxLAN ID after the packet is decapsulated or
> storing this ID inside the packet payload itself or swapping an
> arbitrary inner and outer packet fields.
> 
> The idea is to allow a copy of a specified number of bits form any
> packet header field into another header field:
> RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.
> 
> struct rte_flow_action_copy_field {
> 	struct rte_flow_action_copy_data dest;
> 	struct rte_flow_action_copy_data src;
> 	uint16_t width;
> };
> 
> Arbitrary header field (as well as mark, metadata or tag values) can be
> used as both source and destination fields. This way we can save an
> arbitrary header field by copying its value to a tag/mark/metadata or
> copy it into another header field directly. tag/mark/metadata can also
> be used as a value to be stored in an arbitrary packet header field.
> 
> struct rte_flow_action_copy_data {
> 	enum rte_flow_field_id field;
> 	uint16_t index;
> 	uint16_t offset;
> };
> 
> The rte_flow_field_id specifies the particular packet field (or
> tag/mark/metadata) to be used as a copy source or destination.
> The index gives access to inner packet headers or elements in the tags
> array. The offset allows to copy a packet field value into the payload.

So index is in reality the layer? How is it numbered exactly?
What is the field id if an offset is given?
Can we say that a field id can always be replaced by an offset?


> It is proposed to implement the "set copy_field" command to store all

You are talking about testpmd here?
It looks unrelated to this patch and not sure it helps understanding.

> the required parameters and then to use this template by specifying the
> index of the needed copy action. For example, to modify the GTP tunnel
> ID after the packet is encapsulated following testpmd rules are used:
> 
>       set copy_field width 32 src field tag index 1 offset 0
>         dst field teid index 0 offset 0
>       flow create 0 ingress pattern ... / end
>         raw_decap index 1 / raw_encap index 2 /
>         copy_field index 1 / end
> 
> A more generic mechanism to overwrite an arbitrary header field may be
> introduced to the RTE flows implementation later:
> RTE_FLOW_ACTION_TYPE_SET_FIELD with the structure defined below.
> 
> struct rte_flow_action_copy_field {
> 	struct rte_flow_action_copy_data dest;
> 	uint8_t *data;
> 	uint16_t width;
> };
> 
> This way we can have the generic way to specify an immediate value and
> use it as data for any packet header field instead of having separate
> RTE Flow action for each of the packet fields. Deprecation notice may
> be issued to RTE_FLOW_ACTION_TYPE_SET_XXX actions after the unified
> method of setting a value to any packet field is implemented.

Yes having a single action for setting any field looks to be a good idea.


> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> ---
>  lib/librte_ethdev/rte_flow.c |  1 +

We are very close to the -rc1 date and implemention is missing.
Please update.

>  lib/librte_ethdev/rte_flow.h | 59 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+)

[...]
> +enum rte_flow_field_id {
> +	RTE_FLOW_FIELD_NONE = 0,
> +	RTE_FLOW_FIELD_MAC_DST,
> +	RTE_FLOW_FIELD_MAC_SRC,
> +	RTE_FLOW_FIELD_VLAN_TYPE,
> +	RTE_FLOW_FIELD_VLAN_ID,
> +	RTE_FLOW_FIELD_MAC_TYPE,
> +	RTE_FLOW_FIELD_IPV4_DSCP,
> +	RTE_FLOW_FIELD_IPV4_TTL,
> +	RTE_FLOW_FIELD_IPV4_SRC,
> +	RTE_FLOW_FIELD_IPV4_DST,
> +	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
> +	RTE_FLOW_FIELD_IPV6_SRC,
> +	RTE_FLOW_FIELD_IPV6_DST,
> +	RTE_FLOW_FIELD_TCP_PORT_SRC,
> +	RTE_FLOW_FIELD_TCP_PORT_DST,
> +	RTE_FLOW_FIELD_TCP_SEQ_NUM,
> +	RTE_FLOW_FIELD_TCP_ACK_NUM,
> +	RTE_FLOW_FIELD_TCP_FLAGS,
> +	RTE_FLOW_FIELD_UDP_PORT_SRC,
> +	RTE_FLOW_FIELD_UDP_PORT_DST,
> +	RTE_FLOW_FIELD_VXLAN_VNI,
> +	RTE_FLOW_FIELD_GENEVE_VNI,
> +	RTE_FLOW_FIELD_GTP_TEID,
> +	RTE_FLOW_FIELD_TAG,
> +	RTE_FLOW_FIELD_MARK,
> +	RTE_FLOW_FIELD_META,
> +};

I don't really like having to list all fields of the world,
but it's probably better than the current situation of
creating a new action for each field.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-05 22:12 ` Alexander Kozyrev
@ 2021-01-05 22:18   ` Thomas Monjalon
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-05 22:18 UTC (permalink / raw)
  To: Alexander Kozyrev
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko

05/01/2021 23:12, Alexander Kozyrev:
> Happy New Year Gentlemen, would you mind to share your thought on the proposed changes?
> I was thinking about renaming it to more general "copy item" to cover all packet fields, tags  and metadata.
> Other than that it is ready for a formal patch integration in my opinion if there are no objections. Please advise.

Our mails crossed, I've just replied to the RFC :)
Yes we need to progress if we want to meet the deadline.
More reviews may help.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-05 22:16 ` Thomas Monjalon
@ 2021-01-07 14:17   ` Alexander Kozyrev
  2021-01-07 15:06     ` Thomas Monjalon
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Kozyrev @ 2021-01-07 14:17 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko

> Tuesday, January 5, 2021 17:17, Thomas Monjalon <thomas@monjalon.net>
> > RTE Flows API lacks the ability to save an arbitrary header field in
> > order to use it later for advanced packet manipulations. Examples
> > include the usage of VxLAN ID after the packet is decapsulated or
> > storing this ID inside the packet payload itself or swapping an
> > arbitrary inner and outer packet fields.
> >
> > The idea is to allow a copy of a specified number of bits form any
> > packet header field into another header field:
> > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.
> >
> > struct rte_flow_action_copy_field {
> > 	struct rte_flow_action_copy_data dest;
> > 	struct rte_flow_action_copy_data src;
> > 	uint16_t width;
> > };
> >
> > Arbitrary header field (as well as mark, metadata or tag values) can be
> > used as both source and destination fields. This way we can save an
> > arbitrary header field by copying its value to a tag/mark/metadata or
> > copy it into another header field directly. tag/mark/metadata can also
> > be used as a value to be stored in an arbitrary packet header field.
> >
> > struct rte_flow_action_copy_data {
> > 	enum rte_flow_field_id field;
> > 	uint16_t index;
> > 	uint16_t offset;
> > };
> >
> > The rte_flow_field_id specifies the particular packet field (or
> > tag/mark/metadata) to be used as a copy source or destination.
> > The index gives access to inner packet headers or elements in the tags
> > array. The offset allows to copy a packet field value into the payload.
> 
> So index is in reality the layer? How is it numbered exactly?
It is a layer for packet fields, inner headers get higher number index.
But is it also an index in the TAG array, so the name comes from it.

> What is the field id if an offset is given?
Field ID stays the same, you can specify a small offset to copy just a few bits
from the entire packet field or a big offset to move to completely different area.

> Can we say that a field id can always be replaced by an offset?
Not really. You can use offset to jump around packet fields for sure, but it is going to be
hard and cumbersome to calculate all the offsets for that. Field ID is much more convenient.

> > It is proposed to implement the "set copy_field" command to store all
> 
> You are talking about testpmd here?
> It looks unrelated to this patch and not sure it helps understanding.
I'm working on testpmd implementation and will send it shortly.

> > the required parameters and then to use this template by specifying the
> > index of the needed copy action. For example, to modify the GTP tunnel
> > ID after the packet is encapsulated following testpmd rules are used:
> >
> >       set copy_field width 32 src field tag index 1 offset 0
> >         dst field teid index 0 offset 0
> >       flow create 0 ingress pattern ... / end
> >         raw_decap index 1 / raw_encap index 2 /
> >         copy_field index 1 / end
> >
> > A more generic mechanism to overwrite an arbitrary header field may be
> > introduced to the RTE flows implementation later:
> > RTE_FLOW_ACTION_TYPE_SET_FIELD with the structure defined below.
> >
> > struct rte_flow_action_copy_field {
> > 	struct rte_flow_action_copy_data dest;
> > 	uint8_t *data;
> > 	uint16_t width;
> > };
> >
> > This way we can have the generic way to specify an immediate value and
> > use it as data for any packet header field instead of having separate
> > RTE Flow action for each of the packet fields. Deprecation notice may
> > be issued to RTE_FLOW_ACTION_TYPE_SET_XXX actions after the unified
> > method of setting a value to any packet field is implemented.
> 
> Yes having a single action for setting any field looks to be a good idea.
Thanks.

> 
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> > ---
> >  lib/librte_ethdev/rte_flow.c |  1 +
> 
> We are very close to the -rc1 date and implemention is missing.
> Please update.
> 
> >  lib/librte_ethdev/rte_flow.h | 59 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+)
> 
> [...]
> > +enum rte_flow_field_id {
> > +	RTE_FLOW_FIELD_NONE = 0,
> > +	RTE_FLOW_FIELD_MAC_DST,
> > +	RTE_FLOW_FIELD_MAC_SRC,
> > +	RTE_FLOW_FIELD_VLAN_TYPE,
> > +	RTE_FLOW_FIELD_VLAN_ID,
> > +	RTE_FLOW_FIELD_MAC_TYPE,
> > +	RTE_FLOW_FIELD_IPV4_DSCP,
> > +	RTE_FLOW_FIELD_IPV4_TTL,
> > +	RTE_FLOW_FIELD_IPV4_SRC,
> > +	RTE_FLOW_FIELD_IPV4_DST,
> > +	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
> > +	RTE_FLOW_FIELD_IPV6_SRC,
> > +	RTE_FLOW_FIELD_IPV6_DST,
> > +	RTE_FLOW_FIELD_TCP_PORT_SRC,
> > +	RTE_FLOW_FIELD_TCP_PORT_DST,
> > +	RTE_FLOW_FIELD_TCP_SEQ_NUM,
> > +	RTE_FLOW_FIELD_TCP_ACK_NUM,
> > +	RTE_FLOW_FIELD_TCP_FLAGS,
> > +	RTE_FLOW_FIELD_UDP_PORT_SRC,
> > +	RTE_FLOW_FIELD_UDP_PORT_DST,
> > +	RTE_FLOW_FIELD_VXLAN_VNI,
> > +	RTE_FLOW_FIELD_GENEVE_VNI,
> > +	RTE_FLOW_FIELD_GTP_TEID,
> > +	RTE_FLOW_FIELD_TAG,
> > +	RTE_FLOW_FIELD_MARK,
> > +	RTE_FLOW_FIELD_META,
> > +};
> 
> I don't really like having to list all fields of the world,
> but it's probably better than the current situation of
> creating a new action for each field.
That is the idea, to get rid of the need to implement another action
every time we want to modify another packet field.
Let me know about a better way if any.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 14:17   ` Alexander Kozyrev
@ 2021-01-07 15:06     ` Thomas Monjalon
  2021-01-07 15:10       ` Alexander Kozyrev
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-07 15:06 UTC (permalink / raw)
  To: Alexander Kozyrev
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

07/01/2021 15:17, Alexander Kozyrev:
> > Tuesday, January 5, 2021 17:17, Thomas Monjalon <thomas@monjalon.net>
> > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > order to use it later for advanced packet manipulations. Examples
> > > include the usage of VxLAN ID after the packet is decapsulated or
> > > storing this ID inside the packet payload itself or swapping an
> > > arbitrary inner and outer packet fields.
> > >
> > > The idea is to allow a copy of a specified number of bits form any
> > > packet header field into another header field:
> > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.
> > >
> > > struct rte_flow_action_copy_field {
> > > 	struct rte_flow_action_copy_data dest;
> > > 	struct rte_flow_action_copy_data src;
> > > 	uint16_t width;
> > > };
> > >
> > > Arbitrary header field (as well as mark, metadata or tag values) can be
> > > used as both source and destination fields. This way we can save an
> > > arbitrary header field by copying its value to a tag/mark/metadata or
> > > copy it into another header field directly. tag/mark/metadata can also
> > > be used as a value to be stored in an arbitrary packet header field.
> > >
> > > struct rte_flow_action_copy_data {
> > > 	enum rte_flow_field_id field;
> > > 	uint16_t index;
> > > 	uint16_t offset;
> > > };
> > >
> > > The rte_flow_field_id specifies the particular packet field (or
> > > tag/mark/metadata) to be used as a copy source or destination.
> > > The index gives access to inner packet headers or elements in the tags
> > > array. The offset allows to copy a packet field value into the payload.
> > 
> > So index is in reality the layer? How is it numbered exactly?
> 
> It is a layer for packet fields, inner headers get higher number index.
> But is it also an index in the TAG array, so the name comes from it.

Sorry it is not obvious.
Please describe the exact numbering in tunnel and VLAN cases.

> > What is the field id if an offset is given?
> 
> Field ID stays the same, you can specify a small offset to copy just a few bits
> from the entire packet field or a big offset to move to completely different area.

I don't understand what is an offset then.
Isn't it the byte or bit where the copy start?
Do you handle sizes smaller than a byte?

> > Can we say that a field id can always be replaced by an offset?
> 
> Not really. You can use offset to jump around packet fields for sure, but it is going to be
> hard and cumbersome to calculate all the offsets for that. Field ID is much more convenient.

I think it depends for who.
For some use cases, it may be easier to pass an offset.
For some drivers, it may be more efficient to directly manage offsets.




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 15:06     ` Thomas Monjalon
@ 2021-01-07 15:10       ` Alexander Kozyrev
  2021-01-07 15:17         ` Thomas Monjalon
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Kozyrev @ 2021-01-07 15:10 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

> > > Thursday, January 7, 2021 10:07, Thomas Monjalon <thomas@monjalon.net>
> > > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > > order to use it later for advanced packet manipulations. Examples
> > > > include the usage of VxLAN ID after the packet is decapsulated or
> > > > storing this ID inside the packet payload itself or swapping an
> > > > arbitrary inner and outer packet fields.
> > > >
> > > > The idea is to allow a copy of a specified number of bits form any
> > > > packet header field into another header field:
> > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.
> > > >
> > > > struct rte_flow_action_copy_field {
> > > > 	struct rte_flow_action_copy_data dest;
> > > > 	struct rte_flow_action_copy_data src;
> > > > 	uint16_t width;
> > > > };
> > > >
> > > > Arbitrary header field (as well as mark, metadata or tag values) can be
> > > > used as both source and destination fields. This way we can save an
> > > > arbitrary header field by copying its value to a tag/mark/metadata or
> > > > copy it into another header field directly. tag/mark/metadata can also
> > > > be used as a value to be stored in an arbitrary packet header field.
> > > >
> > > > struct rte_flow_action_copy_data {
> > > > 	enum rte_flow_field_id field;
> > > > 	uint16_t index;
> > > > 	uint16_t offset;
> > > > };
> > > >
> > > > The rte_flow_field_id specifies the particular packet field (or
> > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > The index gives access to inner packet headers or elements in the tags
> > > > array. The offset allows to copy a packet field value into the payload.
> > >
> > > So index is in reality the layer? How is it numbered exactly?
> >
> > It is a layer for packet fields, inner headers get higher number index.
> > But is it also an index in the TAG array, so the name comes from it.
> 
> Sorry it is not obvious.
> Please describe the exact numbering in tunnel and VLAN cases.
> 
> > > What is the field id if an offset is given?
> >
> > Field ID stays the same, you can specify a small offset to copy just a few bits
> > from the entire packet field or a big offset to move to completely different
> area.
> 
> I don't understand what is an offset then.
> Isn't it the byte or bit where the copy start?
> Do you handle sizes smaller than a byte?
It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address for example.
 
> > > Can we say that a field id can always be replaced by an offset?
> >
> > Not really. You can use offset to jump around packet fields for sure, but it is
> going to be
> > hard and cumbersome to calculate all the offsets for that. Field ID is much
> more convenient.
> 
> I think it depends for who.
> For some use cases, it may be easier to pass an offset.
> For some drivers, it may be more efficient to directly manage offsets.
It is possible with this RFC, driver can choose what to use: id and/or offset.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 15:10       ` Alexander Kozyrev
@ 2021-01-07 15:17         ` Thomas Monjalon
  2021-01-07 15:22           ` Alexander Kozyrev
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-07 15:17 UTC (permalink / raw)
  To: Alexander Kozyrev
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

07/01/2021 16:10, Alexander Kozyrev:
> > > > Thursday, January 7, 2021 10:07, Thomas Monjalon <thomas@monjalon.net>
> > > > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > > > order to use it later for advanced packet manipulations. Examples
> > > > > include the usage of VxLAN ID after the packet is decapsulated or
> > > > > storing this ID inside the packet payload itself or swapping an
> > > > > arbitrary inner and outer packet fields.
> > > > >
> > > > > The idea is to allow a copy of a specified number of bits form any
> > > > > packet header field into another header field:
> > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined below.
> > > > >
> > > > > struct rte_flow_action_copy_field {
> > > > > 	struct rte_flow_action_copy_data dest;
> > > > > 	struct rte_flow_action_copy_data src;
> > > > > 	uint16_t width;
> > > > > };
> > > > >
> > > > > Arbitrary header field (as well as mark, metadata or tag values) can be
> > > > > used as both source and destination fields. This way we can save an
> > > > > arbitrary header field by copying its value to a tag/mark/metadata or
> > > > > copy it into another header field directly. tag/mark/metadata can also
> > > > > be used as a value to be stored in an arbitrary packet header field.
> > > > >
> > > > > struct rte_flow_action_copy_data {
> > > > > 	enum rte_flow_field_id field;
> > > > > 	uint16_t index;
> > > > > 	uint16_t offset;
> > > > > };
> > > > >
> > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > The index gives access to inner packet headers or elements in the tags
> > > > > array. The offset allows to copy a packet field value into the payload.
> > > >
> > > > So index is in reality the layer? How is it numbered exactly?
> > >
> > > It is a layer for packet fields, inner headers get higher number index.
> > > But is it also an index in the TAG array, so the name comes from it.
> > 
> > Sorry it is not obvious.
> > Please describe the exact numbering in tunnel and VLAN cases.
> > 
> > > > What is the field id if an offset is given?
> > >
> > > Field ID stays the same, you can specify a small offset to copy just a few bits
> > > from the entire packet field or a big offset to move to completely different
> > area.
> > 
> > I don't understand what is an offset then.
> > Isn't it the byte or bit where the copy start?
> > Do you handle sizes smaller than a byte?
> 
> It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address for example.

Now I'm confused.
You mean rte_flow_action_copy_data.offset is a bit offset?

> > > > Can we say that a field id can always be replaced by an offset?
> > >
> > > Not really. You can use offset to jump around packet fields for sure, but it is
> > going to be
> > > hard and cumbersome to calculate all the offsets for that. Field ID is much
> > more convenient.
> > 
> > I think it depends for who.
> > For some use cases, it may be easier to pass an offset.
> > For some drivers, it may be more efficient to directly manage offsets.
> 
> It is possible with this RFC, driver can choose what to use: id and/or offset.

We can set field and index to 0, and use only offset?
Then it is a byte offset from the beginning mbuf.data?






^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 15:17         ` Thomas Monjalon
@ 2021-01-07 15:22           ` Alexander Kozyrev
  2021-01-07 16:54             ` Thomas Monjalon
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Kozyrev @ 2021-01-07 15:22 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

> 07/01/2021 16:10, Alexander Kozyrev:
> > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> <thomas@monjalon.net>
> > > > > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > > > > order to use it later for advanced packet manipulations. Examples
> > > > > > include the usage of VxLAN ID after the packet is decapsulated or
> > > > > > storing this ID inside the packet payload itself or swapping an
> > > > > > arbitrary inner and outer packet fields.
> > > > > >
> > > > > > The idea is to allow a copy of a specified number of bits form any
> > > > > > packet header field into another header field:
> > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined
> below.
> > > > > >
> > > > > > struct rte_flow_action_copy_field {
> > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > 	uint16_t width;
> > > > > > };
> > > > > >
> > > > > > Arbitrary header field (as well as mark, metadata or tag values) can be
> > > > > > used as both source and destination fields. This way we can save an
> > > > > > arbitrary header field by copying its value to a tag/mark/metadata or
> > > > > > copy it into another header field directly. tag/mark/metadata can also
> > > > > > be used as a value to be stored in an arbitrary packet header field.
> > > > > >
> > > > > > struct rte_flow_action_copy_data {
> > > > > > 	enum rte_flow_field_id field;
> > > > > > 	uint16_t index;
> > > > > > 	uint16_t offset;
> > > > > > };
> > > > > >
> > > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > > The index gives access to inner packet headers or elements in the tags
> > > > > > array. The offset allows to copy a packet field value into the payload.
> > > > >
> > > > > So index is in reality the layer? How is it numbered exactly?
> > > >
> > > > It is a layer for packet fields, inner headers get higher number index.
> > > > But is it also an index in the TAG array, so the name comes from it.
> > >
> > > Sorry it is not obvious.
> > > Please describe the exact numbering in tunnel and VLAN cases.
> > >
> > > > > What is the field id if an offset is given?
> > > >
> > > > Field ID stays the same, you can specify a small offset to copy just a few
> bits
> > > > from the entire packet field or a big offset to move to completely different
> > > area.
> > >
> > > I don't understand what is an offset then.
> > > Isn't it the byte or bit where the copy start?
> > > Do you handle sizes smaller than a byte?
> >
> > It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address for
> example.
> 
> Now I'm confused.
> You mean rte_flow_action_copy_data.offset is a bit offset?

rte_flow_action_copy_data.offset and rte_flow_action_copy_field.width
are measured in bits, right.


> > > > > Can we say that a field id can always be replaced by an offset?
> > > >
> > > > Not really. You can use offset to jump around packet fields for sure, but it
> is
> > > going to be
> > > > hard and cumbersome to calculate all the offsets for that. Field ID is much
> > > more convenient.
> > >
> > > I think it depends for who.
> > > For some use cases, it may be easier to pass an offset.
> > > For some drivers, it may be more efficient to directly manage offsets.
> >
> > It is possible with this RFC, driver can choose what to use: id and/or offset.
> 

> We can set field and index to 0, and use only offset?
Yes, I'm not inending to put any restrictions against that.
> Then it is a byte offset from the beginning mbuf.data?
Yes, but it is still bit offset, not byte offset.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 15:22           ` Alexander Kozyrev
@ 2021-01-07 16:54             ` Thomas Monjalon
  2021-01-07 16:57               ` Alexander Kozyrev
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-07 16:54 UTC (permalink / raw)
  To: Alexander Kozyrev
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

07/01/2021 16:22, Alexander Kozyrev:
> > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > <thomas@monjalon.net>
> > > > > > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > > > > > order to use it later for advanced packet manipulations. Examples
> > > > > > > include the usage of VxLAN ID after the packet is decapsulated or
> > > > > > > storing this ID inside the packet payload itself or swapping an
> > > > > > > arbitrary inner and outer packet fields.
> > > > > > >
> > > > > > > The idea is to allow a copy of a specified number of bits form any
> > > > > > > packet header field into another header field:
> > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined
> > below.
> > > > > > >
> > > > > > > struct rte_flow_action_copy_field {
> > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > 	uint16_t width;
> > > > > > > };
> > > > > > >
> > > > > > > Arbitrary header field (as well as mark, metadata or tag values) can be
> > > > > > > used as both source and destination fields. This way we can save an
> > > > > > > arbitrary header field by copying its value to a tag/mark/metadata or
> > > > > > > copy it into another header field directly. tag/mark/metadata can also
> > > > > > > be used as a value to be stored in an arbitrary packet header field.
> > > > > > >
> > > > > > > struct rte_flow_action_copy_data {
> > > > > > > 	enum rte_flow_field_id field;
> > > > > > > 	uint16_t index;
> > > > > > > 	uint16_t offset;
> > > > > > > };
> > > > > > >
> > > > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > > > The index gives access to inner packet headers or elements in the tags
> > > > > > > array. The offset allows to copy a packet field value into the payload.
> > > > > >
> > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > >
> > > > > It is a layer for packet fields, inner headers get higher number index.
> > > > > But is it also an index in the TAG array, so the name comes from it.
> > > >
> > > > Sorry it is not obvious.
> > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > >
> > > > > > What is the field id if an offset is given?
> > > > >
> > > > > Field ID stays the same, you can specify a small offset to copy just a few
> > bits
> > > > > from the entire packet field or a big offset to move to completely different
> > > > area.
> > > >
> > > > I don't understand what is an offset then.
> > > > Isn't it the byte or bit where the copy start?
> > > > Do you handle sizes smaller than a byte?
> > >
> > > It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address for
> > example.
> > 
> > Now I'm confused.
> > You mean rte_flow_action_copy_data.offset is a bit offset?
> 
> rte_flow_action_copy_data.offset and rte_flow_action_copy_field.width
> are measured in bits, right.

So the offset is limited to 16 bits?
How can it be useful? Is it an offset starting from the specified field?


> > > > > > Can we say that a field id can always be replaced by an offset?
> > > > >
> > > > > Not really. You can use offset to jump around packet fields for sure, but it
> > is
> > > > going to be
> > > > > hard and cumbersome to calculate all the offsets for that. Field ID is much
> > > > more convenient.
> > > >
> > > > I think it depends for who.
> > > > For some use cases, it may be easier to pass an offset.
> > > > For some drivers, it may be more efficient to directly manage offsets.
> > >
> > > It is possible with this RFC, driver can choose what to use: id and/or offset.
> > 
> 
> > We can set field and index to 0, and use only offset?
> Yes, I'm not inending to put any restrictions against that.
> > Then it is a byte offset from the beginning mbuf.data?
> Yes, but it is still bit offset, not byte offset.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 16:54             ` Thomas Monjalon
@ 2021-01-07 16:57               ` Alexander Kozyrev
  2021-01-07 17:05                 ` Thomas Monjalon
  0 siblings, 1 reply; 16+ messages in thread
From: Alexander Kozyrev @ 2021-01-07 16:57 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

> 07/01/2021 16:22, Alexander Kozyrev:
> > > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > > <thomas@monjalon.net>
> > > > > > > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > > > > > > order to use it later for advanced packet manipulations. Examples
> > > > > > > > include the usage of VxLAN ID after the packet is decapsulated or
> > > > > > > > storing this ID inside the packet payload itself or swapping an
> > > > > > > > arbitrary inner and outer packet fields.
> > > > > > > >
> > > > > > > > The idea is to allow a copy of a specified number of bits form any
> > > > > > > > packet header field into another header field:
> > > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined
> > > below.
> > > > > > > >
> > > > > > > > struct rte_flow_action_copy_field {
> > > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > > 	uint16_t width;
> > > > > > > > };
> > > > > > > >
> > > > > > > > Arbitrary header field (as well as mark, metadata or tag values) can
> be
> > > > > > > > used as both source and destination fields. This way we can save an
> > > > > > > > arbitrary header field by copying its value to a tag/mark/metadata
> or
> > > > > > > > copy it into another header field directly. tag/mark/metadata can
> also
> > > > > > > > be used as a value to be stored in an arbitrary packet header field.
> > > > > > > >
> > > > > > > > struct rte_flow_action_copy_data {
> > > > > > > > 	enum rte_flow_field_id field;
> > > > > > > > 	uint16_t index;
> > > > > > > > 	uint16_t offset;
> > > > > > > > };
> > > > > > > >
> > > > > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > > > > The index gives access to inner packet headers or elements in the
> tags
> > > > > > > > array. The offset allows to copy a packet field value into the
> payload.
> > > > > > >
> > > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > > >
> > > > > > It is a layer for packet fields, inner headers get higher number index.
> > > > > > But is it also an index in the TAG array, so the name comes from it.
> > > > >
> > > > > Sorry it is not obvious.
> > > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > > >
> > > > > > > What is the field id if an offset is given?
> > > > > >
> > > > > > Field ID stays the same, you can specify a small offset to copy just a
> few
> > > bits
> > > > > > from the entire packet field or a big offset to move to completely
> different
> > > > > area.
> > > > >
> > > > > I don't understand what is an offset then.
> > > > > Isn't it the byte or bit where the copy start?
> > > > > Do you handle sizes smaller than a byte?
> > > >
> > > > It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address for
> > > example.
> > >
> > > Now I'm confused.
> > > You mean rte_flow_action_copy_data.offset is a bit offset?
> >
> > rte_flow_action_copy_data.offset and rte_flow_action_copy_field.width
> > are measured in bits, right.
> 
> So the offset is limited to 16 bits?
> How can it be useful? Is it an offset starting from the specified field?
Why 16? It can be up to 2^16=65536 bits. Do you think that is not enough?
And it starts from the specific packet field pointed by the Field ID, correct.

> 
> > > > > > > Can we say that a field id can always be replaced by an offset?
> > > > > >
> > > > > > Not really. You can use offset to jump around packet fields for sure, but
> it
> > > is
> > > > > going to be
> > > > > > hard and cumbersome to calculate all the offsets for that. Field ID is
> much
> > > > > more convenient.
> > > > >
> > > > > I think it depends for who.
> > > > > For some use cases, it may be easier to pass an offset.
> > > > > For some drivers, it may be more efficient to directly manage offsets.
> > > >
> > > > It is possible with this RFC, driver can choose what to use: id and/or offset.
> > >
> >
> > > We can set field and index to 0, and use only offset?
> > Yes, I'm not inending to put any restrictions against that.
> > > Then it is a byte offset from the beginning mbuf.data?
> > Yes, but it is still bit offset, not byte offset.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 16:57               ` Alexander Kozyrev
@ 2021-01-07 17:05                 ` Thomas Monjalon
  2021-01-07 20:14                   ` Alexander Kozyrev
  2021-01-08 12:16                   ` Slava Ovsiienko
  0 siblings, 2 replies; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-07 17:05 UTC (permalink / raw)
  To: Alexander Kozyrev
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

07/01/2021 17:57, Alexander Kozyrev:
> > 07/01/2021 16:22, Alexander Kozyrev:
> > > > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > > > <thomas@monjalon.net>
> > > > > > > > > RTE Flows API lacks the ability to save an arbitrary header field in
> > > > > > > > > order to use it later for advanced packet manipulations. Examples
> > > > > > > > > include the usage of VxLAN ID after the packet is decapsulated or
> > > > > > > > > storing this ID inside the packet payload itself or swapping an
> > > > > > > > > arbitrary inner and outer packet fields.
> > > > > > > > >
> > > > > > > > > The idea is to allow a copy of a specified number of bits form any
> > > > > > > > > packet header field into another header field:
> > > > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure defined
> > > > below.
> > > > > > > > >
> > > > > > > > > struct rte_flow_action_copy_field {
> > > > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > > > 	uint16_t width;
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > Arbitrary header field (as well as mark, metadata or tag values) can
> > be
> > > > > > > > > used as both source and destination fields. This way we can save an
> > > > > > > > > arbitrary header field by copying its value to a tag/mark/metadata
> > or
> > > > > > > > > copy it into another header field directly. tag/mark/metadata can
> > also
> > > > > > > > > be used as a value to be stored in an arbitrary packet header field.
> > > > > > > > >
> > > > > > > > > struct rte_flow_action_copy_data {
> > > > > > > > > 	enum rte_flow_field_id field;
> > > > > > > > > 	uint16_t index;
> > > > > > > > > 	uint16_t offset;
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > > > > > The index gives access to inner packet headers or elements in the
> > tags
> > > > > > > > > array. The offset allows to copy a packet field value into the
> > payload.
> > > > > > > >
> > > > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > > > >
> > > > > > > It is a layer for packet fields, inner headers get higher number index.
> > > > > > > But is it also an index in the TAG array, so the name comes from it.
> > > > > >
> > > > > > Sorry it is not obvious.
> > > > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > > > >
> > > > > > > > What is the field id if an offset is given?
> > > > > > >
> > > > > > > Field ID stays the same, you can specify a small offset to copy just a
> > few
> > > > bits
> > > > > > > from the entire packet field or a big offset to move to completely
> > different
> > > > > > area.
> > > > > >
> > > > > > I don't understand what is an offset then.
> > > > > > Isn't it the byte or bit where the copy start?
> > > > > > Do you handle sizes smaller than a byte?
> > > > >
> > > > > It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address for
> > > > example.
> > > >
> > > > Now I'm confused.
> > > > You mean rte_flow_action_copy_data.offset is a bit offset?
> > >
> > > rte_flow_action_copy_data.offset and rte_flow_action_copy_field.width
> > > are measured in bits, right.
> > 
> > So the offset is limited to 16 bits?
> > How can it be useful? Is it an offset starting from the specified field?
> 
> Why 16? It can be up to 2^16=65536 bits. Do you think that is not enough?

Yes 8KB may be too small for huge packets.
I recommend 32 bits.

> And it starts from the specific packet field pointed by the Field ID, correct.

I think it would be more useful as a global offset
starting from the first bit of the packet.


> > > > > > > > Can we say that a field id can always be replaced by an offset?
> > > > > > >
> > > > > > > Not really. You can use offset to jump around packet fields for sure, but
> > it
> > > > is
> > > > > > going to be
> > > > > > > hard and cumbersome to calculate all the offsets for that. Field ID is
> > much
> > > > > > more convenient.
> > > > > >
> > > > > > I think it depends for who.
> > > > > > For some use cases, it may be easier to pass an offset.
> > > > > > For some drivers, it may be more efficient to directly manage offsets.
> > > > >
> > > > > It is possible with this RFC, driver can choose what to use: id and/or offset.
> > > >
> > >
> > > > We can set field and index to 0, and use only offset?
> > > Yes, I'm not inending to put any restrictions against that.
> > > > Then it is a byte offset from the beginning mbuf.data?
> > > Yes, but it is still bit offset, not byte offset.






^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 17:05                 ` Thomas Monjalon
@ 2021-01-07 20:14                   ` Alexander Kozyrev
  2021-01-07 20:21                     ` Thomas Monjalon
  2021-01-08 12:16                   ` Slava Ovsiienko
  1 sibling, 1 reply; 16+ messages in thread
From: Alexander Kozyrev @ 2021-01-07 20:14 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

> 07/01/2021 17:57, Alexander Kozyrev:
> > > 07/01/2021 16:22, Alexander Kozyrev:
> > > > > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > > > > <thomas@monjalon.net>
> > > > > > > > > > RTE Flows API lacks the ability to save an arbitrary header field
> in
> > > > > > > > > > order to use it later for advanced packet manipulations.
> Examples
> > > > > > > > > > include the usage of VxLAN ID after the packet is decapsulated
> or
> > > > > > > > > > storing this ID inside the packet payload itself or swapping an
> > > > > > > > > > arbitrary inner and outer packet fields.
> > > > > > > > > >
> > > > > > > > > > The idea is to allow a copy of a specified number of bits form
> any
> > > > > > > > > > packet header field into another header field:
> > > > > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure
> defined
> > > > > below.
> > > > > > > > > >
> > > > > > > > > > struct rte_flow_action_copy_field {
> > > > > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > > > > 	uint16_t width;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > Arbitrary header field (as well as mark, metadata or tag values)
> can
> > > be
> > > > > > > > > > used as both source and destination fields. This way we can save
> an
> > > > > > > > > > arbitrary header field by copying its value to a
> tag/mark/metadata
> > > or
> > > > > > > > > > copy it into another header field directly. tag/mark/metadata
> can
> > > also
> > > > > > > > > > be used as a value to be stored in an arbitrary packet header
> field.
> > > > > > > > > >
> > > > > > > > > > struct rte_flow_action_copy_data {
> > > > > > > > > > 	enum rte_flow_field_id field;
> > > > > > > > > > 	uint16_t index;
> > > > > > > > > > 	uint16_t offset;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > > > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > > > > > > The index gives access to inner packet headers or elements in
> the
> > > tags
> > > > > > > > > > array. The offset allows to copy a packet field value into the
> > > payload.
> > > > > > > > >
> > > > > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > > > > >
> > > > > > > > It is a layer for packet fields, inner headers get higher number index.
> > > > > > > > But is it also an index in the TAG array, so the name comes from it.
> > > > > > >
> > > > > > > Sorry it is not obvious.
> > > > > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > > > > >
> > > > > > > > > What is the field id if an offset is given?
> > > > > > > >
> > > > > > > > Field ID stays the same, you can specify a small offset to copy just a
> > > few
> > > > > bits
> > > > > > > > from the entire packet field or a big offset to move to completely
> > > different
> > > > > > > area.
> > > > > > >
> > > > > > > I don't understand what is an offset then.
> > > > > > > Isn't it the byte or bit where the copy start?
> > > > > > > Do you handle sizes smaller than a byte?
> > > > > >
> > > > > > It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address
> for
> > > > > example.
> > > > >
> > > > > Now I'm confused.
> > > > > You mean rte_flow_action_copy_data.offset is a bit offset?
> > > >
> > > > rte_flow_action_copy_data.offset and rte_flow_action_copy_field.width
> > > > are measured in bits, right.
> > >
> > > So the offset is limited to 16 bits?
> > > How can it be useful? Is it an offset starting from the specified field?
> >
> > Why 16? It can be up to 2^16=65536 bits. Do you think that is not enough?
> 
> Yes 8KB may be too small for huge packets.
> I recommend 32 bits.
Sounds good, will make it 32-bit in the implementation.

> > And it starts from the specific packet field pointed by the Field ID, correct.
> 
> I think it would be more useful as a global offset
> starting from the first bit of the packet.
The API gives you this flexibility when you specify None as the Field ID.
But Field ID is useful when you don't want to calculate the offset by yourself.
You can just say: I would like to copy IP address in the inner header (index 1),
for example, and leave offset as 0 instead of trying to figure out where it is:
set copy_field width 32 src field ipv4 index 1 offset 0 dst field tag index 0 offset 0

> > > > > > > > > Can we say that a field id can always be replaced by an offset?
> > > > > > > >
> > > > > > > > Not really. You can use offset to jump around packet fields for sure,
> but
> > > it
> > > > > is
> > > > > > > going to be
> > > > > > > > hard and cumbersome to calculate all the offsets for that. Field ID is
> > > much
> > > > > > > more convenient.
> > > > > > >
> > > > > > > I think it depends for who.
> > > > > > > For some use cases, it may be easier to pass an offset.
> > > > > > > For some drivers, it may be more efficient to directly manage offsets.
> > > > > >
> > > > > > It is possible with this RFC, driver can choose what to use: id and/or
> offset.
> > > > >
> > > >
> > > > > We can set field and index to 0, and use only offset?
> > > > Yes, I'm not inending to put any restrictions against that.
> > > > > Then it is a byte offset from the beginning mbuf.data?
> > > > Yes, but it is still bit offset, not byte offset.
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 20:14                   ` Alexander Kozyrev
@ 2021-01-07 20:21                     ` Thomas Monjalon
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2021-01-07 20:21 UTC (permalink / raw)
  To: Alexander Kozyrev
  Cc: dev, Slava Ovsiienko, Ori Kam, ferruh.yigit, andrew.rybchenko,
	ajit.khaparde, jerinj

07/01/2021 21:14, Alexander Kozyrev:
> > 07/01/2021 17:57, Alexander Kozyrev:
> > > > 07/01/2021 16:22, Alexander Kozyrev:
> > > > > > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > > > > > <thomas@monjalon.net>
> > > > > > > > > > > RTE Flows API lacks the ability to save an arbitrary header field
> > in
> > > > > > > > > > > order to use it later for advanced packet manipulations.
> > Examples
> > > > > > > > > > > include the usage of VxLAN ID after the packet is decapsulated
> > or
> > > > > > > > > > > storing this ID inside the packet payload itself or swapping an
> > > > > > > > > > > arbitrary inner and outer packet fields.
> > > > > > > > > > >
> > > > > > > > > > > The idea is to allow a copy of a specified number of bits form
> > any
> > > > > > > > > > > packet header field into another header field:
> > > > > > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure
> > defined
> > > > > > below.
> > > > > > > > > > >
> > > > > > > > > > > struct rte_flow_action_copy_field {
> > > > > > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > > > > > 	uint16_t width;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > Arbitrary header field (as well as mark, metadata or tag values)
> > can
> > > > be
> > > > > > > > > > > used as both source and destination fields. This way we can save
> > an
> > > > > > > > > > > arbitrary header field by copying its value to a
> > tag/mark/metadata
> > > > or
> > > > > > > > > > > copy it into another header field directly. tag/mark/metadata
> > can
> > > > also
> > > > > > > > > > > be used as a value to be stored in an arbitrary packet header
> > field.
> > > > > > > > > > >
> > > > > > > > > > > struct rte_flow_action_copy_data {
> > > > > > > > > > > 	enum rte_flow_field_id field;
> > > > > > > > > > > 	uint16_t index;
> > > > > > > > > > > 	uint16_t offset;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > The rte_flow_field_id specifies the particular packet field (or
> > > > > > > > > > > tag/mark/metadata) to be used as a copy source or destination.
> > > > > > > > > > > The index gives access to inner packet headers or elements in
> > the
> > > > tags
> > > > > > > > > > > array. The offset allows to copy a packet field value into the
> > > > payload.
> > > > > > > > > >
> > > > > > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > > > > > >
> > > > > > > > > It is a layer for packet fields, inner headers get higher number index.
> > > > > > > > > But is it also an index in the TAG array, so the name comes from it.
> > > > > > > >
> > > > > > > > Sorry it is not obvious.
> > > > > > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > > > > > >
> > > > > > > > > > What is the field id if an offset is given?
> > > > > > > > >
> > > > > > > > > Field ID stays the same, you can specify a small offset to copy just a
> > > > few
> > > > > > bits
> > > > > > > > > from the entire packet field or a big offset to move to completely
> > > > different
> > > > > > > > area.
> > > > > > > >
> > > > > > > > I don't understand what is an offset then.
> > > > > > > > Isn't it the byte or bit where the copy start?
> > > > > > > > Do you handle sizes smaller than a byte?
> > > > > > >
> > > > > > > It is the bit offset, you can copy 20 bits out of 32 bits of IPv4 address
> > for
> > > > > > example.
> > > > > >
> > > > > > Now I'm confused.
> > > > > > You mean rte_flow_action_copy_data.offset is a bit offset?
> > > > >
> > > > > rte_flow_action_copy_data.offset and rte_flow_action_copy_field.width
> > > > > are measured in bits, right.
> > > >
> > > > So the offset is limited to 16 bits?
> > > > How can it be useful? Is it an offset starting from the specified field?
> > >
> > > Why 16? It can be up to 2^16=65536 bits. Do you think that is not enough?
> > 
> > Yes 8KB may be too small for huge packets.
> > I recommend 32 bits.
> Sounds good, will make it 32-bit in the implementation.
> 
> > > And it starts from the specific packet field pointed by the Field ID, correct.
> > 
> > I think it would be more useful as a global offset
> > starting from the first bit of the packet.
> The API gives you this flexibility when you specify None as the Field ID.
> But Field ID is useful when you don't want to calculate the offset by yourself.
> You can just say: I would like to copy IP address in the inner header (index 1),
> for example, and leave offset as 0 instead of trying to figure out where it is:
> set copy_field width 32 src field ipv4 index 1 offset 0 dst field tag index 0 offset 0

OK it makes sense.

I think you should better explain the calculations in the doxygen comments.




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-07 17:05                 ` Thomas Monjalon
  2021-01-07 20:14                   ` Alexander Kozyrev
@ 2021-01-08 12:16                   ` Slava Ovsiienko
  2021-01-10  6:50                     ` Ori Kam
  1 sibling, 1 reply; 16+ messages in thread
From: Slava Ovsiienko @ 2021-01-08 12:16 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon, Alexander Kozyrev
  Cc: dev, Ori Kam, ferruh.yigit, andrew.rybchenko, ajit.khaparde, jerinj

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, January 7, 2021 19:06
> To: Alexander Kozyrev <akozyrev@nvidia.com>
> Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam
> <orika@nvidia.com>; ferruh.yigit@intel.com;
> andrew.rybchenko@oktetlabs.ru; ajit.khaparde@broadcom.com;
> jerinj@marvell.com
> Subject: Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
> 
> 07/01/2021 17:57, Alexander Kozyrev:
> > > 07/01/2021 16:22, Alexander Kozyrev:
> > > > > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > > > > <thomas@monjalon.net>
> > > > > > > > > > RTE Flows API lacks the ability to save an arbitrary
> > > > > > > > > > header field in order to use it later for advanced
> > > > > > > > > > packet manipulations. Examples include the usage of
> > > > > > > > > > VxLAN ID after the packet is decapsulated or storing
> > > > > > > > > > this ID inside the packet payload itself or swapping an arbitrary
> inner and outer packet fields.
> > > > > > > > > >
> > > > > > > > > > The idea is to allow a copy of a specified number of
> > > > > > > > > > bits form any packet header field into another header field:
> > > > > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure
> > > > > > > > > > defined
> > > > > below.
> > > > > > > > > >
> > > > > > > > > > struct rte_flow_action_copy_field {
> > > > > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > > > > 	uint16_t width;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > Arbitrary header field (as well as mark, metadata or
> > > > > > > > > > tag values) can
> > > be
> > > > > > > > > > used as both source and destination fields. This way
> > > > > > > > > > we can save an arbitrary header field by copying its
> > > > > > > > > > value to a tag/mark/metadata
> > > or
> > > > > > > > > > copy it into another header field directly.
> > > > > > > > > > tag/mark/metadata can
> > > also
> > > > > > > > > > be used as a value to be stored in an arbitrary packet header
> field.
> > > > > > > > > >
> > > > > > > > > > struct rte_flow_action_copy_data {
> > > > > > > > > > 	enum rte_flow_field_id field;
> > > > > > > > > > 	uint16_t index;
> > > > > > > > > > 	uint16_t offset;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > The rte_flow_field_id specifies the particular packet
> > > > > > > > > > field (or
> > > > > > > > > > tag/mark/metadata) to be used as a copy source or
> destination.
> > > > > > > > > > The index gives access to inner packet headers or
> > > > > > > > > > elements in the
> > > tags
> > > > > > > > > > array. The offset allows to copy a packet field value
> > > > > > > > > > into the
> > > payload.
> > > > > > > > >
> > > > > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > > > > >
> > > > > > > > It is a layer for packet fields, inner headers get higher number
> index.
> > > > > > > > But is it also an index in the TAG array, so the name comes from it.
> > > > > > >
> > > > > > > Sorry it is not obvious.
> > > > > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > > > > >
> > > > > > > > > What is the field id if an offset is given?
> > > > > > > >
> > > > > > > > Field ID stays the same, you can specify a small offset to
> > > > > > > > copy just a
> > > few
> > > > > bits
> > > > > > > > from the entire packet field or a big offset to move to
> > > > > > > > completely
> > > different
> > > > > > > area.
> > > > > > >
> > > > > > > I don't understand what is an offset then.
> > > > > > > Isn't it the byte or bit where the copy start?
> > > > > > > Do you handle sizes smaller than a byte?
> > > > > >
> > > > > > It is the bit offset, you can copy 20 bits out of 32 bits of
> > > > > > IPv4 address for
> > > > > example.
> > > > >
> > > > > Now I'm confused.
> > > > > You mean rte_flow_action_copy_data.offset is a bit offset?
> > > >
> > > > rte_flow_action_copy_data.offset and
> > > > rte_flow_action_copy_field.width are measured in bits, right.
> > >
> > > So the offset is limited to 16 bits?
> > > How can it be useful? Is it an offset starting from the specified field?
> >
> > Why 16? It can be up to 2^16=65536 bits. Do you think that is not enough?
> 
> Yes 8KB may be too small for huge packets.
> I recommend 32 bits.
> 
> > And it starts from the specific packet field pointed by the Field ID, correct.
> 
> I think it would be more useful as a global offset starting from the first bit of
> the packet.

M-m-m, sorry, I think this anchor (the first bit of the packet) is not applicable in general due to
the field position in the packet is flexible and varying. For example, packet may contain 
VLAN tag, IP header options, TCP options, or may not, and absolute offset 
(from the packet beginning) does not work well. 

Hence, to unambiguously specify the desired part (as bit field) of the packet we need the following indices:
- protocol field, we do it with field enum, and field index (if any - in case if we have multiple
similar items in the packet - say, inner or outer IP, or array of metadata tags)
- the bit offset in the field - to specify the beginning of our target bitfield
- the bitfield width in bits - to specify how many bits we would like to proceed

With this bitfield addressing approach we get the opportunity to manipulate with
bitfields in the almost any protocol fields. For example we could copy congestion
control bits from inner IP header to outer one. We can have fine handling the bits 
in metadata, and so on.

> > > > > > > > > Can we say that a field id can always be replaced by an offset?
Theoretically we could consider defining some fixed packet structure and use
offsets from there, say:
MAC_DST is 0
MAC_SRC is 6
MAC_TYPE is 12
VLAN_TAG is 14
VLAN_TYPE is 16
....

But, there are to many variations in the possible packet formats - tagged/untagged,
CVLANs, IP4/6, IP options/extensions, the huge zoopark of tunnel... It would introduce
and ambiguity and would require to predefine a lot of packet formats. 

With best regards,
Slava


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
  2021-01-08 12:16                   ` Slava Ovsiienko
@ 2021-01-10  6:50                     ` Ori Kam
  0 siblings, 0 replies; 16+ messages in thread
From: Ori Kam @ 2021-01-10  6:50 UTC (permalink / raw)
  To: Slava Ovsiienko, NBU-Contact-Thomas Monjalon, Alexander Kozyrev
  Cc: dev, ferruh.yigit, andrew.rybchenko, ajit.khaparde, jerinj

Hi

> -----Original Message-----
> From: Slava Ovsiienko <viacheslavo@nvidia.com>
> Sent: Friday, January 8, 2021 2:16 PM
> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Alexander
> Kozyrev <akozyrev@nvidia.com>
> Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>; ferruh.yigit@intel.com;
> andrew.rybchenko@oktetlabs.ru; ajit.khaparde@broadcom.com;
> jerinj@marvell.com
> Subject: RE: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, January 7, 2021 19:06
> > To: Alexander Kozyrev <akozyrev@nvidia.com>
> > Cc: dev@dpdk.org; Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam
> > <orika@nvidia.com>; ferruh.yigit@intel.com;
> > andrew.rybchenko@oktetlabs.ru; ajit.khaparde@broadcom.com;
> > jerinj@marvell.com
> > Subject: Re: [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action
> >
> > 07/01/2021 17:57, Alexander Kozyrev:
> > > > 07/01/2021 16:22, Alexander Kozyrev:
> > > > > > 07/01/2021 16:10, Alexander Kozyrev:
> > > > > > > > > > Thursday, January 7, 2021 10:18, Thomas Monjalon
> > > > > > <thomas@monjalon.net>
> > > > > > > > > > > RTE Flows API lacks the ability to save an arbitrary
> > > > > > > > > > > header field in order to use it later for advanced
> > > > > > > > > > > packet manipulations. Examples include the usage of
> > > > > > > > > > > VxLAN ID after the packet is decapsulated or storing
> > > > > > > > > > > this ID inside the packet payload itself or swapping an
> arbitrary
> > inner and outer packet fields.
> > > > > > > > > > >
> > > > > > > > > > > The idea is to allow a copy of a specified number of
> > > > > > > > > > > bits form any packet header field into another header field:
> > > > > > > > > > > RTE_FLOW_ACTION_TYPE_COPY_FIELD with the structure
> > > > > > > > > > > defined
> > > > > > below.
> > > > > > > > > > >
> > > > > > > > > > > struct rte_flow_action_copy_field {
> > > > > > > > > > > 	struct rte_flow_action_copy_data dest;
> > > > > > > > > > > 	struct rte_flow_action_copy_data src;
> > > > > > > > > > > 	uint16_t width;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > Arbitrary header field (as well as mark, metadata or
> > > > > > > > > > > tag values) can
> > > > be
> > > > > > > > > > > used as both source and destination fields. This way
> > > > > > > > > > > we can save an arbitrary header field by copying its
> > > > > > > > > > > value to a tag/mark/metadata
> > > > or
> > > > > > > > > > > copy it into another header field directly.
> > > > > > > > > > > tag/mark/metadata can
> > > > also
> > > > > > > > > > > be used as a value to be stored in an arbitrary packet header
> > field.
> > > > > > > > > > >
> > > > > > > > > > > struct rte_flow_action_copy_data {
> > > > > > > > > > > 	enum rte_flow_field_id field;
> > > > > > > > > > > 	uint16_t index;
> > > > > > > > > > > 	uint16_t offset;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > The rte_flow_field_id specifies the particular packet
> > > > > > > > > > > field (or
> > > > > > > > > > > tag/mark/metadata) to be used as a copy source or
> > destination.
> > > > > > > > > > > The index gives access to inner packet headers or
> > > > > > > > > > > elements in the
> > > > tags
> > > > > > > > > > > array. The offset allows to copy a packet field value
> > > > > > > > > > > into the
> > > > payload.
> > > > > > > > > >
> > > > > > > > > > So index is in reality the layer? How is it numbered exactly?
> > > > > > > > >
> > > > > > > > > It is a layer for packet fields, inner headers get higher number
> > index.
> > > > > > > > > But is it also an index in the TAG array, so the name comes from
> it.
> > > > > > > >
> > > > > > > > Sorry it is not obvious.
> > > > > > > > Please describe the exact numbering in tunnel and VLAN cases.
> > > > > > > >
> > > > > > > > > > What is the field id if an offset is given?
> > > > > > > > >
> > > > > > > > > Field ID stays the same, you can specify a small offset to
> > > > > > > > > copy just a
> > > > few
> > > > > > bits
> > > > > > > > > from the entire packet field or a big offset to move to
> > > > > > > > > completely
> > > > different
> > > > > > > > area.
> > > > > > > >
> > > > > > > > I don't understand what is an offset then.
> > > > > > > > Isn't it the byte or bit where the copy start?
> > > > > > > > Do you handle sizes smaller than a byte?
> > > > > > >
> > > > > > > It is the bit offset, you can copy 20 bits out of 32 bits of
> > > > > > > IPv4 address for
> > > > > > example.
> > > > > >
> > > > > > Now I'm confused.
> > > > > > You mean rte_flow_action_copy_data.offset is a bit offset?
> > > > >
> > > > > rte_flow_action_copy_data.offset and
> > > > > rte_flow_action_copy_field.width are measured in bits, right.
> > > >
> > > > So the offset is limited to 16 bits?
> > > > How can it be useful? Is it an offset starting from the specified field?
> > >
> > > Why 16? It can be up to 2^16=65536 bits. Do you think that is not enough?
> >
> > Yes 8KB may be too small for huge packets.
> > I recommend 32 bits.
> >
> > > And it starts from the specific packet field pointed by the Field ID, correct.
> >
> > I think it would be more useful as a global offset starting from the first bit of
> > the packet.
> 
> M-m-m, sorry, I think this anchor (the first bit of the packet) is not applicable in
> general due to
> the field position in the packet is flexible and varying. For example, packet may
> contain
> VLAN tag, IP header options, TCP options, or may not, and absolute offset
> (from the packet beginning) does not work well.
> 
> Hence, to unambiguously specify the desired part (as bit field) of the packet we
> need the following indices:
> - protocol field, we do it with field enum, and field index (if any - in case if we
> have multiple
> similar items in the packet - say, inner or outer IP, or array of metadata tags)
> - the bit offset in the field - to specify the beginning of our target bitfield
> - the bitfield width in bits - to specify how many bits we would like to proceed
> 
> With this bitfield addressing approach we get the opportunity to manipulate
> with
> bitfields in the almost any protocol fields. For example we could copy
> congestion
> control bits from inner IP header to outer one. We can have fine handling the
> bits
> in metadata, and so on.
> 
+1 

> > > > > > > > > > Can we say that a field id can always be replaced by an offset?
> Theoretically we could consider defining some fixed packet structure and use
> offsets from there, say:
> MAC_DST is 0
> MAC_SRC is 6
> MAC_TYPE is 12
> VLAN_TAG is 14
> VLAN_TYPE is 16
> ....
> 
> But, there are to many variations in the possible packet formats -
> tagged/untagged,
> CVLANs, IP4/6, IP options/extensions, the huge zoopark of tunnel... It would
> introduce
> and ambiguity and would require to predefine a lot of packet formats.
> 
> With best regards,
> Slava

Regards,
Ori

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-01-10  6:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18  1:31 [dpdk-dev] [RFC] ethdev: introduce copy_field rte flow action Alexander Kozyrev
2021-01-05 22:12 ` Alexander Kozyrev
2021-01-05 22:18   ` Thomas Monjalon
2021-01-05 22:16 ` Thomas Monjalon
2021-01-07 14:17   ` Alexander Kozyrev
2021-01-07 15:06     ` Thomas Monjalon
2021-01-07 15:10       ` Alexander Kozyrev
2021-01-07 15:17         ` Thomas Monjalon
2021-01-07 15:22           ` Alexander Kozyrev
2021-01-07 16:54             ` Thomas Monjalon
2021-01-07 16:57               ` Alexander Kozyrev
2021-01-07 17:05                 ` Thomas Monjalon
2021-01-07 20:14                   ` Alexander Kozyrev
2021-01-07 20:21                     ` Thomas Monjalon
2021-01-08 12:16                   ` Slava Ovsiienko
2021-01-10  6:50                     ` Ori Kam

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).