[v4,2/6] ethdev: add capability to keep shared objects on restart

  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>
---
 doc/guides/prog_guide/rte_flow.rst | 26 ++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h            |  3 +++
 2 files changed, 29 insertions(+)
  

Hi Dmitry,

> -----Original Message-----
> From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Sent: Thursday, October 21, 2021 9:35 AM
> Subject: [PATCH v4 2/6] ethdev: add capability to keep shared objects on restart
> 
> 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>
> ---
>  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
>  /**@}*/
> 
>  /*
> --
> 2.25.1

Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori
  

On Wed, Oct 20, 2021 at 11:35 PM Dmitry Kozlyuk <dkozlyuk@oss.nvidia.com> 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: Ajit Khaparde <ajit.khaparde@broadcom.com>

>
  

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

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