Re: [dpdk-dev] [PATCH v3 1/6] ethdev: add capability to keep flow rules on restart

[Ori Kam <orika@nvidia.com>]

Hi Dmitry,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dmitry Kozlyuk
> Sent: Tuesday, October 19, 2021 3:37 PM
> To: dev@dpdk.org
> Cc: Qi Zhang <qi.z.zhang@intel.com>; Ori Kam <orika@oss.nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Subject: [dpdk-dev] [PATCH v3 1/6] ethdev: add capability to keep flow rules on restart
> 
> Previously, it was not specified what happens to the flow rules when the device is stopped, possibly
> reconfigured, then started.
> If flow rules were kept, it could be convenient for application developers, because they wouldn't need
> to save and restore them.
> However, due to the number of flows and possible creation rate it is impractical to save all flow rules in
> DPDK layer. This means that flow rules persistence really depends on whether PMD and HW can
> implement it efficiently. It can also be limited by the rule item and action types, and its attributes
> transfer bit (a combination of an item/action type and a value of the transfer bit is called a ruel
> feature).
> 
> Add a device capability bit for PMDs that can keep at least some of the flow rules across restart.
> Without this capability behavior is still unspecified and it is declared that the application must flush the
> rules before stopping the device.
> Allow the application to test for persitence of rules using a particular feature by attempting to create a
> flow rule using that feature when the device is stopped and checking for the specific error.
> This is logical because if the PMD can to create the flow rule when the device is not started and use it
> after the start happens, it is natural that it can move its internal flow rule object to the same state
> when the device is stopped and restore the state when the device is started.
> 
> Rule persistence across a reconfigurations is not required, because tracking all the rules and
> configuration-dependent resources they use may be infeasible. In case a PMD cannot keep the rules
> across reconfiguration, it is allowed just to report an error.
> Application must then flush the rules before attempting it.
> 
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 25 +++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h            |  7 +++++++
>  lib/ethdev/rte_flow.h              |  1 +
>  3 files changed, 33 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 2b42d5ec8c..ff67b211e3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -87,6 +87,31 @@ To avoid resource leaks on the PMD side, handles must be explicitly  destroyed
> by the application before releasing associated resources such as  queues and ports.
> 
> +If ``RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP`` is not advertised, rules 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_flush()`` or ``rte_flow_destroy()``
> +before stopping the device to ensure no rules remain.
> +
> +If ``RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP`` is advertised, this means the
> +PMD can keep at least some rules across the device stop and start.
> +However, ``rte_eth_dev_configure()`` may fail if any rules remain, so
> +the application must flush them before attempting a reconfiguration.
> +Keeping may be unsupported for some types of rule items and actions, as
> +well as depending on the value of flow attributes transfer bit.
> +A combination of an item or action type and a value of the transfer bit
> +is called a rule feature.
> +To test if rules with a particular feature are kept, the application
> +must try to create a valid rule using this feature when the device is
> +stopped (after it has been configured or started previously).
> +If it fails with an error of type ``RTE_FLOW_ERROR_TYPE_STATE``, rules
> +using this feature are flushed when the device is stopped.
> +If it suceeds, such rules will be kept when the device is stopped,
> +provided they do not use other features that are not supported.
> +Rules that are created when the device is stopped, including the rules
> +created for the test, will be kept after the device is started.
> +
>  The following sections cover:
> 
>  - **Attributes** (represented by ``struct rte_flow_attr``): properties of a diff --git
> a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index 6d80514ba7..a0b388bb25 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -90,6 +90,11 @@
>   *     - flow director filtering mode (but not filtering rules)
>   *     - NIC queue statistics mappings
>   *
> + * The following configuration may be retained or not
> + * depending on the device capabilities:
> + *
> + *     - flow rules
> + *
>   * Any other configuration will not be stored and will need to be re-entered
>   * before a call to rte_eth_dev_start().
>   *
> @@ -1445,6 +1450,8 @@ struct rte_eth_conf {  #define
> RTE_ETH_DEV_CAPA_RUNTIME_RX_QUEUE_SETUP 0x00000001
>  /** Device supports Tx queue setup after device started. */  #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
>  /**@}*/
> 
>  /*
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index a89945061a..aa0182d021 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3344,6 +3344,7 @@ enum rte_flow_error_type {
>  	RTE_FLOW_ERROR_TYPE_ACTION_NUM, /**< Number of actions. */
>  	RTE_FLOW_ERROR_TYPE_ACTION_CONF, /**< Action configuration. */
>  	RTE_FLOW_ERROR_TYPE_ACTION, /**< Specific action. */
> +	RTE_FLOW_ERROR_TYPE_STATE, /**< Current device state. */
>  };
> 
>  /**
> --
> 2.25.1

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