Re: [dpdk-dev] [PATCH v4 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: Thursday, October 21, 2021 9:35 AM
> To: dev@dpdk.org
> Cc: 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 v4 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 persistence 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 | 31 ++++++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h            |  7 +++++++
>  lib/ethdev/rte_flow.h              |  1 +
>  3 files changed, 39 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index aeba374182..9beaae3df3 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -87,6 +87,37 @@ 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.
> 
> +When the device is stopped, its rules do not process the traffic.
> +In particular, transfer rules created using some device
> +stop affecting the traffic even if they refer to different 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 a single an item or action type
> +and a value of the transfer bit is called a rule feature.
> +For example: a COUNT action with the transfer bit set.
> +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 not started
> +(either before the first start or after a stop).
> +If it fails with an error of type ``RTE_FLOW_ERROR_TYPE_STATE``,
> +all rules using this feature must be flushed by the application
> +before stopping the device.
> +If it succeeds, 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 014270d316..9cf23fecce 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 2b6efeef8c..16ef33819b 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3676,6 +3676,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