Re: [dpdk-dev] ***Spam*** [RFC] ethdev: support query of AGE action

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 9/9/20 12:25 PM, Dekel Peled wrote:
> Existing API supports AGE action to monitor the aging of a single flow.
> 
> Recent patch [1] introduced the shared action API.
> Using this API, an action can be created as shared, unattached to any
> flow rule.
> Multiple flows can then be created using the shared action.
> The new API also supports query operation of a shared action.

Full story is nice, but IMHO shared actions are out of scope of
the patch since AGE action query makes sense without shared
actions as well. There is no point to over-complicate the description.

> 
> This RFC proposes the response format for query of a shared AGE action.
> This response format can be also used for existing flow query operation.
> 
> Following this RFC, the MLX5 PMD will implement support of shared AGE
> action.
> This new feature will utilize the Flow Hit ASO Object mechanism.
> A Flow Hit ASO Object contains 512 flags, each flag can be assigned to a
> specific shared AGE action.
> Traffic hit on any flow which uses the shared AGE action will set the
> flag assigned to this action.
> MLX5 PMD will monitor the state of all allocated Flow Hit flags.
> The duration of time with no flow hit will be aggregated until aging
> timeout expires.
> Application will use the shared action query operation to find the
> AGE action state.
> The response will be returned in the format proposed here.

I think mlx5 details are OK for RFC, but should be in mlx5
patch finally (not ethdev patch).

> 
> [1] https://mails.dpdk.org/archives/dev/2020-July/174110.html
> 
> Signed-off-by: Dekel Peled <dekelp@nvidia.com>
> ---
>  lib/librte_ethdev/rte_flow.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index 7b9f82b..5f7235c 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -2064,6 +2064,7 @@ enum rte_flow_action_type {
>  	 * See struct rte_flow_action_age.
>  	 * See function rte_flow_get_aged_flows
>  	 * see enum RTE_ETH_EVENT_FLOW_AGED
> +	 * See struct rte_flow_query_age
>  	 */
>  	RTE_FLOW_ACTION_TYPE_AGE,
>  
> @@ -2181,6 +2182,19 @@ struct rte_flow_query_count {
>  };
>  
>  /**
> + * RTE_FLOW_ACTION_TYPE_AGE (query)
> + *
> + * Query structure to retrieve the aging status information of a
> + * shared AGE action, or a flow rule using the AGE action.
> + */
> +struct rte_flow_query_age {
> +	uint32_t aged:1; /**< 1 if aging timeout expired, 0 otherwise. */
> +	uint32_t last_hit_time_valid:1; /**< 1 if last_hit_time value valid. */
> +	uint32_t reserved:6; /**< Reserved, must be zero. */
> +	uint32_t last_hit_time:24; /**< Seconds since last traffic hit. */

How is the field width chosen? 24 bits in seconds is 194 days
which is more than half of a year. Just to match action AGE
width? (if so, a bit late questions to AGE action API).

> +};
> +
> +/**
>   * Hash function types.
>   */
>  enum rte_eth_hash_function {
> 

May be struct rte_flow_action_age description should be updated
to mention that it is possible to query (similar to COUNT
action).

In general, LGTM.