Re: [dpdk-dev] [PATCH v2] [RFC] ethdev: support flow aging

[Jerin Jacob <jerinjacobk@gmail.com>]

On Mon, Mar 16, 2020 at 6:22 PM BillZhou <dongz@mellanox.com> wrote:
>
> One of the reasons to destroy a flow is the fact that no packet matches the
> flow for "timeout" time.
> For example, when TCP\UDP sessions are suddenly closed.
>
> Currently, there is no any dpdk mechanism for flow aging and the
> applications use there own ways to detect and destroy aged-out flows.
>
> This RFC introduces flow aging APIs to offload the flow aging task from
> the application to the port.
>
> Design:
> - A new rte_flow action: RTE_FLOW_ACTION_TYPE_AGE to set the timeout and
>   the application flow context for each flow.
> - A new ethdev event: RTE_ETH_EVENT_FLOW_AGED for the driver to report
>   that there are new aged-out flows.
> - A new rte_flow API: rte_flow_get_aged_flows to get the aged-out flows
>   contexts from the port.
>
> By this design each PMD can use its best way to do the aging with the
> device offloads supported by its HW.
>
> Signed-off-by: BillZhou <dongz@mellanox.com>
> ---
> v2:For API rte_flow_get_aged_flows, delete "struct rte_flow *flows[]"
> this parameter.
> ---
>  lib/librte_ethdev/rte_ethdev.h |  1 +
>  lib/librte_ethdev/rte_flow.h   | 56 ++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index d1a593ad11..03135a7138 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3015,6 +3015,7 @@ enum rte_eth_event_type {
>         RTE_ETH_EVENT_NEW,      /**< port is probed */
>         RTE_ETH_EVENT_DESTROY,  /**< port is released */
>         RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
> +       RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows detected in the port */
>         RTE_ETH_EVENT_MAX       /**< max value of this enum */
>  };
>
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 5625dc4917..1fc05bf56c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -2051,6 +2051,14 @@ enum rte_flow_action_type {
>          * See struct rte_flow_action_set_dscp.
>          */
>         RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP,
> +
> +       /**
> +        * Report as aged-out if timeout passed without any matching on the
> +        * flow.
> +        *
> +        * See struct rte_flow_action_age.
> +        */
> +       RTE_FLOW_ACTION_TYPE_AGE,
>  };
>
>  /**
> @@ -2633,6 +2641,22 @@ struct rte_flow_action {
>         const void *conf; /**< Pointer to action configuration object. */
>  };
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_AGE
> + *
> + * Report as aged-out if timeout passed without any matching on the flow.
> + *
> + * The flow context and the flow handle will be reported by the
> + * rte_flow_get_aged_flows API.
> + */
> +struct rte_flow_action_age {
> +       uint16_t timeout; /**< Time in seconds. */
> +       void *context; /**< The user flow context. */
> +};
> +
>  /**
>   * Opaque type returned after successfully creating a flow.
>   *
> @@ -3224,6 +3248,38 @@ rte_flow_conv(enum rte_flow_conv_op op,
>               const void *src,
>               struct rte_flow_error *error);
>
> +/**
> + * Get aged-out flows of a given port.
> + *
> + * RTE_ETH_EVENT_FLOW_AGED is triggered when a port detects aged-out flows.
> + * This function can be called to get the aged flows usynchronously from the

s/usynchronously/ asynchronously


> + * event callback or synchronously when the user wants it.
> + * The callback synchronization is on the user responsibility.
> + *
> + * @param port_id
> + *   Port identifier of Ethernet device.
> + * @param[in/out] contexts
> + *   An allocated array to get the aged-out flows contexts from input age
> + *   action config, if input contexts is null, return the aged-out flows.
> + *   NULL indicates the flow contexts should not be reported.
> + * @param[in] nb_context

By default, everything is [in]. Not need to mention [in] explicitly.

> + *   The allocated array entries number of @p contexts if exist.
> + * @param[out] error
> + *   Perform verbose error reporting if not NULL. Initialized in case of
> + *   error only.
> + *
> + * @return
> + *   0 in case there are not any aged-out contexts or flows, otherwise if
> + *   positive is the number of the reported aged-out contexts or flows to
> + *   @p contexts, a negative errno value otherwise and rte_errno is set.
> + *
> + * @see rte_flow_action_age

RTE_ETH_EVENT_FLOW_AGED can be added in @see

Other than the above nits,

This RFC looks good to me.

> + */
> +__rte_experimental
> +int
> +rte_flow_get_aged_flows(uint16_t port_id, void *contexts[],
> +                       int nb_context, struct rte_flow_error *error);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.21.0
>