diff mbox series

[4/4] ethdev: document indirect flow action life cycle

Message ID 20210727073121.895620-5-dkozlyuk@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers show
Series net/mlx5: keep indirect actions across port restart | expand

Checks

Context Check Description
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot success github build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Dmitry Kozlyuk July 27, 2021, 7:31 a.m. UTC
rte_flow_action_handle_create() did not specify what happens
with an indirect action when device is stopped, possibly reconfigured,
and started again.

It is proposed that indirect actions persisted across such a sequence.
This allows for easier API usage and better HW resources utilization
by saving indirect actions flush and re-creation with associated error
handling and rollback.

If between stop and start a device is reconfigured in a way that is
incompatible with an existing indirect action, PMD is required to report
an error at the device start. This is mandatory, because flow API does
not supply users with capabilities, so this is the only way for a user
to learn that configuration is invalid. Errors are not reported
at configuration stage to give the user a chance to remove or change
offending actions. For example, if number of queues changes and an RSS
indirect action specifies queues that went away, user must update
the action before starting the device. PMD is not allowed to silently
adjust indirect actions (in the same example, to remove queues from
the RSS), so that all configuration is explicit.

Fixes: 4b61b8774be9 ("ethdev: introduce indirect flow action")
Cc: bingz@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 10 ++++++++++
 lib/ethdev/rte_flow.h              |  4 ++++
 2 files changed, 14 insertions(+)

Comments

Ori Kam July 28, 2021, 9:50 a.m. UTC | #1
Hi Dmitry,

> -----Original Message-----
> From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Sent: Tuesday, July 27, 2021 10:31 AM
>
> Subject: [PATCH 4/4] ethdev: document indirect flow action life cycle
> 
> rte_flow_action_handle_create() did not specify what happens with an
> indirect action when device is stopped, possibly reconfigured, and started
> again.
> 
> It is proposed that indirect actions persisted across such a sequence.
> This allows for easier API usage and better HW resources utilization by saving
> indirect actions flush and re-creation with associated error handling and
> rollback.
> 
> If between stop and start a device is reconfigured in a way that is
> incompatible with an existing indirect action, PMD is required to report an
> error at the device start. This is mandatory, because flow API does not supply
> users with capabilities, so this is the only way for a user to learn that
> configuration is invalid. Errors are not reported at configuration stage to give
> the user a chance to remove or change offending actions. For example, if
> number of queues changes and an RSS indirect action specifies queues that
> went away, user must update the action before starting the device. PMD is
> not allowed to silently adjust indirect actions (in the same example, to
> remove queues from the RSS), so that all configuration is explicit.
> 
I think some of the errors can be checked during configuration.
For example when app removes a queue the PMD can check if there is any
reference on this queue and fail at this point.

> Fixes: 4b61b8774be9 ("ethdev: introduce indirect flow action")
> Cc: bingz@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 10 ++++++++++
>  lib/ethdev/rte_flow.h              |  4 ++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..06dd06d9a6 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2785,6 +2785,16 @@ updated depend on the type of the ``action`` and
> different for every type.
>  The indirect action specified data (e.g. counter) can be queried by
> ``rte_flow_action_handle_query()``.
> 
> +Indirect actions persist across device configure, stop, and start.
+1
> +If a new configuration is incompatible with an existing indirect
> +action, the start operation will fail. "Incompatible" means that if
> +this action was destroyed and created again, creation would fail.

I think this test should be done during the configuration.
It is the application responsibility to use valid actions, it should be stated
that changing the values of configuration may result in in valid actions.

> +It is a programmer's responsibility to remove or update offending actions.
> +
> +PMD developers should use the same diagnostics for
> +``rte_eth_dev_start()`` as for ``rte_flow_action_handle_create()``. PMD
> +is not allowed to silently ignore or correct offending actions.
> +
>  .. _table_rte_flow_action_handle:
> 
>  .. table:: INDIRECT
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
> 70f455d47d..f571a27fe7 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3969,6 +3969,10 @@ struct rte_flow_indir_action_conf {
>   * The created object handle has single state and configuration
>   * across all the flow rules using it.
>   *
> + * Indirect actions persist across device configure, stop, and start.
> + * If a new configuration is incompatible with an existing indirect
> + * action, rte_eth_dev_start() will fail.
> + *

I don't think that dev start should check configuration it may slow down startup.

>   * @param[in] port_id
>   *    The port identifier of the Ethernet device.
>   * @param[in] conf
> --
> 2.25.1

Thanks,
Ori
diff mbox series

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 2b42d5ec8c..06dd06d9a6 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2785,6 +2785,16 @@  updated depend on the type of the ``action`` and different for every type.
 The indirect action specified data (e.g. counter) can be queried by
 ``rte_flow_action_handle_query()``.
 
+Indirect actions persist across device configure, stop, and start.
+If a new configuration is incompatible with an existing indirect action,
+the start operation will fail. "Incompatible" means that if this action
+was destroyed and created again, creation would fail.
+It is a programmer's responsibility to remove or update offending actions.
+
+PMD developers should use the same diagnostics for ``rte_eth_dev_start()``
+as for ``rte_flow_action_handle_create()``. PMD is not allowed to silently
+ignore or correct offending actions.
+
 .. _table_rte_flow_action_handle:
 
 .. table:: INDIRECT
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 70f455d47d..f571a27fe7 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3969,6 +3969,10 @@  struct rte_flow_indir_action_conf {
  * The created object handle has single state and configuration
  * across all the flow rules using it.
  *
+ * Indirect actions persist across device configure, stop, and start.
+ * If a new configuration is incompatible with an existing indirect
+ * action, rte_eth_dev_start() will fail.
+ *
  * @param[in] port_id
  *    The port identifier of the Ethernet device.
  * @param[in] conf