Correct, Ori. We'll soon send the testpmd part and pmd draft. Regards, Asaf Penso
>-----Original Message----- >From: dev <dev-boun...@dpdk.org> On Behalf Of Ori Kam >Sent: Sunday, January 10, 2021 10:01 AM >To: Alexander Kozyrev <akozy...@nvidia.com>; dev@dpdk.org >Cc: Slava Ovsiienko <viachesl...@nvidia.com>; NBU-Contact-Thomas >Monjalon <tho...@monjalon.net>; ferruh.yi...@intel.com; >andrew.rybche...@oktetlabs.ru >Subject: Re: [dpdk-dev] [PATCH] ethdev: introduce generic copy rte flow >action > >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%2Fpatch >> es.d >pdk.org%2Fpatch%2F85384%2F&data=04%7C01%7Corika%40nvidia.com >% >> 7Cd04c2e49c3a840994da408d8b39f3304%7C43083d15727340c1b7db39efd9cc >> >c17a%7C0%7C0%7C637456843629116269%7CUnknown%7CTWFpbGZsb3d8eyJ >> >WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7 >C >> >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
