Re: [dpdk-dev] [RFC PATCH 2/2] ethdev: add capability to keep indirect actions on restart

[Ajit Khaparde <ajit.khaparde@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 4079 bytes --]

On Wed, Sep 1, 2021 at 1:55 AM Dmitry Kozlyuk <dkozlyuk@nvidia.com> wrote:
>
> rte_flow_action_handle_create() did not mention what happens
> with an indirect action when a device is stopped, possibly reconfigured,
> and started again. It is natural for some indirect actions to be
> persistent, like counters and meters; keeping others just saves
> application time and complexity. However, not all PMDs can support it.
> It is proposed to add a device capability to indicate if indirect actions
> are kept across the above sequence or implicitly destroyed.
>
> It may happen that in the future a PMD acquires support for a type of
> indirect actions that it cannot keep across a restart. It is undesirable
> to stop advertising the capability so that applications that don't use
> actions of the problematic type can still take advantage of it.
> This is why PMDs are allowed to keep only a subset of indirect actions
> provided that the vendor mandatorily documents it.
Sorry - I am seeing this late.
This could become confusing.
May be it is better for the PMDs to specify which actions are persistent.
How about adding a bit for the possible actions of interest.
And then PMDs can set bits for actions which can be persistent across
stop, start and reconfigurations?

>
> If the device is being reconfigured in a way that is incompatible with
> an existing indirect action, PMD is required to report an error.
> 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. For example, if queue count changes and RSS
> indirect action specifies queues that are going away, the user must
> update the action before removing the queues or remove the action and
> all flow rules that were using it.
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 12 ++++++++++++
>  lib/ethdev/rte_ethdev.h            |  5 +++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 0a03097a7c..da90b52f48 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2794,6 +2794,18 @@ 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()``.
>
> +By default indirect actions are destroyed when the device is stopped.
> +If the device advertises ``RTE_ETH_DEV_CAPA_FLOW_INDIRECT_ACTION_KEEP``,
> +indirect actions persist across the device stop and start with possible
> +reconfiguration in between. Some configuration changes may be incompatible
> +with existing indirect actions, in this case ``rte_eth_dev_configure()`` and/or
> +``rte_eth_rx/tx_queue_setup()`` will fail. At this point PMD developers
> +are encouraged to log errors identical to the ones that would be emitted by
> +``rte_flow_action_handle_create()`` if the new configuration was active.
> +Even if this capability is advertised, there may be kinds of indirect actions
> +that the device cannot keep. They are implicitly destroyed at device stop.
> +PMD developers must document such kinds of actions if applicable.
> +
>  .. _table_rte_flow_action_handle:
>
>  .. table:: INDIRECT
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 1616bdf2dd..c3be5afcb2 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1450,6 +1450,11 @@ struct rte_eth_conf {
>  #define RTE_ETH_DEV_CAPA_RUNTIME_TX_QUEUE_SETUP 0x00000002
>  /** Device keeps flow rules across restart and reconfiguration. */
>  #define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP 0x00000004
> +/**
> + * Device keeps indirect actions across restart and reconfiguration.
> + * For a specific PMD this may not be applicable to certain action types.
> + */
> +#define RTE_ETH_DEV_CAPA_FLOW_INDIRECT_ACTION_KEEP 0x00000008
>  /**@}*/
>
>  /*
> --
> 2.25.1
>