Hi Alexander, I guess that the test-pmd part will be available later right?
> -----Original Message----- > From: Alexander Kozyrev <akozy...@nvidia.com> > Sent: Friday, January 8, 2021 8:33 AM > Subject: [PATCH] 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 or directly copied from other packet field. > > The number of bits to copy as well as the offset to start from can > be specified to allow a partial copy or copy into an arbitrary > place in a packet for greater flexibility. > Since the question why you are using enum and not just offset from the start of the packet, was discussed and raised by number of people it will be best if it will appear in the commit log, at least the advantages to this implementation. > RFC: > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.d > pdk.org%2Fpatch%2F85384%2F&data=04%7C01%7Corika%40nvidia.com% > 7Cd04c2e49c3a840994da408d8b39f3304%7C43083d15727340c1b7db39efd9cc > c17a%7C0%7C0%7C637456843629116269%7CUnknown%7CTWFpbGZsb3d8eyJ > WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C > 1000&sdata=85096rASBtzbjU42pV6sPkl3nVt5HlR6%2BL9nxI3qgFA%3D&a > mp;reserved=0 > > Signed-off-by: Alexander Kozyrev <akozy...@nvidia.com> > --- > doc/guides/prog_guide/rte_flow.rst | 35 ++++++++++++++++++ > lib/librte_ethdev/rte_flow.c | 1 + > lib/librte_ethdev/rte_flow.h | 59 ++++++++++++++++++++++++++++++ > 3 files changed, 95 insertions(+) > > diff --git a/doc/guides/prog_guide/rte_flow.rst > b/doc/guides/prog_guide/rte_flow.rst > index 86b3444803..b737ff9dad 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -2766,6 +2766,41 @@ 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. > + > +An arbitrary header field (as well as mark, metadata or tag values) > +can be used as both source and destination items as set by ``item``. > + For tag I think you should also use the index right? > +Inner packet header fields can be accessed using the ``index`` and > +it is possible to start the copy from the ``offset`` bits in an item. Please specify what means index 0 /1 ... 0 is outer most? Inner most? You can look at the RSS level for reference. I think it will be best to use the same values. What happens if we want to copy between different sizes? for example copy IPV6 src to number of tags? I assume we will be using offset and split the copy command to number of actions right? > + > +.. _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`` | ID of a packet field/mark/metadata/tag | > + | ``index`` | index of outer/inner header or tag array | > + | ``offset`` | number of bits to skip during the copy | > + +---------------+------------------------------------------+ > + > Negative types > ~~~~~~~~~~~~~~ > > 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..0540c861fb 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_NONE = 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, > +}; I don't think this name is good since it not rte_flow_item this is just internal enumeration for this action. > + > +struct rte_flow_action_copy_data { > + enum rte_flow_item_id item; > + uint32_t index; > + uint32_t offset; Why use 32 bits? Since this copy only one register with max len of 32 bit. The max offset can 31? Same for the index. > +}; > + > +/** > + * 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 arbituary 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; Why use 32 bit register? > +}; > + > /* Mbuf dynamic field offset for metadata. */ > extern int32_t rte_flow_dynf_metadata_offs; > > -- > 2.24.1 Best, Ori
