From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 74CA442F93; Wed, 2 Aug 2023 12:25:11 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 02CDE40DDB; Wed, 2 Aug 2023 12:25:11 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id A52E94021D; Wed, 2 Aug 2023 12:25:09 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 651B720634; Wed, 2 Aug 2023 12:25:09 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH] ethdev: introduce generic flow item and action Date: Wed, 2 Aug 2023 12:25:07 +0200 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87AB8@smartserver.smartshare.dk> In-Reply-To: <20230802173451.3151646-1-qi.z.zhang@intel.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] ethdev: introduce generic flow item and action Thread-Index: AdnFIdkpM2GpYjexRNKzjoB4hBOiOAABxMkg References: <20230802173451.3151646-1-qi.z.zhang@intel.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Qi Zhang" , , , , , , Cc: , , , , , "Cristian Dumitrescu" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Qi Zhang [mailto:qi.z.zhang@intel.com] > Sent: Wednesday, 2 August 2023 19.35 >=20 > From: Cristian Dumitrescu >=20 > For network devices that are programmable through languages such as > the P4 language, there are no pre-defined flow items and actions. >=20 > The format of the protocol header and metadata fields that are used to > specify the flow items that make up the flow pattern, as well as the > flow actions, are all defined by the program, with an infinity of > possible combinations, as opposed to being selected from a finite > pre-defined list. >=20 > It is virtually impossible to pre-define all the flow items and the > flow actions that programs might ever use, as these are only limited > by the set of HW resources and the program developer's imagination. >=20 > To support the programmable network devices, we are introducing: >=20 > * A generic flow item: The flow item is expressed as an array of bytes > of a given length, whose meaning is defined by the program loaded by > the network device. The flow item is not "generic", it is "opaque": Only the application = knows what this flow item does. I hate the concept for two reasons: 1. The inability for applications to detect which flow items the = underlying hardware supports. 2. The risk that vendors will use this instead of introducing new flow = item types, available for anyone to implement. If you proceed anyway, there's a few comments inline below. > The device is still expected to accept the > existing pre-defined flow items such as Ethernet, IPv4/IPv6 headers, > etc, as long as the program currently loaded on the device is defining > and using flow items with identical format, but the device is not > limited to the current set of pre-defined RTE_FLOW flow items. >=20 > * A generic flow action: The flow action exact processing is defined > by the program loaded by the network device, with the user expected to > know the set of program actions for the purpose of assigning actions > to flows. The device is still expected to accept the existing > pre-defined flow actions such as packet drop, packet redirection to > output queues, packet modifications, etc, as long as the program > currently loaded on the device is defining and using flow actions that > perform identical processing, but the device is not limited to the > current set of pre-defined RTE_FLOW flow actions. >=20 > Signed-off-by: Cristian Dumitrescu > Signed-off-by: Qi Zhang > --- > lib/ethdev/rte_flow.h | 82 = +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 82 insertions(+) >=20 > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h > index 3fe57140f9..f7889d7dd0 100644 > --- a/lib/ethdev/rte_flow.h > +++ b/lib/ethdev/rte_flow.h > @@ -688,6 +688,15 @@ enum rte_flow_item_type { > * @see struct rte_flow_item_ib_bth. > */ > RTE_FLOW_ITEM_TYPE_IB_BTH, > + > + /** > + * Matches a custom protocol header or metadata field represented > + * as a byte string of a given length, whose meaning is typically > + * defined by the data plane program. > + * > + * @see struct rte_flow_item_generic. > + */ > + RTE_FLOW_ITEM_TYPE_GENERIC, Rename: _GENERIC -> _OPAQUE, everywhere. > }; >=20 > /** > @@ -2311,6 +2320,32 @@ static const struct rte_flow_item_tx_queue > rte_flow_item_tx_queue_mask =3D { > }; > #endif >=20 > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice > + * > + * RTE_FLOW_ITEM_TYPE_GENERIC > + * > + * Matches a custom protocol header or metadata field represented as = a byte > + * array of a given length, whose meaning is typically defined by the = data > + * plane program. "byte array" -> "opaque data type" > + * > + * The field length must be non-zero. The field value must be = non-NULL, with > the > + * value bytes specified in network byte order. > + */ > +struct rte_flow_item_generic { > + uint32_t length; /**< Field length. */ > + const uint8_t *value; /**< Field value. */ Suggestion: Change the value type from "const uint8_t *" to "const void = *". It makes it easier for the application to use a hierarchy of = structures internally for this. > +}; > + > +/** Default mask for RTE_FLOW_ITEM_TYPE_GENERIC. */ > +#ifndef __cplusplus > +static const struct rte_flow_item_generic rte_flow_item_generic_mask = =3D { > + .length =3D 0xffffffff, > + .value =3D NULL, > +}; > +#endif > + > /** > * Action types. > * > @@ -2989,6 +3024,14 @@ enum rte_flow_action_type { > * @see struct rte_flow_action_indirect_list > */ > RTE_FLOW_ACTION_TYPE_INDIRECT_LIST, > + > + /** > + * Custom action whose processing is typically defined by the data = plane > + * program. > + * > + * @see struct rte_flow_action_generic. > + */ > + RTE_FLOW_ACTION_TYPE_GENERIC, > }; >=20 > /** > @@ -4064,6 +4107,45 @@ struct rte_flow_indirect_update_flow_meter_mark = { > enum rte_color init_color; > }; >=20 > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice. > + * > + * Generic action argument configuration parameters. > + * > + * The action argument field length must be non-zero. The action = argument > field > + * value must be non-NULL, with the value bytes specified in network = byte > order. > + * > + * @see struct rte_flow_action_generic > + */ > +struct rte_flow_action_generic_argument { > + /** Argument field length. */ > + uint32_t length; > + /** Argument field value. */ > + const uint8_t *value; > +}; > + > +/** > + * @warning > + * @b EXPERIMENTAL: this structure may change without prior notice. > + * > + * RTE_FLOW_ACTION_TYPE_GENERIC > + * > + * Generic action configuration parameters. > + * > + * Each action can have zero or more arguments. > + * > + * @see RTE_FLOW_ACTION_TYPE_GENERIC > + */ > +struct rte_flow_action_generic { > + /** Action ID. */ > + uint32_t action_id; > + /** Number of action arguments. */ > + uint32_t action_args_num; > + /** Action arguments array. */ > + const struct rte_flow_action_generic_argument *action_args; > +}; This is a list of arguments, not one argument. You should append = "_array" somewhere in the name, and add a normal (non-list) action, = e.g.: struct rte_flow_action_opaque { //Or: "struct rte_flow_action_generic", if you want to keep that name. /** Action ID. */ uint32_t action_id; /** Argument length. */ uint32_t length; /** Argument value. */ const uint8_t *value; // Or: "const void *value;" for the same reason as for the flow item. }; > + > /* Mbuf dynamic field offset for metadata. */ > extern int32_t rte_flow_dynf_metadata_offs; >=20 > -- > 2.31.1