From: Alexander Kozyrev <akozyrev@nvidia.com> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net> Cc: "dev@dpdk.org" <dev@dpdk.org>, Slava Ovsiienko <viacheslavo@nvidia.com>, Ori Kam <orika@nvidia.com>, "ferruh.yigit@intel.com" <ferruh.yigit@intel.com>, "andrew.rybchenko@oktetlabs.ru" <andrew.rybchenko@oktetlabs.ru>, "jerinjacobk@gmail.com" <jerinjacobk@gmail.com> Subject: Re: [dpdk-dev] [PATCH v7 1/2] ethdev: introduce generic modify rte flow action Date: Mon, 18 Jan 2021 16:03:28 +0000 Message-ID: <BN7PR12MB2707A9FB1336382BEB32E828AFA40@BN7PR12MB2707.namprd12.prod.outlook.com> (raw) In-Reply-To: <4720582.E7ApZDAgU3@thomas> > From: Thomas Monjalon <thomas@monjalon.net> on Sunday, January 17, 2021 18:16 > 16/01/2021 05:50, Alexander Kozyrev: > > Implement the generic modify flow API to allow manipulations on > > an arbitrary header field (as well as mark, metadata or tag) using > > data from another field or a user-specified value. > > This generic modify mechanism removes the necessity to implement > > a separate RTE Flow action every time we need to modify a new packet > > field in the future. > > > > Supported operation are: > > - set: copy data from source to destination. > > - add: integer addition, stores the result in destination. > > - sub: integer subtraction, stores the result in destination. > > > > The field ID is used to specify the desired source/destination packet > > field in order to simplify the API for various encapsulation models. > > Specifying the packet field ID with the needed encapsulation level > > is able to quickly get a packet field for any inner packet header. > > > > Alternatively, the special ID (ITEM_START) can be used to point to > > the very beginning of a packet. This ID in conjunction with the > > offset parameter provides great flexibility to copy/modify any part of > > a packet as needed. > > > > The number of bits to use from a source as well as the offset can be > > be specified to allow a partial copy or dividing a big packet field > > into multiple small fields (e.g. copying 128 bits of IPv6 to 4 tags). > > > > An immediate value (or pointer to it) can be specified instead of the > > level and the offset for the special FIELD_VALUE ID (or FIELD_POINTER). > > Can be used as a source only. > > > > RFC: > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch > es.dpdk.org%2Fpatch%2F85384%2F&data=04%7C01%7Cakozyrev%40nv > idia.com%7Ce20d2c38372b4062420d08d8bb3dd549%7C43083d15727340c1b7d > b39efd9ccc17a%7C0%7C0%7C637465221537183554%7CUnknown%7CTWFpb > GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI > 6Mn0%3D%7C1000&sdata=gWEdmyAabnCCmeFyi%2FvHs0aheMmWNx > IyR%2BTC96O3Yck%3D&reserved=0 > > You don't need to refer to the RFC in the commit message. Removing link to it. > > > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com> > > Acked-by: Ori Kam <orika@nvidia.com> > > > > --- > > +Action: ``MODIFY_FIELD`` > > +^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +Modify ``dst`` field according to ``op`` selected (move, addition, > > +subtraction) with ``width`` bits of data from ``src`` field. > > "move" is changed to "set", right? Right, thanks for catching this. > > > +Any arbitrary header field (as well as mark, metadata or tag values) > > +can be used as both source and destination fields as set by ``field``. > > +The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it > > +``RTE_FLOW_FIELD_POINTER``) is allowed as a source only. > > +``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet. > > + > > +``op`` selects the operation to perform on a destination field. > > +- ``set`` copies the data from ``src`` field to ``dst`` field. > > +- ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``. > > +- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst`` > > + > > +``width`` defines a number of bits to use from ``src`` field. > > + > > +``level`` is used to access any packet field on any encapsulation level > > +as well as any tag element in the tag array. > > +- ``0`` means the default behaviour. Depending on the packet type, it can > > +mean outermost, innermost or anything in between. > > I feel the interpretation of the level 0 is driver-dependent? It is driver-dependent, the behavior is the same as in case of RSS. > > +- ``1`` requests access to the outermost packet encapsulation level. > > +- ``2`` and subsequent values requests access to the specified packet > > +encapsulation level, from outermost to innermost (lower to higher > values). > > +For the tag array ``level`` translates directly into the array index. > > You should define what is a tag array. Ok, but it is defined already in RTE_FLOW_ACTION_TYPE_SET_TAG action. > > + > > +``offset`` specifies the number of bits to skip from a field's start. > > +That allows performing a partial copy of the needed part or to divide a big > > +packet field into multiple smaller fields. Alternatively, ``offset`` allows > > +going past the specified packet field boundary to copy a field to an > > +arbitrary place in a packet, essentially providing a way to copy any part of > > +a packet to any other part of it if supported by an underlying PMD driver. > > All features of this specification require PMD support, > so I think it is useless to mention here. Removing. > > + > > +``value`` sets an immediate value to be used as a source or points to a > > +location of the value in memory. It is used instead of ``level`` and ``offset`` > > +for ``RTE_FLOW_FIELD_VALUE`` and ``RTE_FLOW_FIELD_POINTER`` > respectively. > > + > > +.. _table_rte_flow_action_modify_field: > > + > > +.. table:: MODIFY_FIELD > > + > > + +-----------------------------------------+ > > + | Field | Value | > > + +===============+=========================+ > > + | ``op`` | operation to perform | > > + | ``dst`` | destination field | > > + | ``src`` | source field | > > + | ``width`` | number of bits to use | > > + +---------------+-------------------------+ > > + > > +.. _table_rte_flow_action_modify_data: > > + > > +.. table:: destination/source field definition > > + > > + +--------------------------------------------------------------------------+ > > + | Field | Value | > > + > +===============+========================================= > =================+ > > + | ``field`` | ID: packet field, mark, meta, tag, immediate, pointer | > > + | ``level`` | encapsulation level of a packet field or tag array index | > > + | ``offset`` | number of bits to skip at the beginning | > > + | ``value`` | immediate value or a pointer to this value | > > + +---------------+----------------------------------------------------------+ > > I know the whole rte_flow guide has this kind of table, > but really it looks like doxygen and can be skipped here. > > If really this redundant info is required, it should be RST definition lists, > not tables. I would keep these tables for the sake of consistency of the rte_flow guide. > [...] > > In my opinion, the most important info is in the doxygen comments. Agree, adding them for all the structures/fields mentioned below. > Please add a doxygen comment for this enum. > > > +enum rte_flow_field_id { > > + RTE_FLOW_FIELD_START = 0, > > Please add a doxygen comment for the value RTE_FLOW_FIELD_START. > > > + 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, > > + RTE_FLOW_FIELD_POINTER, > > Please add a doxygen comment for the value RTE_FLOW_FIELD_POINTER. > > > + RTE_FLOW_FIELD_VALUE, > > Please add a doxygen comment for the value RTE_FLOW_FIELD_VALUE. > > > +struct rte_flow_action_modify_data { > > + enum rte_flow_field_id field; > > + RTE_STD_C11 > > + union { > > + struct { > > + uint32_t level; > > + uint32_t offset; > > + }; > > + uint64_t value; > > + }; > > +}; > > Please add doxygen for this struct and fields. > > > + > > +enum rte_flow_modify_op { > > + RTE_FLOW_MODIFY_SET = 0, > > + RTE_FLOW_MODIFY_ADD, > > + RTE_FLOW_MODIFY_SUB, > > +}; > > Please add doxygen for this enum. > > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this structure may change without prior notice > > + * > > + * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD > > + * > > + * Modifies a destination header field according to the specified > > + * operation. Another packet field can be used as a source as well > > + * as tag, mark, metadata or an immediate value or a pointer to it. > > + * Width is the number of bits used from the source item. > > width should be documented below as other fields. > > > + */ > > +struct rte_flow_action_modify_field { > > + enum rte_flow_modify_op operation; > > + struct rte_flow_action_modify_data dst; > > + struct rte_flow_action_modify_data src; > > + uint32_t width; > > +}; > >
next prev parent reply other threads:[~2021-01-18 16:03 UTC|newest] Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-01-08 6:32 [dpdk-dev] [PATCH] ethdev: introduce generic copy " Alexander Kozyrev 2021-01-10 8:00 ` Ori Kam 2021-01-10 9:36 ` Asaf Penso 2021-01-12 14:15 ` Alexander Kozyrev 2021-01-12 14:52 ` Ori Kam 2021-01-12 15:13 ` Ori Kam 2021-01-12 17:19 ` Alexander Kozyrev 2021-01-12 5:01 ` [dpdk-dev] [PATCH v2 0/2] generic copy rte flow action support Alexander Kozyrev 2021-01-12 5:01 ` [dpdk-dev] [PATCH v2 1/2] ethdev: introduce generic copy rte flow action Alexander Kozyrev 2021-01-12 5:01 ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: add support for " Alexander Kozyrev 2021-01-12 14:58 ` Ori Kam 2021-01-13 3:38 ` [dpdk-dev] [PATCH v3 0/2] generic copy rte flow action support Alexander Kozyrev 2021-01-13 3:38 ` [dpdk-dev] [PATCH v3 1/2] ethdev: introduce generic copy rte flow action Alexander Kozyrev 2021-01-13 11:12 ` Ori Kam 2021-01-13 3:38 ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: add support for " Alexander Kozyrev 2021-01-13 17:07 ` [dpdk-dev] [PATCH v4 0/2] generic copy rte flow action support Alexander Kozyrev 2021-01-13 17:07 ` [dpdk-dev] [PATCH v4 1/2] ethdev: introduce generic copy rte flow action Alexander Kozyrev 2021-01-14 12:22 ` Ori Kam 2021-01-13 17:07 ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: add support for " Alexander Kozyrev 2021-01-14 15:18 ` Ori Kam 2021-01-15 15:37 ` Alexander Kozyrev 2021-01-15 15:42 ` [dpdk-dev] [PATCH v5 0/2] generic modify rte flow action support Alexander Kozyrev 2021-01-15 15:42 ` [dpdk-dev] [PATCH v5 1/2] ethdev: introduce generic modify rte flow action Alexander Kozyrev 2021-01-15 18:03 ` Ori Kam 2021-01-17 9:23 ` Slava Ovsiienko 2021-01-15 15:42 ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: add support for modify field " Alexander Kozyrev 2021-01-15 18:04 ` Ori Kam 2021-01-16 4:44 ` [dpdk-dev] [PATCH v6 0/2] generic modify rte flow action support Alexander Kozyrev 2021-01-16 4:44 ` [dpdk-dev] [PATCH v6 1/2] ethdev: introduce generic modify rte flow action Alexander Kozyrev 2021-01-16 4:44 ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: add support for modify field " Alexander Kozyrev 2021-01-16 4:50 ` [dpdk-dev] [PATCH v7 0/2] generic modify rte flow action support Alexander Kozyrev 2021-01-16 4:50 ` [dpdk-dev] [PATCH v7 1/2] ethdev: introduce generic modify rte flow action Alexander Kozyrev 2021-01-17 23:15 ` Thomas Monjalon 2021-01-18 16:03 ` Alexander Kozyrev [this message] 2021-01-16 4:50 ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: add support for modify field " Alexander Kozyrev 2021-01-18 16:18 ` [dpdk-dev] [PATCH v8 0/2] generic modify rte flow action support Alexander Kozyrev 2021-01-18 16:18 ` [dpdk-dev] [PATCH v8 1/2] ethdev: introduce generic modify rte flow action Alexander Kozyrev 2021-01-18 17:51 ` Thomas Monjalon 2021-01-18 16:18 ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: add support for modify field " Alexander Kozyrev 2021-01-18 20:05 ` [dpdk-dev] [PATCH v8 0/2] generic modify rte flow action support Ajit Khaparde 2021-01-18 21:40 ` [dpdk-dev] [PATCH v9 " Alexander Kozyrev 2021-01-18 21:40 ` [dpdk-dev] [PATCH v9 1/2] ethdev: introduce generic modify rte flow action Alexander Kozyrev 2021-01-18 21:40 ` [dpdk-dev] [PATCH v9 2/2] app/testpmd: add support for modify field " Alexander Kozyrev 2021-01-19 1:21 ` [dpdk-dev] [PATCH v9 0/2] generic modify rte flow action support Ferruh Yigit 2021-01-14 13:59 ` [dpdk-dev] [PATCH] ethdev: introduce generic copy rte flow action Jerin Jacob 2021-01-14 15:02 ` Ori Kam 2021-01-15 14:00 ` Jerin Jacob 2021-01-15 15:33 ` Alexander Kozyrev
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=BN7PR12MB2707A9FB1336382BEB32E828AFA40@BN7PR12MB2707.namprd12.prod.outlook.com \ --to=akozyrev@nvidia.com \ --cc=andrew.rybchenko@oktetlabs.ru \ --cc=dev@dpdk.org \ --cc=ferruh.yigit@intel.com \ --cc=jerinjacobk@gmail.com \ --cc=orika@nvidia.com \ --cc=thomas@monjalon.net \ --cc=viacheslavo@nvidia.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \ dev@dpdk.org public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git