Hello Ori,
> First I'm assuming this is just RFC, since there is no code implementation.
>
I'll add code implementation in v4.
> > -----Original Message-----
> > From: Gregory Etelson <getel...@nvidia.com>
> > Sent: Sunday, May 7, 2023 12:51 PM
> >
> > Indirect API creates a shared flow action with unique action handle.
> > Flow rules can access the shared flow action and resources related to
> > that action through the indirect action handle.
> > In addition, the API allows to update existing shared flow action
> > configuration. After the update completes, new action configuration
> > is available to all flows that reference that shared action.
> >
> > Indirect actions list expands the indirect action API:
> > • Indirect action list creates a handle for one or several
> > flow actions, while legacy indirect action handle references
> > single action only.
> > Input flow actions arranged in END terminated list.
> > • Flow rule can provide rule specific configuration parameters to
> > existing shared handle.
> > Updates of flow rule specific configuration will not change the base
> > action configuration.
> > Base action configuration was set during the action creation.
> >
> > Indirect action list handle defines 2 types of resources:
> > • Mutable handle resource can be changed during handle lifespan.
> > • Immutable handle resource value is set during handle creation
> > and cannot be changed.
> >
> > There are 2 types of mutable indirect handle contexts:
> > • Action mutable context is always shared between all flows
> > that referenced indirect actions list handle.
> > Action mutable context can be changed by explicit invocation
> > of indirect handle update function.
> > • Flow mutable context is private to a flow.
> > Flow mutable context can be updated by indirect list handle
> > flow rule configuration.
> >
> > flow 1:
> > / indirect handle H conf C1 /
> > | |
> > | |
> > | | flow 2:
> > | | / indirect handle H conf C2 /
> > | | | |
> > | | | |
> > | | | |
> > =========================================================
> > ^ | | | |
> > | | V | V
> > | ~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~
> > | flow mutable flow mutable
> > | context 1 context 2
> > | ~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~
> > indirect | | |
> > action | | |
> > context | V V
> > | -----------------------------------------------------
> > | action mutable context
> > | -----------------------------------------------------
> > v action immutable context
> > =========================================================
> >
> > Indirect action types - immutable, action / flow mutable, are mutually
> > exclusive and depend on the action definition.
> > For example:
> > • Indirect METER_MARK policy is immutable action member and profile is
> > action mutable action member.
> > • Indirect METER_MARK flow action defines init_color as flow mutable
> > member.
> > • Indirect QUOTA flow action does not define flow mutable members.
> >
> > Template API:
> >
> > Action template format:
> >
> > template .. indirect_list handle Htmpl conf Ctmpl ..
> > mask .. indirect_list handle Hmask conf Cmask ..
> >
> > 1 If Htmpl was masked (Hmask != 0), PMD compiles base action
> > configuration during action template, table template and flow rule
> > phases - depending on PMD action implementation.
> > Otherwise, action is compiled from scratch during flow rule
> > processing.
> >
> Why not just say if Hmask != NULL this action is fixed for this template,
> else action will be set per each flow?
>
> Also since this is a list of action it is possible that one action will have
> null
> while other will have a fixed value.
> This means that the Ctmpl should also be checked per each entery:
> Conf[0] != null, conf[1] != null.
> This will also mean that if application wants to use fixed confs he also
> must supply a valid example action_list, (can be different action_list object
> per flow but must be composed of the same actions.)
>
Will update indirect_list action template description in v4
> > 2 If Htmpl and Ctmpl were masked (Hmask !=0 and Cmask != 0), table
> > template processing overwrites base action configuration with Ctmpl
> > parameters.
> >
> What do you mean based action?
> If the Htmpl and Cmask is non zero it means that the configuration and
> action are based on the one set in the template create and not per flow.
>
> > Flow rule format:
> >
> > actions .. indirect_list handle Hflow conf Cflow ..
> >
> > 3 If Htmpl was masked, Hflow can reference a different action of the
> > same type as Htmpl.
> >
> > 4 If Cflow was specified, it overwrites action configuration.
> >
> > Signed-off-by: Gregory Etelson <getel...@nvidia.com>
> > ---
> > v3: do not deprecate indirect flow action.
> > ---
> > lib/ethdev/rte_flow.h | 266 +++++++++++++++++++++++++++++++++++
> > lib/ethdev/rte_flow_driver.h | 41 ++++++
> > 2 files changed, 307 insertions(+)
> >
> > diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index 713ba8b65c..ac1f51e564 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -2912,6 +2912,13 @@ enum rte_flow_action_type {
> > * applied to the given ethdev Rx queue.
> > */
> > RTE_FLOW_ACTION_TYPE_SKIP_CMAN,
> > +
> > + /**
> > + * Action handle to reference flow actions list.
> > + *
> > + * @see struct rte_flow_action_indirect_list
> > + */
> > + RTE_FLOW_ACTION_TYPE_INDIRECT_LIST,
> > };
> >
> > /**
> > @@ -6118,6 +6125,265 @@
> > rte_flow_async_action_handle_query_update(uint16_t port_id, uint32_t
> > queue_id,
> > void *user_data,
> > struct rte_flow_error *error);
> >
> > +struct rte_flow_action_list_handle;
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Configure INDIRECT_LIST flow action.
> > + *
> > + * @see RTE_FLOW_ACTION_TYPE_INDIRECT_LIST
> > + */
> > +struct rte_flow_action_indirect_list {
> > + struct rte_flow_action_list_handle; /**< Indirect action list handle */
> Since the comments above the member the comment should start with /** only
>
Will update the comment in v4.
> > + /**
> > + * Flow mutable configuration array.
> > + * NULL if the handle has no flow mutable configuration update.
> > + * Otherwise, if the handle was created with list A1 / A2 .. An / END
> > + * size of conf is n.
> > + * conf[i] points to flow mutable update of Ai in the handle
> > + * actions list or NULL if Ai has no update.
> > + */
> > + const void **conf;
> > +};
> > +
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Create an indirect flow action object from flow actions list.
> > + * The object is identified by a unique handle.
> > + * The handle has single state and configuration
> > + * across all the flow rules using it.
> > + *
> > + * @param[in] port_id
> > + * The port identifier of the Ethernet device.
> > + * @param[in] conf
> > + * Action configuration for the indirect action list creation.
> > + * @param[in] actions
> > + * Specific configuration of the indirect action lists.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL. PMDs initialize this
> > + * structure in case of error only.
> > + * @return
> > + * A valid handle in case of success, NULL otherwise and rte_errno is set
> > + * to one of the error codes defined:
> > + * - (-ENODEV) if *port_id* invalid.
> > + * - (-ENOSYS) if underlying device does not support this functionality.
> > + * - (-EIO) if underlying device is removed.
> > + * - (-EINVAL) if *actions* list invalid.
> > + * - (-ENOTSUP) if *action* list element valid but unsupported.
> > + * - (-E2BIG) to many elements in *actions*
> How can you get this error?
>
Will remove E2BIG in v4.
> > + */
> > +__rte_experimental
> > +struct rte_flow_action_list_handle *
> > +rte_flow_action_list_handle_create(uint16_t port_id,
> > + const
> > + struct rte_flow_indir_action_conf *conf,
> > + const struct rte_flow_action *actions,
> > + struct rte_flow_error *error);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Async function call to create an indirect flow action object
> > + * from flow actions list.
> > + * The object is identified by a unique handle.
> > + * The handle has single state and configuration
> > + * across all the flow rules using it.
> > + *
> > + * @param[in] port_id
> > + * The port identifier of the Ethernet device.
> > + * @param[in] queue_id
> > + * Flow queue which is used to update the rule.
> > + * @param[in] attr
> > + * Indirect action update operation attributes.
> > + * @param[in] conf
> > + * Action configuration for the indirect action list creation.
> > + * @param[in] actions
> > + * Specific configuration of the indirect action list.
> > + * @param[in] user_data
> > + * The user data that will be returned on async completion event.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL. PMDs initialize this
> > + * structure in case of error only.
> > + * @return
> > + * A valid handle in case of success, NULL otherwise and rte_errno is set
> > + * to one of the error codes defined:
> > + * - (-ENODEV) if *port_id* invalid.
> > + * - (-ENOSYS) if underlying device does not support this functionality.
> > + * - (-EIO) if underlying device is removed.
> > + * - (-EINVAL) if *actions* list invalid.
> > + * - (-ENOTSUP) if *action* list element valid but unsupported.
> > + * - (-E2BIG) to many elements in *actions*
> Same as above question
>
Will remove E2BIG in v4.
> > + */
> > +__rte_experimental
> > +struct rte_flow_action_list_handle *
> > +rte_flow_async_action_list_handle_create(uint16_t port_id, uint32_t
> > queue_id,
> > + const struct rte_flow_op_attr *attr,
> > + const struct
> > + rte_flow_indir_action_conf *conf,
> > + const struct rte_flow_action
> > *actions,
> > + void *user_data,
> > + struct rte_flow_error *error);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Destroy indirect actions list by handle.
> > + *
> > + * @param[in] port_id
> > + * The port identifier of the Ethernet device.
> > + * @param[in] handle
> > + * Handle for the indirect actions list to be destroyed.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL. PMDs initialize this
> > + * structure in case of error only.
> > + * @return
> > + * - (0) if success.
> > + * - (-ENODEV) if *port_id* invalid.
> > + * - (-ENOSYS) if underlying device does not support this functionality.
> > + * - (-EIO) if underlying device is removed.
> > + * - (-ENOENT) if actions list pointed by *action* handle was not found.
> > + * - (-EBUSY) if actions list pointed by *action* handle still used
> > + * rte_errno is also set.
> Why not keep the same return description as above?
>
Will update the format in v4.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_action_list_handle_destroy(uint16_t port_id,
> > + struct rte_flow_action_list_handle
> > *handle,
> > + struct rte_flow_error *error);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Enqueue indirect action list destruction operation.
> > + * The destroy queue must be the same
> > + * as the queue on which the action was created.
> > + *
> > + * @param[in] port_id
> > + * Port identifier of Ethernet device.
> > + * @param[in] queue_id
> > + * Flow queue which is used to destroy the rule.
> > + * @param[in] op_attr
> > + * Indirect action destruction operation attributes.
> > + * @param[in] handle
> > + * Handle for the indirect action object to be destroyed.
> > + * @param[in] user_data
> > + * The user data that will be returned on the completion events.
> > + * @param[out] error
> > + * Perform verbose error reporting if not NULL.
> > + * PMDs initialize this structure in case of error only.
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise and rte_errno is set.
> Same errors as above?
>
Will update the format in v4.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_async_action_list_handle_destroy
> > + (uint16_t port_id, uint32_t queue_id,
> > + const struct rte_flow_op_attr *op_attr,
> > + struct rte_flow_action_list_handle *handle,
> > + void *user_data, struct rte_flow_error *error);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Query and/or update indirect flow actions list.
> > + * If both query and update not NULL, the function atomically
> > + * queries and updates indirect action. Query and update are carried in
> > order
> > + * specified in the mode parameter.
> > + * If ether query or update is NULL, the function executes
> > + * complementing operation.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param handle
> > + * Handle for the indirect actions list object to be updated.
> > + * @param update
> > + * If not NULL, update profile specification used to modify the action
> > + * pointed by handle.
> > + * @see struct rte_flow_action_indirect_list
> It should be clear that the update/query is in the order the action was
> created and if the action i doesn't have configuration it should be s set to
> NULL.
> And goes without saying that the list size must be equal to the number of
> actions,
> that this list is build from.
>
Will update in v4.
> > + * @param query
> > + * If not NULL pointer to storage for the associated query data type.
> > + * @see struct rte_flow_action_indirect_list
> > + * @param mode
> > + * Operational mode.
> > + * @param error
> > + * Perform verbose error reporting if not NULL.
> > + * PMDs initialize this structure in case of error only.
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise and rte_errno is set.
> > + * - (-ENODEV) if *port_id* invalid.
> > + * - (-ENOTSUP) if underlying device does not support this functionality.
> > + * - (-EINVAL) if *handle* or *mode* invalid or
> > + * both *query* and *update* are NULL.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_action_list_handle_query_update(uint16_t port_id,
> > + const struct
> > + rte_flow_action_list_handle *handle,
> > + const void **update, void **query,
> > + enum rte_flow_query_update_mode
> > mode,
> > + struct rte_flow_error *error);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Enqueue async indirect flow actions list query and/or update
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param queue_id
> > + * Flow queue which is used to update the rule.
> > + * @param attr
> > + * Indirect action update operation attributes.
> > + * @param handle
> > + * Handle for the indirect actions list object to be updated.
> > + * @param update
> > + * If not NULL, update profile specification used to modify the action
> > + * pointed by handle.
> > + * @see struct rte_flow_action_indirect_list
> > + * @param query
> > + * If not NULL, pointer to storage for the associated query data type.
> > + * Query result returned on async completion event.
> > + * @see struct rte_flow_action_indirect_list
> > + * @param mode
> > + * Operational mode.
> > + * @param user_data
> > + * The user data that will be returned on async completion event.
> > + * @param error
> > + * Perform verbose error reporting if not NULL.
> > + * PMDs initialize this structure in case of error only.
> > + *
> > + * @return
> > + * 0 on success, a negative errno value otherwise and rte_errno is set.
> > + * - (-ENODEV) if *port_id* invalid.
> > + * - (-ENOTSUP) if underlying device does not support this functionality.
> > + * - (-EINVAL) if *handle* or *mode* invalid or
> > + * both *update* and *query* are NULL.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_async_action_list_handle_query_update(uint16_t port_id,
> > uint32_t queue_id,
> > + const struct rte_flow_op_attr *attr,
> > + const struct
> > + rte_flow_action_list_handle
> > *handle,
> > + const void *update, void *query,
> > + enum
> > rte_flow_query_update_mode mode,
> > + void *user_data,
> > + struct rte_flow_error *error);
> > +
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/ethdev/rte_flow_driver.h b/lib/ethdev/rte_flow_driver.h
> > index a129a4605d..8dc803023c 100644
> > --- a/lib/ethdev/rte_flow_driver.h
> > +++ b/lib/ethdev/rte_flow_driver.h
> > @@ -121,6 +121,17 @@ struct rte_flow_ops {
> > const void *update, void *query,
> > enum rte_flow_query_update_mode qu_mode,
> > struct rte_flow_error *error);
> > + /** @see rte_flow_action_list_handle_create() */
> > + struct rte_flow_action_list_handle *(*action_list_handle_create)
> > + (struct rte_eth_dev *dev,
> > + const struct rte_flow_indir_action_conf *conf,
> > + const struct rte_flow_action actions[],
> > + struct rte_flow_error *error);
> > + /** @see rte_flow_action_list_handle_destroy() */
> > + int (*action_list_handle_destroy)
> > + (struct rte_eth_dev *dev,
> > + struct rte_flow_action_list_handle *handle,
> > + struct rte_flow_error *error);
> > /** See rte_flow_tunnel_decap_set() */
> > int (*tunnel_decap_set)
> > (struct rte_eth_dev *dev,
> > @@ -302,6 +313,36 @@ struct rte_flow_ops {
> > const void *update, void *query,
> > enum rte_flow_query_update_mode qu_mode,
> > void *user_data, struct rte_flow_error *error);
> > + /** @see rte_flow_async_action_list_handle_create() */
> > + struct rte_flow_action_list_handle *
> > + (*async_action_list_handle_create)
> > + (struct rte_eth_dev *dev, uint32_t queue_id,
> > + const struct rte_flow_op_attr *attr,
> > + const struct rte_flow_indir_action_conf *conf,
> > + const struct rte_flow_action *actions,
> > + void *user_data, struct rte_flow_error *error);
> > + /** @see rte_flow_async_action_list_handle_destroy() */
> > + int (*async_action_list_handle_destroy)
> > + (struct rte_eth_dev *dev, uint32_t queue_id,
> > + const struct rte_flow_op_attr *op_attr,
> > + struct rte_flow_action_list_handle *action_handle,
> > + void *user_data, struct rte_flow_error *error);
> > + /** @see rte_flow_action_list_handle_query_update() */
> > + int (*action_list_handle_query_update)
> > + (uint16_t port_id,
> > + const struct rte_flow_action_list_handle *handle,
> > + const void **update, void **query,
> > + enum rte_flow_query_update_mode mode,
> > + struct rte_flow_error *error);
> > + /** @see rte_flow_async_action_list_handle_query_update() */
> > + int (*async_action_list_handle_query_update)
> > + (uint16_t port_id, uint32_t queue_id,
> > + const struct rte_flow_op_attr *attr,
> > + const struct rte_flow_action_list_handle *handle,
> > + const void **update, void **query,
> > + enum rte_flow_query_update_mode mode,
> > + void *user_data, struct rte_flow_error *error);
> > +
> > };
> >
> > /**
> > --
> > 2.34.1
>
> Best,
> Ori
