[v4,1/2] ethdev: add query_update sync and async function calls
Checks
Commit Message
Current API allows either query or update indirect flow action.
If port hardware allows both update and query in a single operation,
application still has to issue 2 separate hardware requests.
The patch adds `rte_flow_action_handle_query_update` function call,
and it's async version `rte_flow_async_action_handle_query_update`
to atomically query and update flow action.
int
rte_flow_action_handle_query_update
(uint16_t port_id, struct rte_flow_action_handle *handle,
const void *update, void *query,
enum rte_flow_query_update_mode mode,
struct rte_flow_error *error);
int
rte_flow_async_action_handle_query_update
(uint16_t port_id, uint32_t queue_id,
const struct rte_flow_op_attr *op_attr,
struct rte_flow_action_handle *action_handle,
const void *update, void *query,
enum rte_flow_query_update_mode mode,
void *user_data, struct rte_flow_error *error);
Application can control query and update order, if that is supported
by port hardware, by setting qu_mode parameter to
RTE_FLOW_QU_QUERY_FIRST or RTE_FLOW_QU_UPDATE_FIRST.
RTE_FLOW_QU_QUERY and RTE_FLOW_QU_UPDATE parameter values provide
query only and update only functionality for backward compatibility
with existing API.
Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
v2: Remove RTE_FLOW_QU_DEFAULT query-update mode.
v3: Update release release notes.
Fix doxygen errors.
v4: Add returned errno codes.
---
doc/guides/rel_notes/release_23_03.rst | 7 ++
lib/ethdev/rte_flow.c | 39 +++++++++
lib/ethdev/rte_flow.h | 107 +++++++++++++++++++++++++
lib/ethdev/rte_flow_driver.h | 15 ++++
lib/ethdev/version.map | 5 ++
5 files changed, 173 insertions(+)
Comments
18/01/2023 11:31, Gregory Etelson:
> Current API allows either query or update indirect flow action.
> If port hardware allows both update and query in a single operation,
> application still has to issue 2 separate hardware requests.
>
> The patch adds `rte_flow_action_handle_query_update` function call,
> and it's async version `rte_flow_async_action_handle_query_update`
> to atomically query and update flow action.
What is the benefit?
Is it a performance optimization? How much?
> int
> rte_flow_action_handle_query_update
> (uint16_t port_id, struct rte_flow_action_handle *handle,
> const void *update, void *query,
> enum rte_flow_query_update_mode mode,
> struct rte_flow_error *error);
>
> int
> rte_flow_async_action_handle_query_update
> (uint16_t port_id, uint32_t queue_id,
> const struct rte_flow_op_attr *op_attr,
> struct rte_flow_action_handle *action_handle,
> const void *update, void *query,
> enum rte_flow_query_update_mode mode,
> void *user_data, struct rte_flow_error *error);
No need to copy the functions in the commit log.
> Application can control query and update order, if that is supported
> by port hardware, by setting qu_mode parameter to
> RTE_FLOW_QU_QUERY_FIRST or RTE_FLOW_QU_UPDATE_FIRST.
>
> RTE_FLOW_QU_QUERY and RTE_FLOW_QU_UPDATE parameter values provide
> query only and update only functionality for backward compatibility
> with existing API.
Why do we need such compatibility?
The existing functions will stay, isn't it?
[...]
> + * RTE_FLOW_QU_QUERY_FIRST
> + * Force port to query action before update.
> + * RTE_FLOW_QU_UPDATE_FIRST
> + * Force port to update action before update.
You mean "before query".
Anyway you don't need to repeat the enum in the comment.
> + *
> + * @see rte_flow_action_handle_query_update()
> + * @see rte_flow_async_action_handle_query_update()
> + */
> +enum rte_flow_query_update_mode {
> + RTE_FLOW_QU_QUERY_FIRST, /* query before update */
> + RTE_FLOW_QU_UPDATE_FIRST, /* query after update */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Query and/or update indirect flow action.
> + * If update parameter is NULL the function queries indirect action.
> + * If query parameter is NULL the function updates indirect action.
> + * If both query and update not NULL, the function atomically
> + * queries and updates indirect action. Query and update carried in order
"are" carried?
> + * specified in the mode parameter.
> + *
> + * @param port_id
> + * Port identifier of Ethernet device.
> + * @param handle
> + * Handle for the indirect action object to be updated.
> + * @param update
> + * Update profile specification used to modify the action pointed by handle.
> + * *update* could be with the same type of the immediate action corresponding
could be "of" the same type
> + * to the *handle* argument when creating, or a wrapper structure includes
> + * action configuration to be updated and bit fields to indicate the member
> + * of fields inside the action to update.
> + * @param query
> + * Pointer to storage for the associated query data type.
> + * @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.
> + * - (ENOTSUP) if underlying device does not support this functionality.
> + * - (EINVAL) - if *handle* invalid
> + */
> +__rte_experimental
> +int
> +rte_flow_action_handle_query_update(uint16_t port_id,
> + struct rte_flow_action_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 action 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 action object to be updated.
> + * @param update
> + * Update profile specification used to modify the action pointed by handle.
> + * *update* could be with the same type of the immediate action corresponding
> + * to the *handle* argument when creating, or a wrapper structure includes
includes -> including
> + * action configuration to be updated and bit fields to indicate the member
> + * of fields inside the action to update.
Where can we find how such wrapper should look like?
> + * @param query
> + * Pointer to storage for the associated query data type.
> + * Query result returned on async completion event.
> + * @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.
> + * - (ENOTSUP) if underlying device does not support this functionality.
> + * - (EINVAL) - if *handle* invalid
> + */
> +__rte_experimental
> +int
> +rte_flow_async_action_handle_query_update(uint16_t port_id, uint32_t queue_id,
> + const struct rte_flow_op_attr *attr,
> + struct rte_flow_action_handle *handle,
> + const void *update, void *query,
> + enum rte_flow_query_update_mode mode,
> + void *user_data,
> + struct rte_flow_error *error);
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -298,6 +298,11 @@ EXPERIMENTAL {
> rte_flow_get_q_aged_flows;
> rte_mtr_meter_policy_get;
> rte_mtr_meter_profile_get;
> +
> + # future
It should be "# added in 23.03"
> + rte_flow_action_handle_query_update;
> + rte_flow_async_action_handle_query_update;
Hello Thomas,
[]
> > Current API allows either query or update indirect flow action.
> > If port hardware allows both update and query in a single operation,
> > application still has to issue 2 separate hardware requests.
> >
> > The patch adds `rte_flow_action_handle_query_update` function call,
> > and it's async version `rte_flow_async_action_handle_query_update`
> > to atomically query and update flow action.
>
> What is the benefit?
> Is it a performance optimization? How much?
>
rte_flow_action_handle_query_update() can query data and conditionally update it in a single HW operation.
That provides accuracy that cannot be achieved with existing API.
For example, quota flow action must be updated when it in the PASS state only.
If application use existing query, by the time it gets query data and analyzes it, HW object state can change.
As the result application update action will not reflect HW configuration.
The function can provide general optimization, but that was not tested.
> > int
> > rte_flow_action_handle_query_update
> > (uint16_t port_id, struct rte_flow_action_handle *handle,
> > const void *update, void *query,
> > enum rte_flow_query_update_mode mode,
> > struct rte_flow_error *error);
> >
> > int
> > rte_flow_async_action_handle_query_update
> > (uint16_t port_id, uint32_t queue_id,
> > const struct rte_flow_op_attr *op_attr,
> > struct rte_flow_action_handle *action_handle,
> > const void *update, void *query,
> > enum rte_flow_query_update_mode mode,
> > void *user_data, struct rte_flow_error *error);
>
> No need to copy the functions in the commit log.
>
Will be updated in v5
> > Application can control query and update order, if that is supported
> > by port hardware, by setting qu_mode parameter to
> > RTE_FLOW_QU_QUERY_FIRST or RTE_FLOW_QU_UPDATE_FIRST.
> >
> > RTE_FLOW_QU_QUERY and RTE_FLOW_QU_UPDATE parameter values
> provide
> > query only and update only functionality for backward compatibility
> > with existing API.
>
> Why do we need such compatibility?
> The existing functions will stay, isn't it?
>
rte_flow_action_handle_query_update() has extended functionality with better HW support.
The plan is to deprecate and replace existing query and update functions.
> [...]
> > + * RTE_FLOW_QU_QUERY_FIRST
> > + * Force port to query action before update.
> > + * RTE_FLOW_QU_UPDATE_FIRST
> > + * Force port to update action before update.
>
> You mean "before query".
> Anyway you don't need to repeat the enum in the comment.
>
Correct, must be "before query".
Will fix in v5.
> > + *
> > + * @see rte_flow_action_handle_query_update()
> > + * @see rte_flow_async_action_handle_query_update()
> > + */
> > +enum rte_flow_query_update_mode {
> > + RTE_FLOW_QU_QUERY_FIRST, /* query before update */
> > + RTE_FLOW_QU_UPDATE_FIRST, /* query after update */
> > +};
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Query and/or update indirect flow action.
> > + * If update parameter is NULL the function queries indirect action.
> > + * If query parameter is NULL the function updates indirect action.
> > + * If both query and update not NULL, the function atomically
> > + * queries and updates indirect action. Query and update carried in
> order
>
> "are" carried?
>
Typo. Will fix in v5.
> > + * specified in the mode parameter.
> > + *
> > + * @param port_id
> > + * Port identifier of Ethernet device.
> > + * @param handle
> > + * Handle for the indirect action object to be updated.
> > + * @param update
> > + * Update profile specification used to modify the action pointed by
> handle.
> > + * *update* could be with the same type of the immediate action
> corresponding
>
> could be "of" the same type
>
Will fix in v5.
> > + * to the *handle* argument when creating, or a wrapper structure
> includes
> > + * action configuration to be updated and bit fields to indicate the
> member
> > + * of fields inside the action to update.
> > + * @param query
> > + * Pointer to storage for the associated query data type.
> > + * @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.
> > + * - (ENOTSUP) if underlying device does not support this functionality.
> > + * - (EINVAL) - if *handle* invalid
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_action_handle_query_update(uint16_t port_id,
> > + struct rte_flow_action_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 action 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 action object to be updated.
> > + * @param update
> > + * Update profile specification used to modify the action pointed by
> handle.
> > + * *update* could be with the same type of the immediate action
> corresponding
> > + * to the *handle* argument when creating, or a wrapper structure
> includes
>
> includes -> including
>
Will fix in v5.
> > + * action configuration to be updated and bit fields to indicate the
> member
> > + * of fields inside the action to update.
>
> Where can we find how such wrapper should look like?
>
That patch will be sent later.
I'll rephrase.
> > + * @param query
> > + * Pointer to storage for the associated query data type.
> > + * Query result returned on async completion event.
> > + * @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.
> > + * - (ENOTSUP) if underlying device does not support this functionality.
> > + * - (EINVAL) - if *handle* invalid
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_async_action_handle_query_update(uint16_t port_id, uint32_t
> queue_id,
> > + const struct rte_flow_op_attr *attr,
> > + struct rte_flow_action_handle *handle,
> > + const void *update, void *query,
> > + enum rte_flow_query_update_mode mode,
> > + void *user_data,
> > + struct rte_flow_error *error);
>
> > --- a/lib/ethdev/version.map
> > +++ b/lib/ethdev/version.map
> > @@ -298,6 +298,11 @@ EXPERIMENTAL {
> > rte_flow_get_q_aged_flows;
> > rte_mtr_meter_policy_get;
> > rte_mtr_meter_profile_get;
> > +
> > + # future
>
> It should be "# added in 23.03"
>
Will fix in v5.
> > + rte_flow_action_handle_query_update;
> > + rte_flow_async_action_handle_query_update;
>
>
18/01/2023 18:34, Gregory Etelson:
> > > Current API allows either query or update indirect flow action.
> > > If port hardware allows both update and query in a single operation,
> > > application still has to issue 2 separate hardware requests.
> > >
> > > The patch adds `rte_flow_action_handle_query_update` function call,
> > > and it's async version `rte_flow_async_action_handle_query_update`
> > > to atomically query and update flow action.
> >
> > What is the benefit?
> > Is it a performance optimization? How much?
> >
>
> rte_flow_action_handle_query_update() can query data and conditionally update it in a single HW operation.
> That provides accuracy that cannot be achieved with existing API.
> For example, quota flow action must be updated when it in the PASS state only.
> If application use existing query, by the time it gets query data and analyzes it, HW object state can change.
> As the result application update action will not reflect HW configuration.
An explanation of the atomicity benefit should be in the commit log.
> The function can provide general optimization, but that was not tested.
> > > Application can control query and update order, if that is supported
> > > by port hardware, by setting qu_mode parameter to
> > > RTE_FLOW_QU_QUERY_FIRST or RTE_FLOW_QU_UPDATE_FIRST.
> > >
> > > RTE_FLOW_QU_QUERY and RTE_FLOW_QU_UPDATE parameter values
> > provide
> > > query only and update only functionality for backward compatibility
> > > with existing API.
> >
> > Why do we need such compatibility?
> > The existing functions will stay, isn't it?
> >
>
> rte_flow_action_handle_query_update() has extended functionality with better HW support.
You cannot argue HW support without knowing what would be the support
of other vendors.
> The plan is to deprecate and replace existing query and update functions.
I disagree with such deprecation.
Anyway it is not the topic of this patch, so I suggest to remove the sentence
about compatibility.
@@ -55,6 +55,13 @@ New Features
Also, make sure to start the actual text at the margin.
=======================================================
+* **Added functions to atomically query and update indirect flow action.**
+
+ Added synchronous and asynchronous functions to atomically query and update
+ indirect flow action:
+
+ - ``rte_flow_action_handle_query_update``
+ - ``rte_flow_async_action_handle_query_update``
Removed Items
-------------
@@ -1883,3 +1883,42 @@ rte_flow_async_action_handle_query(uint16_t port_id,
action_handle, data, user_data, error);
return flow_err(port_id, ret, error);
}
+
+int
+rte_flow_action_handle_query_update(uint16_t port_id,
+ struct rte_flow_action_handle *handle,
+ const void *update, void *query,
+ enum rte_flow_query_update_mode mode,
+ struct rte_flow_error *error)
+{
+ struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+ const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+ int ret;
+
+ if (!ops || !ops->action_handle_query_update)
+ return -ENOTSUP;
+ ret = ops->action_handle_query_update(dev, handle, update, query,
+ mode, error);
+ return flow_err(port_id, ret, error);
+}
+
+int
+rte_flow_async_action_handle_query_update(uint16_t port_id, uint32_t queue_id,
+ const struct rte_flow_op_attr *attr,
+ struct rte_flow_action_handle *handle,
+ const void *update, void *query,
+ enum rte_flow_query_update_mode mode,
+ void *user_data,
+ struct rte_flow_error *error)
+{
+ struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+ const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
+ int ret;
+
+ if (!ops || !ops->async_action_handle_query_update)
+ return -ENOTSUP;
+ ret = ops->async_action_handle_query_update(dev, queue_id, attr,
+ handle, update, query, mode,
+ user_data, error);
+ return flow_err(port_id, ret, error);
+}
@@ -5622,6 +5622,113 @@ rte_flow_async_action_handle_query(uint16_t port_id,
void *user_data,
struct rte_flow_error *error);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Query_update operational mode.
+ *
+ * RTE_FLOW_QU_QUERY_FIRST
+ * Force port to query action before update.
+ * RTE_FLOW_QU_UPDATE_FIRST
+ * Force port to update action before update.
+ *
+ * @see rte_flow_action_handle_query_update()
+ * @see rte_flow_async_action_handle_query_update()
+ */
+enum rte_flow_query_update_mode {
+ RTE_FLOW_QU_QUERY_FIRST, /* query before update */
+ RTE_FLOW_QU_UPDATE_FIRST, /* query after update */
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Query and/or update indirect flow action.
+ * If update parameter is NULL the function queries indirect action.
+ * If query parameter is NULL the function updates indirect action.
+ * If both query and update not NULL, the function atomically
+ * queries and updates indirect action. Query and update carried in order
+ * specified in the mode parameter.
+ *
+ * @param port_id
+ * Port identifier of Ethernet device.
+ * @param handle
+ * Handle for the indirect action object to be updated.
+ * @param update
+ * Update profile specification used to modify the action pointed by handle.
+ * *update* could be with the same type of the immediate action corresponding
+ * to the *handle* argument when creating, or a wrapper structure includes
+ * action configuration to be updated and bit fields to indicate the member
+ * of fields inside the action to update.
+ * @param query
+ * Pointer to storage for the associated query data type.
+ * @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.
+ * - (ENOTSUP) if underlying device does not support this functionality.
+ * - (EINVAL) - if *handle* invalid
+ */
+__rte_experimental
+int
+rte_flow_action_handle_query_update(uint16_t port_id,
+ struct rte_flow_action_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 action 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 action object to be updated.
+ * @param update
+ * Update profile specification used to modify the action pointed by handle.
+ * *update* could be with the same type of the immediate action corresponding
+ * to the *handle* argument when creating, or a wrapper structure includes
+ * action configuration to be updated and bit fields to indicate the member
+ * of fields inside the action to update.
+ * @param query
+ * Pointer to storage for the associated query data type.
+ * Query result returned on async completion event.
+ * @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.
+ * - (ENOTSUP) if underlying device does not support this functionality.
+ * - (EINVAL) - if *handle* invalid
+ */
+__rte_experimental
+int
+rte_flow_async_action_handle_query_update(uint16_t port_id, uint32_t queue_id,
+ const struct rte_flow_op_attr *attr,
+ struct rte_flow_action_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
@@ -114,6 +114,13 @@ struct rte_flow_ops {
const struct rte_flow_action_handle *handle,
void *data,
struct rte_flow_error *error);
+ /** See rte_flow_action_handle_query_update() */
+ int (*action_handle_query_update)
+ (struct rte_eth_dev *dev,
+ struct rte_flow_action_handle *handle,
+ const void *update, void *query,
+ enum rte_flow_query_update_mode qu_mode,
+ struct rte_flow_error *error);
/** See rte_flow_tunnel_decap_set() */
int (*tunnel_decap_set)
(struct rte_eth_dev *dev,
@@ -276,6 +283,14 @@ struct rte_flow_ops {
void *data,
void *user_data,
struct rte_flow_error *error);
+ /** See rte_flow_async_action_handle_query_update */
+ int (*async_action_handle_query_update)
+ (struct rte_eth_dev *dev, uint32_t queue_id,
+ const struct rte_flow_op_attr *op_attr,
+ struct rte_flow_action_handle *action_handle,
+ const void *update, void *query,
+ enum rte_flow_query_update_mode qu_mode,
+ void *user_data, struct rte_flow_error *error);
};
/**
@@ -298,6 +298,11 @@ EXPERIMENTAL {
rte_flow_get_q_aged_flows;
rte_mtr_meter_policy_get;
rte_mtr_meter_profile_get;
+
+ # future
+ rte_flow_action_handle_query_update;
+ rte_flow_async_action_handle_query_update;
+
};
INTERNAL {