Re: [dpdk-dev] [PATCH v4 2/6] ethdev: add capability to keep shared objects on restart

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 10/21/21 9:34 AM, Dmitry Kozlyuk wrote:
> rte_flow_action_handle_create() did not mention what happens
> with an indirect action when a device is stopped and started again.
> It is natural for some indirect actions, like counter, to be persistent.
> Keeping others at least saves application time and complexity.
> However, not all PMDs can support it, or the support may be limited
> by particular action kinds, that is, combinations of action type
> and the value of the transfer bit in its configuration.
> 
> Add a device capability to indicate if at least some indirect actions
> are kept across the above sequence. Without this capability the behavior
> is still unspecified, and application is required to destroy
> the indirect actions before stopping the device.
> In the future, indirect actions may not be the only type of objects
> shared between flow rules. The capability bit intends to cover all
> possible types of such objects, hence its name.
> 
> Declare that the application can test for the persistence
> of a particular indirect action kind by attempting to create
> an indirect action of that kind when the device is stopped
> and checking for the specific error type.
> This is logical because if the PMD can to create an indirect action
> when the device is not started and use it after the start happens,
> it is natural that it can move its internal flow shared object
> to the same state when the device is stopped and restore the state
> when the device is started.
> 
> Indirect action persistence across a reconfigurations is not required.
> In case a PMD cannot keep the indirect actions across reconfiguration,
> it is allowed just to report an error.
> Application must then flush the indirect actions before attempting it.
> 
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

with below review notes processed

> ---
>   doc/guides/prog_guide/rte_flow.rst | 26 ++++++++++++++++++++++++++
>   lib/ethdev/rte_ethdev.h            |  3 +++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 9beaae3df3..bef143862b 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2965,6 +2965,32 @@ 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()``.
>   
> +If ``RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP`` is not advertised,
> +indirect actions cannot be created until the device is started for the first time
> +and cannot be kept when the device is stopped.
> +However, PMD also does not flush them automatically on stop,
> +so the application must call ``rte_flow_action_handle_destroy()``
> +before stopping the device to ensure no indirect actions remain.
> +
> +If ``RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP`` is advertised,
> +this means that the PMD can keep at least some indirect actions
> +across device stop and start.
> +However, ``rte_eth_dev_configure()`` may fail if any indirect actions remain,
> +so the application must destroy them before attempting a reconfiguration.
> +Keeping may be only supported for certain kinds of indirect actions.
> +A kind is a combination of an action type and a value of its transfer bit.
> +For example: an indirect counter with the transfer bit reset.
> +To test if a particular kind of indirect actions is kept,
> +the application must try to create a valid indirect action of that kind
> +when the device is not started (either before the first start of after a stop).
> +If it fails with an error of type ``RTE_FLOW_ERROR_TYPE_STATE``,
> +application must destroy all indirect actions of this kind
> +before stopping the device.
> +If it succeeds, all indirect actions of the same kind are kept
> +when the device is stopped.
> +Indirect actions of a kept kind that are created when the device is stopped,
> +including the ones created for the test, will be kept after the device start.
> +
>   .. _table_rte_flow_action_handle:
>   
>   .. table:: INDIRECT
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 9cf23fecce..5375844484 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -94,6 +94,7 @@
>    * depending on the device capabilities:
>    *
>    *     - flow rules
> + *     - flow-related shared objects, e.g. indirect actions
>    *
>    * Any other configuration will not be stored and will need to be re-entered
>    * before a call to rte_eth_dev_start().
> @@ -1452,6 +1453,8 @@ struct rte_eth_conf {
>   #define RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP 0x00000002
>   /** Device supports keeping flow rules across restart. */
>   #define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP 0x00000004
> +/** Device supports keeping shared flow objects across restart. */
> +#define RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP 0x00000008

RTE_BIT32(3) plus experimental markup

>   /**@}*/
>   
>   /*
>