[PATCH v4] ethdev: add special flags when creating async transfer table

[Andrew Rybchenko]

On 11/4/22 13:44, Rongwei Liu wrote:
> The transfer domain rule is able to match traffic wire/vport
> origin which are corresponding to two kinds of underlayer resources.
> 
> Wire means traffic arrives from the uplink port while vport means
> traffic initiated from VF/SF.
> 
> In customer deployments, they usually match only one kind of
> traffic in single flow table: either from wire or from vport.
> PMD can save significant resources if passing special hint from rte
> layer.
> 
> There are two possible approaches, using IPv4 as an example:
> 1. Use pattern item.
>     pattern_template: pattern ANY_VPORT / eth / ipv4 is 1.1.1.1 / end
>     async flow create: pattern ANY_VPORT / eth / ipv4 is 1.1.1.2 / end
>     "ANY_VPORT" needs to be present in each async rule even if it's
>     just a hint. No value to match.
> 
> 2. Add special flags into table_attr. It will be:
>     template_table 0 create table_id 0 group 1 transfer vf_orig
> 
> Approach 1 needs to specify the pattern in each flow rules which wastes
> memory and not end user friendly.
> This patch takes the 2nd approach and introduce one new member
> specialize into rte_flow_table_attr to indicate async flow table matching
> optimization: from wire, from vport.
> 
> It helps to save underlayer memory and also on insertion rate.
> 
> By default, there is no hint, so the behavior of the transfer domain
> doesn't change.
> 
> 1. Match wire origin only
>     flow template_table 0 create group 0 priority 0 transfer wire_orig...
> 2. Match vf origin only
>     flow template_table 0 create group 0 priority 0 transfer vport_orig...
> 
> Signed-off-by: Rongwei Liu <rongweil at nvidia.com>
> Acked-by: Ori Kam <orika at nvidia.com>
> 
> v2: Move the new field to template table attribute.
> v4: Mark it as optional and clear the concept.
> ---
>   app/test-pmd/cmdline_flow.c                 | 26 +++++++++++++++++
>   doc/guides/prog_guide/rte_flow.rst          | 15 ++++++++++
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +-
>   lib/ethdev/rte_flow.h                       | 32 +++++++++++++++++++++
>   4 files changed, 75 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 88108498e0..15f2af9b40 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -184,6 +184,8 @@ enum index {
>   	TABLE_INGRESS,
>   	TABLE_EGRESS,
>   	TABLE_TRANSFER,
> +	TABLE_TRANSFER_WIRE_ORIG,
> +	TABLE_TRANSFER_VPORT_ORIG,
>   	TABLE_RULES_NUMBER,
>   	TABLE_PATTERN_TEMPLATE,
>   	TABLE_ACTIONS_TEMPLATE,
> @@ -1158,6 +1160,8 @@ static const enum index next_table_attr[] = {
>   	TABLE_INGRESS,
>   	TABLE_EGRESS,
>   	TABLE_TRANSFER,
> +	TABLE_TRANSFER_WIRE_ORIG,
> +	TABLE_TRANSFER_VPORT_ORIG,
>   	TABLE_RULES_NUMBER,
>   	TABLE_PATTERN_TEMPLATE,
>   	TABLE_ACTIONS_TEMPLATE,
> @@ -2933,6 +2937,18 @@ static const struct token token_list[] = {
>   		.next = NEXT(next_table_attr),
>   		.call = parse_table,
>   	},
> +	[TABLE_TRANSFER_WIRE_ORIG] = {
> +		.name = "wire_orig",
> +		.help = "affect rule direction to transfer",
> +		.next = NEXT(next_table_attr),
> +		.call = parse_table,
> +	},
> +	[TABLE_TRANSFER_VPORT_ORIG] = {
> +		.name = "vport_orig",
> +		.help = "affect rule direction to transfer",
> +		.next = NEXT(next_table_attr),
> +		.call = parse_table,
> +	},
>   	[TABLE_RULES_NUMBER] = {
>   		.name = "rules_number",
>   		.help = "number of rules in table",
> @@ -8993,6 +9009,16 @@ parse_table(struct context *ctx, const struct token *token,
>   	case TABLE_TRANSFER:
>   		out->args.table.attr.flow_attr.transfer = 1;
>   		return len;
> +	case TABLE_TRANSFER_WIRE_ORIG:
> +		if (!out->args.table.attr.flow_attr.transfer)
> +			return -1;
> +		out->args.table.attr.specialize = RTE_FLOW_TRANSFER_WIRE_ORIG;
> +		return len;
> +	case TABLE_TRANSFER_VPORT_ORIG:
> +		if (!out->args.table.attr.flow_attr.transfer)
> +			return -1;
> +		out->args.table.attr.specialize = RTE_FLOW_TRANSFER_VPORT_ORIG;
> +		return len;
>   	default:
>   		return -1;
>   	}
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 3e6242803d..d9ca041ae4 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -3605,6 +3605,21 @@ and pattern and actions templates are created.
>                      &actions_templates, nb_actions_templ,
>                      &error);
>   
> +Table Attribute: Specialize
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Application can help optimizing underlayer resources and insertion rate
> +by specializing template table.
> +Specialization is done by providing hints
> +in the template table attribute ``specialize``.
> +
> +This attribute is not mandatory for each PMD to implement.
> +If a hint is not supported, it will be silently ignored,
> +and no special optimization is done.
> +
> +If a table is specialized, the application should make sure the rules
> +comply with the table attribute.
> +
>   Asynchronous operations
>   -----------------------
>   
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 96c5ae0fe4..b3238415f4 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -3145,7 +3145,8 @@ It is bound to ``rte_flow_template_table_create()``::
>   
>      flow template_table {port_id} create
>          [table_id {id}] [group {group_id}]
> -       [priority {level}] [ingress] [egress] [transfer]
> +       [priority {level}] [ingress] [egress]
> +       [transfer [vport_orig] [wire_orig]]
>          rules_number {number}
>          pattern_template {pattern_template_id}
>          actions_template {actions_template_id}
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 8858b56428..1eab12796f 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -5186,6 +5186,34 @@ rte_flow_actions_template_destroy(uint16_t port_id,
>    */
>   struct rte_flow_template_table;
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Special optional flags for template table attribute.
> + * Each bit stands for a table specialization
> + * offering a potential optimization at PMD layer.
> + * PMD can ignore the unsupported bits silently.
> + */
> +enum rte_flow_template_table_specialize {
> +	/**
> +	 * Specialize table for transfer flows which come only from wire.
> +	 * It allows PMD not to allocate resources for non-wire originated traffic.
> +	 * This bit is not a matching criteria, just an optimization hint.
> +	 * Flow rules which match non-wire originated traffic will be missed
> +	 * if the hint is supported.
> +	 */
> +	RTE_FLOW_TRANSFER_WIRE_ORIG = RTE_BIT32(0),
> +	/**
> +	 * Specialize table for transfer flows which come only from vport (e.g. VF, SF).
> +	 * It allows PMD not to allocate resources for non-vport originated traffic.
> +	 * This bit is not a matching criteria, just an optimization hint.
> +	 * Flow rules which match non-vport originated traffic will be missed
> +	 * if the hint is supported.
> +	 */
> +	RTE_FLOW_TRANSFER_VPORT_ORIG = RTE_BIT32(1),
> +};
> +
>   /**
>    * @warning
>    * @b EXPERIMENTAL: this API may change without prior notice.
> @@ -5201,6 +5229,10 @@ struct rte_flow_template_table_attr {
>   	 * Maximum number of flow rules that this table holds.
>   	 */
>   	uint32_t nb_flows;
> +	/**
> +	 * Optional hint flags for PMD optimization.
> +	 */
> +	enum rte_flow_template_table_specialize specialize;


IMHO it is not 100% correct to use enum for flag since
RTE_FLOW_TRANSFER_WIRE_ORIG | RTE_FLOW_TRANSFER_VPORT_ORIG
is not the enum member. uint32_t is a better option here since
bits are defined as RTE_BIT32. enum should be mentioned in the
description.

>   };
>   
>   /**