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

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 3/16/20 3:52 PM, BillZhou 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>

LGTM

> ---
> 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. */

Is it intentionally defined small? May be it is better to
use uint32_t? I just want to understand the rational behind
the type choice.

> +	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
> + * 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
> + *   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_experimental
> +int
> +rte_flow_get_aged_flows(uint16_t port_id, void *contexts[],
> +			int nb_context, struct rte_flow_error *error);
> +
>  #ifdef __cplusplus
>  }
>  #endif
>