[v7] ethdev: add special flags when creating async transfer table
Checks
Commit Message
In case flow rules match only one kind of traffic in a flow table,
then optimization can be done via allocation of this table.
Such optimization is possible only if the application gives a hint
about its usage of the table during initial configuration.
The transfer domain rules may process traffic from wire or vport,
which may correspond to two kinds of underlayer resources.
That's why the first two hints introduced in this patch are about
wire and vport traffic specialization.
Wire means traffic arrives from the uplink port while vport means
traffic initiated from VF/SF.
There are two possible approaches for providing the hints.
Using IPv4 as an example:
1. Use pattern item in both template table and flow rules.
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 flow rule even if it's
just a hint. No value to match because matching is already done by
IPv4 item.
2. Add special flags into table_attr.
template_table 0 create table_id 0 group 1 transfer vport_orig
Approach 1 needs to specify the pattern in each flow rule which wastes
memory and is not user friendly.
This patch takes the 2nd approach and introduces one new member
"specialize" into rte_flow_table_attr to indicate possible flow table
optimization.
By default, there is no hint, so the behavior of the transfer domain
doesn't change.
There is no guarantee that the hint will be used by the PMD.
Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
v2: Move the new field to template table attribute.
v4: Mark it as optional and clear the concept.
v5: Change specialize type to uint32_t.
v6: Change the flags to macros and re-construct the commit log.
v7: Fix build failure.
---
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 | 28 +++++++++++++++++++++
4 files changed, 71 insertions(+), 1 deletion(-)
Comments
On 11/14/2022 11:59 AM, Rongwei Liu wrote:
> In case flow rules match only one kind of traffic in a flow table,
> then optimization can be done via allocation of this table.
> Such optimization is possible only if the application gives a hint
> about its usage of the table during initial configuration.
>
> The transfer domain rules may process traffic from wire or vport,
> which may correspond to two kinds of underlayer resources.
> That's why the first two hints introduced in this patch are about
> wire and vport traffic specialization.
> Wire means traffic arrives from the uplink port while vport means
> traffic initiated from VF/SF.
>
> There are two possible approaches for providing the hints.
> Using IPv4 as an example:
> 1. Use pattern item in both template table and flow rules.
>
> 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 flow rule even if it's
> just a hint. No value to match because matching is already done by
> IPv4 item.
>
> 2. Add special flags into table_attr.
>
> template_table 0 create table_id 0 group 1 transfer vport_orig
>
> Approach 1 needs to specify the pattern in each flow rule which wastes
> memory and is not user friendly.
> This patch takes the 2nd approach and introduces one new member
> "specialize" into rte_flow_table_attr to indicate possible flow table
> optimization.
>
> By default, there is no hint, so the behavior of the transfer domain
> doesn't change.
> There is no guarantee that the hint will be used by the PMD.
>
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
Hi Andrew, Ivan,
Do you have objection/comment to latest version, if not I will proceed
with patch?
Thanks,
ferruh
On 1/17/2023 3:13 PM, Ferruh Yigit wrote:
> On 11/14/2022 11:59 AM, Rongwei Liu wrote:
>> In case flow rules match only one kind of traffic in a flow table,
>> then optimization can be done via allocation of this table.
>> Such optimization is possible only if the application gives a hint
>> about its usage of the table during initial configuration.
>>
>> The transfer domain rules may process traffic from wire or vport,
>> which may correspond to two kinds of underlayer resources.
>> That's why the first two hints introduced in this patch are about
>> wire and vport traffic specialization.
>> Wire means traffic arrives from the uplink port while vport means
>> traffic initiated from VF/SF.
>>
>> There are two possible approaches for providing the hints.
>> Using IPv4 as an example:
>> 1. Use pattern item in both template table and flow rules.
>>
>> 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 flow rule even if it's
>> just a hint. No value to match because matching is already done by
>> IPv4 item.
>>
>> 2. Add special flags into table_attr.
>>
>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>
>> Approach 1 needs to specify the pattern in each flow rule which wastes
>> memory and is not user friendly.
>> This patch takes the 2nd approach and introduces one new member
>> "specialize" into rte_flow_table_attr to indicate possible flow table
>> optimization.
>>
>> By default, there is no hint, so the behavior of the transfer domain
>> doesn't change.
>> There is no guarantee that the hint will be used by the PMD.
>>
>> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
>> Acked-by: Ori Kam <orika@nvidia.com>
>
> Hi Andrew, Ivan,
>
> Do you have objection/comment to latest version, if not I will proceed
> with patch?
>
BTW, there is an implementation of this flag in some driver, right?
HI Ferruh:
BR
Rongwei
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Wednesday, January 18, 2023 01:02
> To: Rongwei Liu <rongweil@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>;
> Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Ivan Malov <ivan.malov@oktetlabs.ru>;
> NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> Subject: Re: [PATCH v7] ethdev: add special flags when creating async transfer
> table
>
> External email: Use caution opening links or attachments
>
>
> On 1/17/2023 3:13 PM, Ferruh Yigit wrote:
> > On 11/14/2022 11:59 AM, Rongwei Liu wrote:
> >> In case flow rules match only one kind of traffic in a flow table,
> >> then optimization can be done via allocation of this table.
> >> Such optimization is possible only if the application gives a hint
> >> about its usage of the table during initial configuration.
> >>
> >> The transfer domain rules may process traffic from wire or vport,
> >> which may correspond to two kinds of underlayer resources.
> >> That's why the first two hints introduced in this patch are about
> >> wire and vport traffic specialization.
> >> Wire means traffic arrives from the uplink port while vport means
> >> traffic initiated from VF/SF.
> >>
> >> There are two possible approaches for providing the hints.
> >> Using IPv4 as an example:
> >> 1. Use pattern item in both template table and flow rules.
> >>
> >> 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 flow rule even if it's
> >> just a hint. No value to match because matching is already done by
> >> IPv4 item.
> >>
> >> 2. Add special flags into table_attr.
> >>
> >> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>
> >> Approach 1 needs to specify the pattern in each flow rule which
> >> wastes memory and is not user friendly.
> >> This patch takes the 2nd approach and introduces one new member
> >> "specialize" into rte_flow_table_attr to indicate possible flow table
> >> optimization.
> >>
> >> By default, there is no hint, so the behavior of the transfer domain
> >> doesn't change.
> >> There is no guarantee that the hint will be used by the PMD.
> >>
> >> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> >> Acked-by: Ori Kam <orika@nvidia.com>
> >
> > Hi Andrew, Ivan,
> >
> > Do you have objection/comment to latest version, if not I will proceed
> > with patch?
> >
>
> BTW, there is an implementation of this flag in some driver, right?
Yes, NVIDIA NIC has an implementation ready. Will pass the new RTE table attribute to PMD once API accepted.
On 11/14/22 14:59, Rongwei Liu wrote:
> In case flow rules match only one kind of traffic in a flow table,
> then optimization can be done via allocation of this table.
> Such optimization is possible only if the application gives a hint
> about its usage of the table during initial configuration.
>
> The transfer domain rules may process traffic from wire or vport,
> which may correspond to two kinds of underlayer resources.
> That's why the first two hints introduced in this patch are about
> wire and vport traffic specialization.
> Wire means traffic arrives from the uplink port while vport means
> traffic initiated from VF/SF.
>
> There are two possible approaches for providing the hints.
> Using IPv4 as an example:
> 1. Use pattern item in both template table and flow rules.
>
> 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 flow rule even if it's
> just a hint. No value to match because matching is already done by
> IPv4 item.
>
> 2. Add special flags into table_attr.
>
> template_table 0 create table_id 0 group 1 transfer vport_orig
>
> Approach 1 needs to specify the pattern in each flow rule which wastes
> memory and is not user friendly.
> This patch takes the 2nd approach and introduces one new member
> "specialize" into rte_flow_table_attr to indicate possible flow table
> optimization.
The above description is misleading. It alternates options (1)
and (2), but in fact (2) requires (1) as well.
(2) is simply done on different level - much earlier, before
flow rules creation. Since resources allocation is assumed to
be done on table creation, we need to know the purpose of the
table in advance to optimize resources allocation.
Since (2) is *not a matching criteria*, but just a hint, (1)
flow rules must have matching criteria anyway.
>
> By default, there is no hint, so the behavior of the transfer domain
> doesn't change.
> There is no guarantee that the hint will be used by the PMD.
>
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
[snip]
> 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.
If a table is specialized, the application must make sure that
all flow rules added to the table have pattern which implies
corresponding matching criteria. For example if a table is
specialized to be wire-origin only, pattern should have
represented port item with ethdev which corresponds to a
physical port (or any other item which matches packets
coming from wire only).
[snip]
On 1/17/23 18:13, Ferruh Yigit wrote:
> On 11/14/2022 11:59 AM, Rongwei Liu wrote:
>> In case flow rules match only one kind of traffic in a flow table,
>> then optimization can be done via allocation of this table.
>> Such optimization is possible only if the application gives a hint
>> about its usage of the table during initial configuration.
>>
>> The transfer domain rules may process traffic from wire or vport,
>> which may correspond to two kinds of underlayer resources.
>> That's why the first two hints introduced in this patch are about
>> wire and vport traffic specialization.
>> Wire means traffic arrives from the uplink port while vport means
>> traffic initiated from VF/SF.
>>
>> There are two possible approaches for providing the hints.
>> Using IPv4 as an example:
>> 1. Use pattern item in both template table and flow rules.
>>
>> 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 flow rule even if it's
>> just a hint. No value to match because matching is already done by
>> IPv4 item.
>>
>> 2. Add special flags into table_attr.
>>
>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>
>> Approach 1 needs to specify the pattern in each flow rule which wastes
>> memory and is not user friendly.
>> This patch takes the 2nd approach and introduces one new member
>> "specialize" into rte_flow_table_attr to indicate possible flow table
>> optimization.
>>
>> By default, there is no hint, so the behavior of the transfer domain
>> doesn't change.
>> There is no guarantee that the hint will be used by the PMD.
>>
>> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
>> Acked-by: Ori Kam <orika@nvidia.com>
>
> Hi Andrew, Ivan,
>
> Do you have objection/comment to latest version, if not I will proceed
> with patch?
>
> Thanks,
> ferruh
Hi Ferruh,
Sorry, but I'm still unhappy with the description.
See my reply.
Thanks,
Andrew.
18/01/2023 08:28, Andrew Rybchenko:
> On 11/14/22 14:59, Rongwei Liu wrote:
> > In case flow rules match only one kind of traffic in a flow table,
> > then optimization can be done via allocation of this table.
> > Such optimization is possible only if the application gives a hint
> > about its usage of the table during initial configuration.
> >
> > The transfer domain rules may process traffic from wire or vport,
> > which may correspond to two kinds of underlayer resources.
> > That's why the first two hints introduced in this patch are about
> > wire and vport traffic specialization.
> > Wire means traffic arrives from the uplink port while vport means
> > traffic initiated from VF/SF.
> >
> > There are two possible approaches for providing the hints.
> > Using IPv4 as an example:
> > 1. Use pattern item in both template table and flow rules.
> >
> > 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 flow rule even if it's
> > just a hint. No value to match because matching is already done by
> > IPv4 item.
> >
> > 2. Add special flags into table_attr.
> >
> > template_table 0 create table_id 0 group 1 transfer vport_orig
> >
> > Approach 1 needs to specify the pattern in each flow rule which wastes
> > memory and is not user friendly.
> > This patch takes the 2nd approach and introduces one new member
> > "specialize" into rte_flow_table_attr to indicate possible flow table
> > optimization.
>
> The above description is misleading. It alternates options (1)
> and (2), but in fact (2) requires (1) as well.
Yes the above description may be misleading
and it seems you are misleaded :)
I will explain below why the option (2) doesn't require (1).
I think we should apply the same example to both cases to make it clear:
1. Use pattern item in both template table and flow rules:
template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
The pattern template 3 will be used only to match flows coming from vports.
ANY_VPORT needs to be present in each flow rule.
ANY_VPORT matching is redundant with IP src 1.1.1.1 because
the user knows 1.1.1.1 is the IP of a vport.
2. Add specialization flag into template table attribute:
template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is 255.255.255.255 / end
flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
The pattern template 3 can be used only to match flows coming from vports.
> (2) is simply done on different level - much earlier, before
> flow rules creation. Since resources allocation is assumed to
> be done on table creation, we need to know the purpose of the
> table in advance to optimize resources allocation.
Actually in both cases we get the hint at template table creation.
But in solution 2 we are not creating a redundant pattern matching,
and we don't need to check it in flow rules, so it is more efficient.
> Since (2) is *not a matching criteria*, but just a hint, (1)
> flow rules must have matching criteria anyway.
No we don't need the matching criteria ANY_VPORT with solution (2)
because we are already matching on an IP src which is a vport.
> > +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.
>
> If a table is specialized, the application must make sure that
> all flow rules added to the table have pattern which implies
> corresponding matching criteria. For example if a table is
> specialized to be wire-origin only, pattern should have
> represented port item with ethdev which corresponds to a
> physical port (or any other item which matches packets
> coming from wire only).
No need of a matching criteria strictly mapping the hint.
Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
and the rules can match on an IP src which is assigned to a vport.
So there is no need to strictly match the vport itself in the rule.
Hope it make thinks clear.
We can improve the commit log as I wrote above.
Hi Rongwei,
Thanks for persevering. I have no strong opinion, but, at least, the
fact that the new flags are no longer meant for use in rte_flow_attr,
which is clearly not the right place for such, is an improvement.
However, let's take a closer look at the current patch, shall we?
But, before we get to that, I'd like to kindly request that you
provide a more concrete example of how this feature is supposed
to be used. Are there some real-life application examples?
Also, to me, it's still unclear how an application can obtain
the knowledge of this hint in the first instance. For example,
can Open vSwitch somehow tell ethdevs representing physical
ports from ones representing "vports" (host endpoints)?
How does it know which attribute to specify?
For the rest of my notes, PSB.
On Mon, 14 Nov 2022, Rongwei Liu wrote:
> In case flow rules match only one kind of traffic in a flow table,
> then optimization can be done via allocation of this table.
This wording might confuse readers. Consider rephrasing it, please:
If multiple flow rules share a common set of match masks, then
they might belong in a flow table which can be pre-allocated.
> Such optimization is possible only if the application gives a hint
> about its usage of the table during initial configuration.
>
> The transfer domain rules may process traffic from wire or vport,
> which may correspond to two kinds of underlayer resources.
Why name it a "vport"? Why not "host"?
host = packets generated by any of the host's "vport"s
wire = packets arriving at the NIC from the network
> That's why the first two hints introduced in this patch are about
> wire and vport traffic specialization.
> Wire means traffic arrives from the uplink port while vport means
> traffic initiated from VF/SF.
By the sound of it, the meaning is confined to just VFs/SFs.
What if the user wants to match packets coming from PFs?
>
> There are two possible approaches for providing the hints.
> Using IPv4 as an example:
> 1. Use pattern item in both template table and flow rules.
>
> 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 flow rule even if it's
> just a hint. No value to match because matching is already done by
> IPv4 item.
Why no value to match on? How does it prevent rogue tenants
from spoofing network headers? If the application receives
a packet on a particular vport's representor, then it may
strictly specify item represented_port pointing to that
vport so that only packets from that vport match.
Why isn't security a consideration?
>
> 2. Add special flags into table_attr.
>
> template_table 0 create table_id 0 group 1 transfer vport_orig
>
> Approach 1 needs to specify the pattern in each flow rule which wastes
> memory and is not user friendly.
What if the user has to insert a group of rules which not only
have the same set of match masks but also share exactly the
same match spec values for a limited subset of network
items (for example, those of an encap. header)? This
way, a subset of network item specs can remain fixed
across many rules. Does that count as wasting memory?
If yes, then the problem does not concern just a single pair
of attributes, but rather deserves a more versatile solution
like some sort of indirect grouping of constant item specs.
Have you considered such options?
> This patch takes the 2nd approach and introduces one new member
> "specialize" into rte_flow_table_attr to indicate possible flow table
> optimization.
The name "specialize" might have some drawbacks:
- spelling difference (specialise/specialize)
- in grep output, will mix with flows' "spec"
- quite long
- not a noun
Why not "scope"? Or something like that?
>
> By default, there is no hint, so the behavior of the transfer domain
> doesn't change.
> There is no guarantee that the hint will be used by the PMD.
>
> 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.
> v5: Change specialize type to uint32_t.
> v6: Change the flags to macros and re-construct the commit log.
> v7: Fix build failure.
> ---
> 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 | 28 +++++++++++++++++++++
> 4 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
Silently ignoring the field does not sit well with the
application's possible intent to drop represented_port
match from the patterns. From my point of view, if the
application sets this attribute, it believes it can
rely on it, that is, packets coming from host won't
match if the attribute asks to match network
only, for instance. Has this been considered?
> +
> +If a table is specialized, the application should make sure the rules
> +comply with the table attribute.
How does the application enforce that? I would appreciate you explain it.
> +
> 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..c27b48c5c1 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -5186,6 +5186,29 @@ rte_flow_actions_template_destroy(uint16_t port_id,
> */
> struct rte_flow_template_table;
>
> +/**@{@name Special optional flags for template table attribute
> + * Each bit is a hint for table specialization,
> + * offering a potential optimization at PMD layer.
> + * PMD can ignore the unsupported bits silently.
> + */
> +/**
> + * 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.
You intended to spell "criterion", I take it. And still, it *is*
a match criterion. I'm not denying the possible need to have
this criterion at the earliest processing stage. That might
be OK, but I still have a hunch that this is too specific.
Please see my comment above about wasting memory.
I guess this type of criterion is not the only
one that may need to be provided as a "hint".
> + * Flow rules which match non-wire originated traffic will be missed
> + * if the hint is supported.
And what if it's unsupported? Is it indeed OK to silently ignore it?
> + */
> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG RTE_BIT32(0)
Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
To me, TRANSFER looks redundant as this bit is already supposed
to be ticked in the "struct rte_flow_attr flow_attr" field
of the "struct rte_flow_template_table_attr".
> +/**
> + * Specialize table for transfer flows which come only from vport (e.g. VF,
> SF).
And PF?
> + * 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.
> + */
> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG RTE_BIT32(1)
Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
> +/**@}*/
> +
> /**
> * @warning
> * @b EXPERIMENTAL: this API may change without prior notice.
> @@ -5201,6 +5224,11 @@ 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.
> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
> + */
> + uint32_t specialize;
Why not "scope" or something?
> };
>
> /**
> --
> 2.27.0
>
Thank you.
Hi Ivan,
BR
Rongwei
> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Monday, January 30, 2023 08:00
> To: Rongwei Liu <rongweil@nvidia.com>
> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: Re: [PATCH v7] ethdev: add special flags when creating async transfer
> table
>
> External email: Use caution opening links or attachments
>
>
> Hi Rongwei,
>
> Thanks for persevering. I have no strong opinion, but, at least, the fact that the
> new flags are no longer meant for use in rte_flow_attr, which is clearly not
> the right place for such, is an improvement.
>
Thanks for the suggestion, move it to rte_flow_table_attr now and it' dedicated to async API.
> However, let's take a closer look at the current patch, shall we?
>
> But, before we get to that, I'd like to kindly request that you provide a more
> concrete example of how this feature is supposed to be used. Are there some
> real-life application examples?
>
Sure.
> Also, to me, it's still unclear how an application can obtain the knowledge of
> this hint in the first instance. For example, can Open vSwitch somehow tell
> ethdevs representing physical ports from ones representing "vports" (host
> endpoints)?
> How does it know which attribute to specify?
>
Hint should be initiated by application and application knows it' traffic pattern which highly relates to deployment.
Let' use VxLAN encap/decap as an example:
1. Traffic from wire should be VxLAN pattern and do the decap, then send to different vports.
flow pattern_template 0 create transfer relaxed no pattern_template_id 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp / vxlan / tag index is 0 data is 0x33 / end
flow actions_template 0 create transfer actions_template_id 4 template raw_decap index 0 / represented_port ethdev_port_id 1 / end mask raw_decap index 0 / represented_port ethdev_port_id 1 / end
flow template_table 0 create group 1 priority 0 transfer wire_orig table_id 4 rules_number 128 pattern_template 4 actions_template 4
2. Traffic from vports should be encap with different VxLAN header and send to wire.
flow actions_template 1 create transfer actions_template_id 5 template raw_encap index 0 / represented_port ethdev_port_id 0 / end mask raw_encap index 0 / represented_port ethdev_port_id 0 / end
flow template_table 0 create group 1 priority 0 transfer vport_orig table_id 5 rules_number 128 pattern_template 4 actions_template 5
> For the rest of my notes, PSB.
>
> On Mon, 14 Nov 2022, Rongwei Liu wrote:
>
> > In case flow rules match only one kind of traffic in a flow table,
> > then optimization can be done via allocation of this table.
>
> This wording might confuse readers. Consider rephrasing it, please:
> If multiple flow rules share a common set of match masks, then they might
> belong in a flow table which can be pre-allocated.
>
> > Such optimization is possible only if the application gives a hint
> > about its usage of the table during initial configuration.
> >
> > The transfer domain rules may process traffic from wire or vport,
> > which may correspond to two kinds of underlayer resources.
>
> Why name it a "vport"? Why not "host"?
>
> host = packets generated by any of the host's "vport"s wire = packets arriving
> at the NIC from the network
Vport is "virtual port" for short and contains "VF/SF" for now.
Per my thoughts, it' clearer and maps to DPDK port probing/management.
>
> > That's why the first two hints introduced in this patch are about wire
> > and vport traffic specialization.
> > Wire means traffic arrives from the uplink port while vport means
> > traffic initiated from VF/SF.
>
> By the sound of it, the meaning is confined to just VFs/SFs.
> What if the user wants to match packets coming from PFs?
>
It should be "wire_orig".
> >
> > There are two possible approaches for providing the hints.
> > Using IPv4 as an example:
> > 1. Use pattern item in both template table and flow rules.
> >
> > 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 flow rule even if it's just
> > a hint. No value to match because matching is already done by
> > IPv4 item.
>
> Why no value to match on? How does it prevent rogue tenants from spoofing
> network headers? If the application receives a packet on a particular vport's
> representor, then it may strictly specify item represented_port pointing to that
> vport so that only packets from that vport match.
>
> Why isn't security a consideration?
>
There is some misunderstanding here. "ANY_VPORT" is the approach (new matching item without value) suggested by you.
I was explaining we need to apply it to each flow rule even if it's only a flag and no value.
> >
> > 2. Add special flags into table_attr.
> >
> > template_table 0 create table_id 0 group 1 transfer vport_orig
> >
> > Approach 1 needs to specify the pattern in each flow rule which wastes
> > memory and is not user friendly.
>
> What if the user has to insert a group of rules which not only have the same
> set of match masks but also share exactly the same match spec values for a
> limited subset of network items (for example, those of an encap. header)? This
> way, a subset of network item specs can remain fixed across many rules. Does
> that count as wasting memory?
>
Per my understanding, you are talking "multiple spec and mask mixing".
We provide a hint in this patch and no assumption on the matching patterns.
I think matching pattern is totally controlled by application layer.
"wasting memory " because your approach needs to scatter in each rule while this patch only needs to set table_attr once.
No relation with matching patter totally.
> If yes, then the problem does not concern just a single pair of attributes, but
> rather deserves a more versatile solution like some sort of indirect grouping of
> constant item specs.
> Have you considered such options?
See above.
>
> > This patch takes the 2nd approach and introduces one new member
> > "specialize" into rte_flow_table_attr to indicate possible flow table
> > optimization.
>
> The name "specialize" might have some drawbacks:
> - spelling difference (specialise/specialize)
> - in grep output, will mix with flows' "spec"
> - quite long
> - not a noun
>
> Why not "scope"? Or something like that?
>
It means special optimization to PMD. "scope" is more rogue.
> >
> > By default, there is no hint, so the behavior of the transfer domain
> > doesn't change.
> > There is no guarantee that the hint will be used by the PMD.
> >
> > 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.
> > v5: Change specialize type to uint32_t.
> > v6: Change the flags to macros and re-construct the commit log.
> > v7: Fix build failure.
> > ---
> > 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 | 28 +++++++++++++++++++++
> > 4 files changed, 71 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
>
> Silently ignoring the field does not sit well with the application's possible intent
> to drop represented_port match from the patterns. From my point of view, if
> the application sets this attribute, it believes it can rely on it, that is, packets
> coming from host won't match if the attribute asks to match network only, for
> instance. Has this been considered?
>
> > +
> > +If a table is specialized, the application should make sure the rules
> > +comply with the table attribute.
>
> How does the application enforce that? I would appreciate you explain it.
>
> > +
> > 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..c27b48c5c1 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -5186,6 +5186,29 @@ rte_flow_actions_template_destroy(uint16_t
> > port_id, */ struct rte_flow_template_table;
> >
> > +/**@{@name Special optional flags for template table attribute
> > + * Each bit is a hint for table specialization,
> > + * offering a potential optimization at PMD layer.
> > + * PMD can ignore the unsupported bits silently.
> > + */
> > +/**
> > + * 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.
>
> You intended to spell "criterion", I take it. And still, it *is* a match criterion.
> I'm not denying the possible need to have this criterion at the earliest
> processing stage. That might be OK, but I still have a hunch that this is too
> specific.
> Please see my comment above about wasting memory.
> I guess this type of criterion is not the only one that may need to be provided
> as a "hint".
>
> > + * Flow rules which match non-wire originated traffic will be missed
> > + * if the hint is supported.
>
> And what if it's unsupported? Is it indeed OK to silently ignore it?
>
> > + */
> > +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
> RTE_BIT32(0)
>
> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
>
> To me, TRANSFER looks redundant as this bit is already supposed to be ticked
> in the "struct rte_flow_attr flow_attr" field of the "struct
> rte_flow_template_table_attr".
>
> > +/**
> > + * Specialize table for transfer flows which come only from vport
> > +(e.g. VF,
> > SF).
>
> And PF?
>
> > + * 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.
> > + */
> > +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
> RTE_BIT32(1)
>
> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
>
> > +/**@}*/
> > +
> > /**
> > * @warning
> > * @b EXPERIMENTAL: this API may change without prior notice.
> > @@ -5201,6 +5224,11 @@ 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.
> > + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
> > + */
> > + uint32_t specialize;
>
> Why not "scope" or something?
>
> > };
> >
> > /**
> > --
> > 2.27.0
> >
>
> Thank you.
Hi Rongwei,
For my responses, PSB.
By the way, now you mention things like wasting memory and insertion
optimisastions, are there any comparative figures to see the effect
of this hint on insertion performance / memory footprint?
Some "before" / "after" examples would really be helpful.
After all, I'm not objecting this patch. But I believe that other
reviewers' concerns should nevertheless be addressed anyway.
On Mon, 30 Jan 2023, Rongwei Liu wrote:
> Hi Ivan,
>
> BR
> Rongwei
>
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@arknetworks.am>
>> Sent: Monday, January 30, 2023 08:00
>> To: Rongwei Liu <rongweil@nvidia.com>
>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
>> <rasland@nvidia.com>
>> Subject: Re: [PATCH v7] ethdev: add special flags when creating async transfer
>> table
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Rongwei,
>>
>> Thanks for persevering. I have no strong opinion, but, at least, the fact that the
>> new flags are no longer meant for use in rte_flow_attr, which is clearly not
>> the right place for such, is an improvement.
>>
> Thanks for the suggestion, move it to rte_flow_table_attr now and it' dedicated to async API.
>> However, let's take a closer look at the current patch, shall we?
>>
>> But, before we get to that, I'd like to kindly request that you provide a more
>> concrete example of how this feature is supposed to be used. Are there some
>> real-life application examples?
>>
> Sure.
>> Also, to me, it's still unclear how an application can obtain the knowledge of
>> this hint in the first instance. For example, can Open vSwitch somehow tell
>> ethdevs representing physical ports from ones representing "vports" (host
>> endpoints)?
>> How does it know which attribute to specify?
>>
> Hint should be initiated by application and application knows it' traffic pattern which highly relates to deployment.
> Let' use VxLAN encap/decap as an example:
> 1. Traffic from wire should be VxLAN pattern and do the decap, then send to different vports.
> flow pattern_template 0 create transfer relaxed no pattern_template_id 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp / vxlan / tag index is 0 data is 0x33 / end
> flow actions_template 0 create transfer actions_template_id 4 template raw_decap index 0 / represented_port ethdev_port_id 1 / end mask raw_decap index 0 / represented_port ethdev_port_id 1 / end
> flow template_table 0 create group 1 priority 0 transfer wire_orig table_id 4 rules_number 128 pattern_template 4 actions_template 4
>
> 2. Traffic from vports should be encap with different VxLAN header and send to wire.
> flow actions_template 1 create transfer actions_template_id 5 template raw_encap index 0 / represented_port ethdev_port_id 0 / end mask raw_encap index 0 / represented_port ethdev_port_id 0 / end
> flow template_table 0 create group 1 priority 0 transfer vport_orig table_id 5 rules_number 128 pattern_template 4 actions_template 5
>
>> For the rest of my notes, PSB.
>>
>> On Mon, 14 Nov 2022, Rongwei Liu wrote:
>>
>>> In case flow rules match only one kind of traffic in a flow table,
>>> then optimization can be done via allocation of this table.
>>
>> This wording might confuse readers. Consider rephrasing it, please:
>> If multiple flow rules share a common set of match masks, then they might
>> belong in a flow table which can be pre-allocated.
>>
>>> Such optimization is possible only if the application gives a hint
>>> about its usage of the table during initial configuration.
>>>
>>> The transfer domain rules may process traffic from wire or vport,
>>> which may correspond to two kinds of underlayer resources.
>>
>> Why name it a "vport"? Why not "host"?
>>
>> host = packets generated by any of the host's "vport"s wire = packets arriving
>> at the NIC from the network
> Vport is "virtual port" for short and contains "VF/SF" for now.
> Per my thoughts, it' clearer and maps to DPDK port probing/management.
I understand that "host" might not be a brilliant name.
If "vport" stands for every port of the NIC that is not a network port,
then this name might be OK to me, but why doesn't it cover PFs? A PF is
clearly not a network / physical port. Why just VF/SF then? Where does
that "for now" decision come from? Just wondering.
>>
>>> That's why the first two hints introduced in this patch are about wire
>>> and vport traffic specialization.
>>> Wire means traffic arrives from the uplink port while vport means
>>> traffic initiated from VF/SF.
>>
>> By the sound of it, the meaning is confined to just VFs/SFs.
>> What if the user wants to match packets coming from PFs?
>>
> It should be "wire_orig".
Forgive me, but that does not sound correct. Say, there's an application
and it has a PF plugged into it: ethdev index 0. And the application
transmits packets using rte_eth_tx_burst() from that port.
You say that these packets can be matched via "wire_orig".
But they do not come from the wire. They come from PF...
>>>
>>> There are two possible approaches for providing the hints.
>>> Using IPv4 as an example:
>>> 1. Use pattern item in both template table and flow rules.
>>>
>>> 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 flow rule even if it's just
>>> a hint. No value to match because matching is already done by
>>> IPv4 item.
>>
>> Why no value to match on? How does it prevent rogue tenants from spoofing
>> network headers? If the application receives a packet on a particular vport's
>> representor, then it may strictly specify item represented_port pointing to that
>> vport so that only packets from that vport match.
>>
>> Why isn't security a consideration?
>>
> There is some misunderstanding here. "ANY_VPORT" is the approach (new matching item without value) suggested by you.
I'm not talking about ANY_VPORT in this particular paragraph.
There's item "represented_port" mentioned over there. I'm just asking
about this "already done by IPv4 item" bit. Yes, it matches on the
header but not on the true origin of the packet (the logical port
of the NIC). If the app knows which logical port the packet
ingresses the NIC, why not match on it for security?
> I was explaining we need to apply it to each flow rule even if it's only a flag and no value.
That's clear. But PSB.
>>>
>>> 2. Add special flags into table_attr.
>>>
>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>
>>> Approach 1 needs to specify the pattern in each flow rule which wastes
>>> memory and is not user friendly.
>>
>> What if the user has to insert a group of rules which not only have the same
>> set of match masks but also share exactly the same match spec values for a
>> limited subset of network items (for example, those of an encap. header)? This
>> way, a subset of network item specs can remain fixed across many rules. Does
>> that count as wasting memory?
>>
> Per my understanding, you are talking "multiple spec and mask mixing".
Say, there's a group of rules, and each of them matches on exactly
the same encap. header (the same in all rules), but different
internal match field values. So, why don't these "fixed"
encap. header items deserve being "optimised" somehow,
the same way as this "wire_orig" does?
If the application knows for sure that there will be packets with exactly
the same encap. header, - that forms this special knowledge that can be
used during init times to help the PMD optimise resource allocation.
Isn't that so? Don't these items deserve some form of a "hint"?
> We provide a hint in this patch and no assumption on the matching patterns.
So I understand. My point is, certain portions of matching patterns
may be "fixed" = entirely the same (masks and specs) in all rules
of a table. Why not give PMD a "hint" about them, too?
> I think matching pattern is totally controlled by application layer.
So is the "direction" spec: the app layer has item represented_port
to control that. But, still, we're here to discuss a hint for that.
Why does the new hint aim exclusively at optimising out this
specific meta item? Why isn't it possible to care about a
generic portion of "know in advance" all-the-same items?
> "wasting memory " because your approach needs to scatter in each rule while this patch only needs to set table_attr once.
> No relation with matching patter totally.
The slight problem with your proposal is that for some reason only
one type of a match criterion deserves a hint moved to the attrs.
Whilst in reality the applicaction may know in advance that
certain subsets of items will not only have the same masks
in all rules but also totally the same specs. If that is
a valid use case, why doesn't it deserve the same (more
generic) optimisation / a hint? Just wondering...
Or has that been addressed already somehow?
>> If yes, then the problem does not concern just a single pair of attributes, but
>> rather deserves a more versatile solution like some sort of indirect grouping of
>> constant item specs.
>> Have you considered such options?
> See above.
>>
>>> This patch takes the 2nd approach and introduces one new member
>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>> optimization.
>>
>> The name "specialize" might have some drawbacks:
>> - spelling difference (specialise/specialize)
>> - in grep output, will mix with flows' "spec"
>> - quite long
>> - not a noun
>>
>> Why not "scope"? Or something like that?
>>
> It means special optimization to PMD. "scope" is more rogue.
Why is it "rogue"? Scope is something limiting the point of view.
So are the suggested flags. Flag "wire_origin" (or whatever it
can be named eventually) limits the scope of matching. No?
>>>
>>> By default, there is no hint, so the behavior of the transfer domain
>>> doesn't change.
>>> There is no guarantee that the hint will be used by the PMD.
>>>
>>> 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.
>>> v5: Change specialize type to uint32_t.
>>> v6: Change the flags to macros and re-construct the commit log.
>>> v7: Fix build failure.
>>> ---
>>> 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 | 28 +++++++++++++++++++++
>>> 4 files changed, 71 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
>>
>> Silently ignoring the field does not sit well with the application's possible intent
>> to drop represented_port match from the patterns. From my point of view, if
>> the application sets this attribute, it believes it can rely on it, that is, packets
>> coming from host won't match if the attribute asks to match network only, for
>> instance. Has this been considered?
>>
>>> +
>>> +If a table is specialized, the application should make sure the rules
>>> +comply with the table attribute.
>>
>> How does the application enforce that? I would appreciate you explain it.
>>
>>> +
>>> 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..c27b48c5c1 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -5186,6 +5186,29 @@ rte_flow_actions_template_destroy(uint16_t
>>> port_id, */ struct rte_flow_template_table;
>>>
>>> +/**@{@name Special optional flags for template table attribute
>>> + * Each bit is a hint for table specialization,
>>> + * offering a potential optimization at PMD layer.
>>> + * PMD can ignore the unsupported bits silently.
>>> + */
>>> +/**
>>> + * 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.
>>
>> You intended to spell "criterion", I take it. And still, it *is* a match criterion.
>> I'm not denying the possible need to have this criterion at the earliest
>> processing stage. That might be OK, but I still have a hunch that this is too
>> specific.
>> Please see my comment above about wasting memory.
>> I guess this type of criterion is not the only one that may need to be provided
>> as a "hint".
>>
>>> + * Flow rules which match non-wire originated traffic will be missed
>>> + * if the hint is supported.
>>
>> And what if it's unsupported? Is it indeed OK to silently ignore it?
>>
>>> + */
>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
>> RTE_BIT32(0)
>>
>> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
>>
>> To me, TRANSFER looks redundant as this bit is already supposed to be ticked
>> in the "struct rte_flow_attr flow_attr" field of the "struct
>> rte_flow_template_table_attr".
>>
>>> +/**
>>> + * Specialize table for transfer flows which come only from vport
>>> +(e.g. VF,
>>> SF).
>>
>> And PF?
>>
>>> + * 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.
>>> + */
>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
>> RTE_BIT32(1)
>>
>> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
>>
>>> +/**@}*/
>>> +
>>> /**
>>> * @warning
>>> * @b EXPERIMENTAL: this API may change without prior notice.
>>> @@ -5201,6 +5224,11 @@ 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.
>>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
>>> + */
>>> + uint32_t specialize;
>>
>> Why not "scope" or something?
>>
>>> };
>>>
>>> /**
>>> --
>>> 2.27.0
>>>
>>
>> Thank you.
>
HI Ivan
BR
Rongwei
> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Monday, January 30, 2023 15:40
> To: Rongwei Liu <rongweil@nvidia.com>
> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer
> table
>
> External email: Use caution opening links or attachments
>
>
> Hi Rongwei,
>
> For my responses, PSB.
>
> By the way, now you mention things like wasting memory and insertion
> optimisastions, are there any comparative figures to see the effect of this hint
> on insertion performance / memory footprint?
> Some "before" / "after" examples would really be helpful.
>
Good to hear we reach agreement almost.
First, the hint has nothing related to matching, only affects PMD resource management.
In my local test, it can save around 50% memory in the VxLAN encap/decap example case.
Insertion rate has very very few improvements.
> After all, I'm not objecting this patch. But I believe that other reviewers'
> concerns should nevertheless be addressed anyway.
Let me try to show the hint is useful.
>
> On Mon, 30 Jan 2023, Rongwei Liu wrote:
>
> > Hi Ivan,
> >
> > BR
> > Rongwei
> >
> >> -----Original Message-----
> >> From: Ivan Malov <ivan.malov@arknetworks.am>
> >> Sent: Monday, January 30, 2023 08:00
> >> To: Rongwei Liu <rongweil@nvidia.com>
> >> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> >> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> >> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> >> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> >> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> >> <rasland@nvidia.com>
> >> Subject: Re: [PATCH v7] ethdev: add special flags when creating async
> >> transfer table
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hi Rongwei,
> >>
> >> Thanks for persevering. I have no strong opinion, but, at least, the
> >> fact that the new flags are no longer meant for use in rte_flow_attr,
> >> which is clearly not the right place for such, is an improvement.
> >>
> > Thanks for the suggestion, move it to rte_flow_table_attr now and it'
> dedicated to async API.
> >> However, let's take a closer look at the current patch, shall we?
> >>
> >> But, before we get to that, I'd like to kindly request that you
> >> provide a more concrete example of how this feature is supposed to be
> >> used. Are there some real-life application examples?
> >>
> > Sure.
> >> Also, to me, it's still unclear how an application can obtain the
> >> knowledge of this hint in the first instance. For example, can Open
> >> vSwitch somehow tell ethdevs representing physical ports from ones
> >> representing "vports" (host endpoints)?
> >> How does it know which attribute to specify?
> >>
> > Hint should be initiated by application and application knows it' traffic
> pattern which highly relates to deployment.
> > Let' use VxLAN encap/decap as an example:
> > 1. Traffic from wire should be VxLAN pattern and do the decap, then send to
> different vports.
> > flow pattern_template 0 create transfer relaxed no pattern_template_id
> > 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp /
> > vxlan / tag index is 0 data is 0x33 / end flow actions_template 0
> > create transfer actions_template_id 4 template raw_decap index 0 /
> > represented_port ethdev_port_id 1 / end mask raw_decap index 0 /
> > represented_port ethdev_port_id 1 / end flow template_table 0 create
> > group 1 priority 0 transfer wire_orig table_id 4 rules_number 128
> > pattern_template 4 actions_template 4
> >
> > 2. Traffic from vports should be encap with different VxLAN header and send
> to wire.
> > flow actions_template 1 create transfer actions_template_id 5 template
> > raw_encap index 0 / represented_port ethdev_port_id 0 / end mask
> > raw_encap index 0 / represented_port ethdev_port_id 0 / end flow
> > template_table 0 create group 1 priority 0 transfer vport_orig
> > table_id 5 rules_number 128 pattern_template 4 actions_template 5
> >
> >> For the rest of my notes, PSB.
> >>
> >> On Mon, 14 Nov 2022, Rongwei Liu wrote:
> >>
> >>> In case flow rules match only one kind of traffic in a flow table,
> >>> then optimization can be done via allocation of this table.
> >>
> >> This wording might confuse readers. Consider rephrasing it, please:
> >> If multiple flow rules share a common set of match masks, then they
> >> might belong in a flow table which can be pre-allocated.
> >>
> >>> Such optimization is possible only if the application gives a hint
> >>> about its usage of the table during initial configuration.
> >>>
> >>> The transfer domain rules may process traffic from wire or vport,
> >>> which may correspond to two kinds of underlayer resources.
> >>
> >> Why name it a "vport"? Why not "host"?
> >>
> >> host = packets generated by any of the host's "vport"s wire = packets
> >> arriving at the NIC from the network
> > Vport is "virtual port" for short and contains "VF/SF" for now.
> > Per my thoughts, it' clearer and maps to DPDK port probing/management.
>
> I understand that "host" might not be a brilliant name.
>
> If "vport" stands for every port of the NIC that is not a network port, then this
> name might be OK to me, but why doesn't it cover PFs? A PF is clearly not a
> network / physical port. Why just VF/SF then? Where does that "for now"
> decision come from? Just wondering.
>
"For now" stands for my understanding. DPDK is always in evolution, right?
You are right, PF should be included in 'vport" concept.
> >>
> >>> That's why the first two hints introduced in this patch are about
> >>> wire and vport traffic specialization.
> >>> Wire means traffic arrives from the uplink port while vport means
> >>> traffic initiated from VF/SF.
> >>
> >> By the sound of it, the meaning is confined to just VFs/SFs.
> >> What if the user wants to match packets coming from PFs?
> >>
> > It should be "wire_orig".
>
> Forgive me, but that does not sound correct. Say, there's an application and it
> has a PF plugged into it: ethdev index 0. And the application transmits packets
> using rte_eth_tx_burst() from that port.
> You say that these packets can be matched via "wire_orig".
> But they do not come from the wire. They come from PF...
Hmm. My mistake.
This may highly depend on PMD implementation. Basically, PFs' traffic may contain
"from wire"/"wire_orig" and '"from local"/"vport_orig".
That' why we emphasize it' optional for PMD.
>
> >>>
> >>> There are two possible approaches for providing the hints.
> >>> Using IPv4 as an example:
> >>> 1. Use pattern item in both template table and flow rules.
> >>>
> >>> 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 flow rule even if it's
> >>> just a hint. No value to match because matching is already done by
> >>> IPv4 item.
> >>
> >> Why no value to match on? How does it prevent rogue tenants from
> >> spoofing network headers? If the application receives a packet on a
> >> particular vport's representor, then it may strictly specify item
> >> represented_port pointing to that vport so that only packets from that vport
> match.
> >>
> >> Why isn't security a consideration?
> >>
> > There is some misunderstanding here. "ANY_VPORT" is the approach (new
> matching item without value) suggested by you.
> I'm not talking about ANY_VPORT in this particular paragraph.
>
> There's item "represented_port" mentioned over there. I'm just asking about
> this "already done by IPv4 item" bit. Yes, it matches on the header but not on
> the true origin of the packet (the logical port of the NIC). If the app knows
> which logical port the packet ingresses the NIC, why not match on it for
> security?
>
Hint is not a matching and it implies how to manage underlayer steering resource.
If "vport_orig" is present, PMD will only apply the steering logic to vport traffic.
The resource is allocated in the async table before each rule. Already cover security considerations.
Matching on "represented_port" needs to program each rule, considering a port range like index "5-10".
Hint tells PMD only to take care of traffic from vport regardless the port index.
> > I was explaining we need to apply it to each flow rule even if it's only a flag
> and no value.
>
> That's clear. But PSB.
>
> >>>
> >>> 2. Add special flags into table_attr.
> >>>
> >>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>
> >>> Approach 1 needs to specify the pattern in each flow rule which
> >>> wastes memory and is not user friendly.
> >>
> >> What if the user has to insert a group of rules which not only have
> >> the same set of match masks but also share exactly the same match
> >> spec values for a limited subset of network items (for example, those
> >> of an encap. header)? This way, a subset of network item specs can
> >> remain fixed across many rules. Does that count as wasting memory?
> >>
> > Per my understanding, you are talking "multiple spec and mask mixing".
>
> Say, there's a group of rules, and each of them matches on exactly the same
> encap. header (the same in all rules), but different internal match field values.
> So, why don't these "fixed"
> encap. header items deserve being "optimised" somehow, the same way as
> this "wire_orig" does?
We are back to original point. Async approach is trying to pre-allocate resources and speed up the insertion.
Resource is allocated in async table stage and we only have mask information.
In each rule, the matching value passes in. I guess you are saying to optimize per different matching values, right?
This needs dynamic calculations per each rule and wastes the resource in async table(table allocates resource for all possible values).
>
> If the application knows for sure that there will be packets with exactly the
> same encap. header, - that forms this special knowledge that can be used
> during init times to help the PMD optimise resource allocation.
> Isn't that so? Don't these items deserve some form of a "hint"?
>
It can deserve some kinds of "hint". But see above, these hints are per rule and resource allocation happens before rules.
> > We provide a hint in this patch and no assumption on the matching patterns.
>
> So I understand. My point is, certain portions of matching patterns may be
> "fixed" = entirely the same (masks and specs) in all rules of a table. Why not
> give PMD a "hint" about them, too?
>
> > I think matching pattern is totally controlled by application layer.
>
> So is the "direction" spec: the app layer has item represented_port to control
> that. But, still, we're here to discuss a hint for that.
> Why does the new hint aim exclusively at optimising out this specific meta
> item? Why isn't it possible to care about a generic portion of "know in
> advance" all-the-same items?
" generic portion of know in advance" is some still kind of dynamic approach, right?
Imagine a situation. DPDK has 10 VFs, each VF may have different VxLAN encap headers.
This hint approach can work for 10 VFs once.
In public cloud deployments, each VF/SF may map to different users, but underlay is almost same(GRE/VxLAN... differ in filed values).
>
> > "wasting memory " because your approach needs to scatter in each rule
> while this patch only needs to set table_attr once.
> > No relation with matching patter totally.
>
> The slight problem with your proposal is that for some reason only one type of
> a match criterion deserves a hint moved to the attrs.
> Whilst in reality the applicaction may know in advance that certain subsets of
> items will not only have the same masks in all rules but also totally the same
> specs. If that is a valid use case, why doesn't it deserve the same (more
> generic) optimisation / a hint? Just wondering...
> Or has that been addressed already somehow?
>
Believe me, the hint helps us to save significant resources already.
Per my view, your proposal is totally valid in sync approach, but please check my response,
Async is trying to allocate resources in advance and speed up insertion ASAP.
> >> If yes, then the problem does not concern just a single pair of
> >> attributes, but rather deserves a more versatile solution like some
> >> sort of indirect grouping of constant item specs.
> >> Have you considered such options?
> > See above.
> >>
> >>> This patch takes the 2nd approach and introduces one new member
> >>> "specialize" into rte_flow_table_attr to indicate possible flow
> >>> table optimization.
> >>
> >> The name "specialize" might have some drawbacks:
> >> - spelling difference (specialise/specialize)
> >> - in grep output, will mix with flows' "spec"
> >> - quite long
> >> - not a noun
> >>
> >> Why not "scope"? Or something like that?
> >>
> > It means special optimization to PMD. "scope" is more rogue.
>
> Why is it "rogue"? Scope is something limiting the point of view.
> So are the suggested flags. Flag "wire_origin" (or whatever it can be named
> eventually) limits the scope of matching. No?
>
Hint won't interfere with matching. It has no knowledge of matching.
Instead, it only controls matching resources. "wire_orig" tells PMD to allocate HW resource for traffic from wire only.
Then traffic from vport is sliently ignored. Hint doesn't know what are matched and how many fields are involves.
> >>>
> >>> By default, there is no hint, so the behavior of the transfer domain
> >>> doesn't change.
> >>> There is no guarantee that the hint will be used by the PMD.
> >>>
> >>> 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.
> >>> v5: Change specialize type to uint32_t.
> >>> v6: Change the flags to macros and re-construct the commit log.
> >>> v7: Fix build failure.
> >>> ---
> >>> 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 | 28 +++++++++++++++++++++
> >>> 4 files changed, 71 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/app/test-pmd/cmdline_flow.c
> >>> b/app/test-pmd/cmdline_flow.c index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
> >>
> >> Silently ignoring the field does not sit well with the application's
> >> possible intent to drop represented_port match from the patterns.
> >> From my point of view, if the application sets this attribute, it
> >> believes it can rely on it, that is, packets coming from host won't
> >> match if the attribute asks to match network only, for instance. Has this
> been considered?
> >>
> >>> +
> >>> +If a table is specialized, the application should make sure the
> >>> +rules comply with the table attribute.
> >>
> >> How does the application enforce that? I would appreciate you explain it.
> >>
> >>> +
> >>> 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..c27b48c5c1 100644
> >>> --- a/lib/ethdev/rte_flow.h
> >>> +++ b/lib/ethdev/rte_flow.h
> >>> @@ -5186,6 +5186,29 @@ rte_flow_actions_template_destroy(uint16_t
> >>> port_id, */ struct rte_flow_template_table;
> >>>
> >>> +/**@{@name Special optional flags for template table attribute
> >>> + * Each bit is a hint for table specialization,
> >>> + * offering a potential optimization at PMD layer.
> >>> + * PMD can ignore the unsupported bits silently.
> >>> + */
> >>> +/**
> >>> + * 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.
> >>
> >> You intended to spell "criterion", I take it. And still, it *is* a match criterion.
> >> I'm not denying the possible need to have this criterion at the
> >> earliest processing stage. That might be OK, but I still have a hunch
> >> that this is too specific.
> >> Please see my comment above about wasting memory.
> >> I guess this type of criterion is not the only one that may need to
> >> be provided as a "hint".
> >>
> >>> + * Flow rules which match non-wire originated traffic will be
> >>> + missed
> >>> + * if the hint is supported.
> >>
> >> And what if it's unsupported? Is it indeed OK to silently ignore it?
> >>
> >>> + */
> >>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
> >> RTE_BIT32(0)
> >>
> >> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
> >>
> >> To me, TRANSFER looks redundant as this bit is already supposed to be
> >> ticked in the "struct rte_flow_attr flow_attr" field of the "struct
> >> rte_flow_template_table_attr".
> >>
> >>> +/**
> >>> + * Specialize table for transfer flows which come only from vport
> >>> +(e.g. VF,
> >>> SF).
> >>
> >> And PF?
> >>
> >>> + * 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.
> >>> + */
> >>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
> >> RTE_BIT32(1)
> >>
> >> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
> >>
> >>> +/**@}*/
> >>> +
> >>> /**
> >>> * @warning
> >>> * @b EXPERIMENTAL: this API may change without prior notice.
> >>> @@ -5201,6 +5224,11 @@ 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.
> >>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
> >>> + */
> >>> + uint32_t specialize;
> >>
> >> Why not "scope" or something?
> >>
> >>> };
> >>>
> >>> /**
> >>> --
> >>> 2.27.0
> >>>
> >>
> >> Thank you.
> >
Hi Rongwei,
Thanks for the professional attitude.
Hope this discussion gets us on the
same page. Please see below.
On Mon, 30 Jan 2023, Rongwei Liu wrote:
> HI Ivan
>
> BR
> Rongwei
>
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@arknetworks.am>
>> Sent: Monday, January 30, 2023 15:40
>> To: Rongwei Liu <rongweil@nvidia.com>
>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
>> <rasland@nvidia.com>
>> Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer
>> table
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Rongwei,
>>
>> For my responses, PSB.
>>
>> By the way, now you mention things like wasting memory and insertion
>> optimisastions, are there any comparative figures to see the effect of this hint
>> on insertion performance / memory footprint?
>> Some "before" / "after" examples would really be helpful.
>>
> Good to hear we reach agreement almost.
Very well.
The key point here is that one may agree that some optimisations
are indeed needed, yes. I don't deny the fact that some vendors
might have issues with how the API maps to the HW capabilities.
Yes, some undesirable resource overhead shall be avoided, but
the high level hints for that have to be designed with care.
> First, the hint has nothing related to matching, only affects PMD resource management.
You say "PMD resource management". For the flow management, that's mostly
vendor-specific, I take it. Let me explain. The application, for instance,
can control the number of Tx descriptors in the queue during setup stage.
Tx descriptors are a common type of HW resource, hence the explicit
control for it available to applications. For flow library, it's
not like that. Different vendors have different "underlayer"
representations, they may vary drastically.
I take it, in the case of the HW you're working with, this hint indeed
maps to something that is entirely resource-related and which does not
belong in this specific vendor's match criteria. I 100% understand
that, in your case, these are separate. But the point is that, on
the high-level programming level (vendor-neutral), such a hint
is in fact a match criterion. Because it tells the driver to
limit the scope of matching to just "from net"/"from vport",
the same way other metadata items do (represented_port).
The only difference is that it refers to a group of
unspecified ports which have something in common.
So, although I don't strongly object having some hints like this one
in the generic API, I nevertheless disagree with describing this as
just "resource-specific" and not being a match criterion. It's just
not always the case. It might not be valid for *all* NIC vendors.
> In my local test, it can save around 50% memory in the VxLAN encap/decap example case.
Forgive me in case this has been already discussed; where's that memory?
I mean, is it some sort of general-purpose memory? Or some HW-specific
table capacity overhead? I'm trying to understand how the feature
will be useful to other vendors, or how common this problem is.
> Insertion rate has very very few improvements.
>> After all, I'm not objecting this patch. But I believe that other reviewers'
>> concerns should nevertheless be addressed anyway.
> Let me try to show the hint is useful.
>>
>> On Mon, 30 Jan 2023, Rongwei Liu wrote:
>>
>>> Hi Ivan,
>>>
>>> BR
>>> Rongwei
>>>
>>>> -----Original Message-----
>>>> From: Ivan Malov <ivan.malov@arknetworks.am>
>>>> Sent: Monday, January 30, 2023 08:00
>>>> To: Rongwei Liu <rongweil@nvidia.com>
>>>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>>>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
>>>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
>>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>>>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
>>>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
>>>> <rasland@nvidia.com>
>>>> Subject: Re: [PATCH v7] ethdev: add special flags when creating async
>>>> transfer table
>>>>
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> Hi Rongwei,
>>>>
>>>> Thanks for persevering. I have no strong opinion, but, at least, the
>>>> fact that the new flags are no longer meant for use in rte_flow_attr,
>>>> which is clearly not the right place for such, is an improvement.
>>>>
>>> Thanks for the suggestion, move it to rte_flow_table_attr now and it'
>> dedicated to async API.
>>>> However, let's take a closer look at the current patch, shall we?
>>>>
>>>> But, before we get to that, I'd like to kindly request that you
>>>> provide a more concrete example of how this feature is supposed to be
>>>> used. Are there some real-life application examples?
>>>>
>>> Sure.
>>>> Also, to me, it's still unclear how an application can obtain the
>>>> knowledge of this hint in the first instance. For example, can Open
>>>> vSwitch somehow tell ethdevs representing physical ports from ones
>>>> representing "vports" (host endpoints)?
>>>> How does it know which attribute to specify?
>>>>
>>> Hint should be initiated by application and application knows it' traffic
>> pattern which highly relates to deployment.
>>> Let' use VxLAN encap/decap as an example:
>>> 1. Traffic from wire should be VxLAN pattern and do the decap, then send to
>> different vports.
>>> flow pattern_template 0 create transfer relaxed no pattern_template_id
>>> 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp /
>>> vxlan / tag index is 0 data is 0x33 / end flow actions_template 0
>>> create transfer actions_template_id 4 template raw_decap index 0 /
>>> represented_port ethdev_port_id 1 / end mask raw_decap index 0 /
>>> represented_port ethdev_port_id 1 / end flow template_table 0 create
>>> group 1 priority 0 transfer wire_orig table_id 4 rules_number 128
>>> pattern_template 4 actions_template 4
>>>
>>> 2. Traffic from vports should be encap with different VxLAN header and send
>> to wire.
>>> flow actions_template 1 create transfer actions_template_id 5 template
>>> raw_encap index 0 / represented_port ethdev_port_id 0 / end mask
>>> raw_encap index 0 / represented_port ethdev_port_id 0 / end flow
>>> template_table 0 create group 1 priority 0 transfer vport_orig
>>> table_id 5 rules_number 128 pattern_template 4 actions_template 5
>>>
>>>> For the rest of my notes, PSB.
>>>>
>>>> On Mon, 14 Nov 2022, Rongwei Liu wrote:
>>>>
>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>> then optimization can be done via allocation of this table.
>>>>
>>>> This wording might confuse readers. Consider rephrasing it, please:
>>>> If multiple flow rules share a common set of match masks, then they
>>>> might belong in a flow table which can be pre-allocated.
>>>>
>>>>> Such optimization is possible only if the application gives a hint
>>>>> about its usage of the table during initial configuration.
>>>>>
>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>> which may correspond to two kinds of underlayer resources.
>>>>
>>>> Why name it a "vport"? Why not "host"?
>>>>
>>>> host = packets generated by any of the host's "vport"s wire = packets
>>>> arriving at the NIC from the network
>>> Vport is "virtual port" for short and contains "VF/SF" for now.
>>> Per my thoughts, it' clearer and maps to DPDK port probing/management.
>>
>> I understand that "host" might not be a brilliant name.
>>
>> If "vport" stands for every port of the NIC that is not a network port, then this
>> name might be OK to me, but why doesn't it cover PFs? A PF is clearly not a
>> network / physical port. Why just VF/SF then? Where does that "for now"
>> decision come from? Just wondering.
>>
> "For now" stands for my understanding. DPDK is always in evolution, right?
> You are right, PF should be included in 'vport" concept.
>>>>
>>>>> That's why the first two hints introduced in this patch are about
>>>>> wire and vport traffic specialization.
>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>> traffic initiated from VF/SF.
>>>>
>>>> By the sound of it, the meaning is confined to just VFs/SFs.
>>>> What if the user wants to match packets coming from PFs?
>>>>
>>> It should be "wire_orig".
>>
>> Forgive me, but that does not sound correct. Say, there's an application and it
>> has a PF plugged into it: ethdev index 0. And the application transmits packets
>> using rte_eth_tx_burst() from that port.
>> You say that these packets can be matched via "wire_orig".
>> But they do not come from the wire. They come from PF...
> Hmm. My mistake.
> This may highly depend on PMD implementation. Basically, PFs' traffic may contain
> "from wire"/"wire_orig" and '"from local"/"vport_orig".
> That' why we emphasize it' optional for PMD.
>>
>>>>>
>>>>> There are two possible approaches for providing the hints.
>>>>> Using IPv4 as an example:
>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>
>>>>> 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 flow rule even if it's
>>>>> just a hint. No value to match because matching is already done by
>>>>> IPv4 item.
>>>>
>>>> Why no value to match on? How does it prevent rogue tenants from
>>>> spoofing network headers? If the application receives a packet on a
>>>> particular vport's representor, then it may strictly specify item
>>>> represented_port pointing to that vport so that only packets from that vport
>> match.
>>>>
>>>> Why isn't security a consideration?
>>>>
>>> There is some misunderstanding here. "ANY_VPORT" is the approach (new
>> matching item without value) suggested by you.
>> I'm not talking about ANY_VPORT in this particular paragraph.
>>
>> There's item "represented_port" mentioned over there. I'm just asking about
>> this "already done by IPv4 item" bit. Yes, it matches on the header but not on
>> the true origin of the packet (the logical port of the NIC). If the app knows
>> which logical port the packet ingresses the NIC, why not match on it for
>> security?
>>
> Hint is not a matching and it implies how to manage underlayer steering resource.
> If "vport_orig" is present, PMD will only apply the steering logic to vport traffic.
> The resource is allocated in the async table before each rule. Already cover security considerations.
> Matching on "represented_port" needs to program each rule, considering a port range like index "5-10".
> Hint tells PMD only to take care of traffic from vport regardless the port index.
>
>>> I was explaining we need to apply it to each flow rule even if it's only a flag
>> and no value.
>>
>> That's clear. But PSB.
>>
>>>>>
>>>>> 2. Add special flags into table_attr.
>>>>>
>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>
>>>>> Approach 1 needs to specify the pattern in each flow rule which
>>>>> wastes memory and is not user friendly.
>>>>
>>>> What if the user has to insert a group of rules which not only have
>>>> the same set of match masks but also share exactly the same match
>>>> spec values for a limited subset of network items (for example, those
>>>> of an encap. header)? This way, a subset of network item specs can
>>>> remain fixed across many rules. Does that count as wasting memory?
>>>>
>>> Per my understanding, you are talking "multiple spec and mask mixing".
>>
>> Say, there's a group of rules, and each of them matches on exactly the same
>> encap. header (the same in all rules), but different internal match field values.
>> So, why don't these "fixed"
>> encap. header items deserve being "optimised" somehow, the same way as
>> this "wire_orig" does?
> We are back to original point. Async approach is trying to pre-allocate resources and speed up the insertion.
> Resource is allocated in async table stage and we only have mask information.
> In each rule, the matching value passes in. I guess you are saying to optimize per different matching values, right?
> This needs dynamic calculations per each rule and wastes the resource in async table(table allocates resource for all possible values).
>>
>> If the application knows for sure that there will be packets with exactly the
>> same encap. header, - that forms this special knowledge that can be used
>> during init times to help the PMD optimise resource allocation.
>> Isn't that so? Don't these items deserve some form of a "hint"?
>>
> It can deserve some kinds of "hint". But see above, these hints are per rule and resource allocation happens before rules.
That's not per rule. Perhaps I should've worded it differently.
Suppose, an application has to insert many flow rules, each
of which has match items A and B. Item A not only has the
same mask in *all* rule instances, but also the same spec.
On the other hand, item B only has the same mask in all
the rules, but its spec is different for each rule.
In this example, the application can allocate a template
with items A and B, but that only provides a fixed mask
for them. And the application will HAVE to provide item
A with exactly the same spec in all rule instances. The
PMD, in turn, will HAVE to process this item every time,
being unable to see it's in fact the same at all times.
To me, this sounds very similar to how you described the
need to always provide item ANY_VPORT in each rule thus
facing some waste of memory and parsing difficulties.
If the application knows that a certain item (or a certain
fraction of items) is going to be entirely the same (mask +
spec) across all the rules, why shouldn't it be able to
express this as a hint to the PMD? Why shouldn't it be
able to avoid providing such items in every new flow
rule instance? The same way the "vport_orig" works.
I'm not demanding that you re-implement or re-design this.
Just trying to find out whether such a problem can indeed
be acknowledged. Or has it been solved already? If not,
then perhaps it pays to just discuss whether solving
it can be combined with this "vport_orig" solution.
What do you think? What do others think?
>>> We provide a hint in this patch and no assumption on the matching patterns.
>>
>> So I understand. My point is, certain portions of matching patterns may be
>> "fixed" = entirely the same (masks and specs) in all rules of a table. Why not
>> give PMD a "hint" about them, too?
>>
>>> I think matching pattern is totally controlled by application layer.
>>
>> So is the "direction" spec: the app layer has item represented_port to control
>> that. But, still, we're here to discuss a hint for that.
>> Why does the new hint aim exclusively at optimising out this specific meta
>> item? Why isn't it possible to care about a generic portion of "know in
>> advance" all-the-same items?
> " generic portion of know in advance" is some still kind of dynamic approach, right?
> Imagine a situation. DPDK has 10 VFs, each VF may have different VxLAN encap headers.
> This hint approach can work for 10 VFs once.
> In public cloud deployments, each VF/SF may map to different users, but underlay is almost same(GRE/VxLAN... differ in filed values).
>>
>>> "wasting memory " because your approach needs to scatter in each rule
>> while this patch only needs to set table_attr once.
>>> No relation with matching patter totally.
>>
>> The slight problem with your proposal is that for some reason only one type of
>> a match criterion deserves a hint moved to the attrs.
>> Whilst in reality the applicaction may know in advance that certain subsets of
>> items will not only have the same masks in all rules but also totally the same
>> specs. If that is a valid use case, why doesn't it deserve the same (more
>> generic) optimisation / a hint? Just wondering...
>> Or has that been addressed already somehow?
>>
> Believe me, the hint helps us to save significant resources already.
I'm not arguing it can be helpful. You're working round the clock
to offer a solution, - that's fine and is greatly appreciated.
But what I'm trying to say is that it looks like the problem
might manifest itself for other type of knowledge that also
may deserve a hint. Hence the questions. Hence the offer to
think of covering more match criteria, not just net/vport.
> Per my view, your proposal is totally valid in sync approach, but please check my response,
> Async is trying to allocate resources in advance and speed up insertion ASAP.
So if it's valid in sync approach, then why can't it be valid in the
async one? And I guess it can reflect positively on the insertion
rate, too. Why limit this "hint" approach to just one aspect then?
I'm sure we're close to understanding each other here.
Yes, "orig_vport" is just a one-bit knowledge, and
seems innocent to add as a hint, but why not make
it possible to have a hint for an arbitrary set
of always-the-same match criteria?
In this case, nobody will ever argue of whether a hint
is a match criterion or if it's not. It will be quite
a generic instrument, potentially useful to vendors.
I'm afraid I can't think of an immediate example of
such usefulness, but at least it will appear as
generic as possible from the API perspective.
>>>> If yes, then the problem does not concern just a single pair of
>>>> attributes, but rather deserves a more versatile solution like some
>>>> sort of indirect grouping of constant item specs.
>>>> Have you considered such options?
>>> See above.
>>>>
>>>>> This patch takes the 2nd approach and introduces one new member
>>>>> "specialize" into rte_flow_table_attr to indicate possible flow
>>>>> table optimization.
>>>>
>>>> The name "specialize" might have some drawbacks:
>>>> - spelling difference (specialise/specialize)
>>>> - in grep output, will mix with flows' "spec"
>>>> - quite long
>>>> - not a noun
>>>>
>>>> Why not "scope"? Or something like that?
>>>>
>>> It means special optimization to PMD. "scope" is more rogue.
>>
>> Why is it "rogue"? Scope is something limiting the point of view.
>> So are the suggested flags. Flag "wire_origin" (or whatever it can be named
>> eventually) limits the scope of matching. No?
>>
> Hint won't interfere with matching. It has no knowledge of matching.
Does specifying "orig_vport" actually provide a *choice* for the
packet origin? Does it filter out everything else? If yes,
then, alas, it *is* matching. Because matching is
choosing something of interest. Let's face it.
As I said above, I do acknowledge the fact that, for some vendors,
this match criterion, internally goes to a different HW aspect
that is separate from matching on, for example, IPv4 addresses.
That's OK. But for some vendors, this might be just a regular
match criterion internally. So let's describe it with care.
> Instead, it only controls matching resources. "wire_orig" tells PMD to allocate HW resource for traffic from wire only.
If it controls "matching resources", it's indeed affiliated with
matching then. Look. When the application creates a template, it
tells the PMD that it is going to match on this, this and this.
Masks... No exact values; they will come at a later stage. But,
with this "wire_orig", the application tells the PMD that not
only it will match on *some* "direction", but it actually
provides a SPEC for that. If it indicates bit "wire_orig",
that equals to setting a "mask" for the "direction enum"
AND a "spec" (WIRE). Isn't that the case?
If it is, then please see my above concerns about possibly having
similar need to provide exact-spec hints for other items as well.
> Then traffic from vport is sliently ignored. Hint doesn't know what are matched and how many fields are involves.
>>>>>
>>>>> By default, there is no hint, so the behavior of the transfer domain
>>>>> doesn't change.
>>>>> There is no guarantee that the hint will be used by the PMD.
>>>>>
>>>>> 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.
>>>>> v5: Change specialize type to uint32_t.
>>>>> v6: Change the flags to macros and re-construct the commit log.
>>>>> v7: Fix build failure.
>>>>> ---
>>>>> 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 | 28 +++++++++++++++++++++
>>>>> 4 files changed, 71 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/app/test-pmd/cmdline_flow.c
>>>>> b/app/test-pmd/cmdline_flow.c index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
>>>>
>>>> Silently ignoring the field does not sit well with the application's
>>>> possible intent to drop represented_port match from the patterns.
>>>> From my point of view, if the application sets this attribute, it
>>>> believes it can rely on it, that is, packets coming from host won't
>>>> match if the attribute asks to match network only, for instance. Has this
>> been considered?
>>>>
>>>>> +
>>>>> +If a table is specialized, the application should make sure the
>>>>> +rules comply with the table attribute.
>>>>
>>>> How does the application enforce that? I would appreciate you explain it.
>>>>
>>>>> +
>>>>> 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..c27b48c5c1 100644
>>>>> --- a/lib/ethdev/rte_flow.h
>>>>> +++ b/lib/ethdev/rte_flow.h
>>>>> @@ -5186,6 +5186,29 @@ rte_flow_actions_template_destroy(uint16_t
>>>>> port_id, */ struct rte_flow_template_table;
>>>>>
>>>>> +/**@{@name Special optional flags for template table attribute
>>>>> + * Each bit is a hint for table specialization,
>>>>> + * offering a potential optimization at PMD layer.
>>>>> + * PMD can ignore the unsupported bits silently.
>>>>> + */
>>>>> +/**
>>>>> + * 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.
>>>>
>>>> You intended to spell "criterion", I take it. And still, it *is* a match criterion.
>>>> I'm not denying the possible need to have this criterion at the
>>>> earliest processing stage. That might be OK, but I still have a hunch
>>>> that this is too specific.
>>>> Please see my comment above about wasting memory.
>>>> I guess this type of criterion is not the only one that may need to
>>>> be provided as a "hint".
>>>>
>>>>> + * Flow rules which match non-wire originated traffic will be
>>>>> + missed
>>>>> + * if the hint is supported.
>>>>
>>>> And what if it's unsupported? Is it indeed OK to silently ignore it?
>>>>
>>>>> + */
>>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
>>>> RTE_BIT32(0)
>>>>
>>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
>>>>
>>>> To me, TRANSFER looks redundant as this bit is already supposed to be
>>>> ticked in the "struct rte_flow_attr flow_attr" field of the "struct
>>>> rte_flow_template_table_attr".
>>>>
>>>>> +/**
>>>>> + * Specialize table for transfer flows which come only from vport
>>>>> +(e.g. VF,
>>>>> SF).
>>>>
>>>> And PF?
>>>>
>>>>> + * 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.
>>>>> + */
>>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
>>>> RTE_BIT32(1)
>>>>
>>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
>>>>
>>>>> +/**@}*/
>>>>> +
>>>>> /**
>>>>> * @warning
>>>>> * @b EXPERIMENTAL: this API may change without prior notice.
>>>>> @@ -5201,6 +5224,11 @@ 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.
>>>>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
>>>>> + */
>>>>> + uint32_t specialize;
>>>>
>>>> Why not "scope" or something?
>>>>
>>>>> };
>>>>>
>>>>> /**
>>>>> --
>>>>> 2.27.0
>>>>>
>>>>
>>>> Thank you.
>>>
>
Thank you.
HI Ivan
BR
Rongwei
> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Tuesday, January 31, 2023 07:00
> To: Rongwei Liu <rongweil@nvidia.com>
> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer
> table
>
> External email: Use caution opening links or attachments
>
>
> Hi Rongwei,
>
> Thanks for the professional attitude.
> Hope this discussion gets us on the
> same page. Please see below.
Thanks for the suggestion and comments. Hope everything goes well.
>
> On Mon, 30 Jan 2023, Rongwei Liu wrote:
>
> > HI Ivan
> >
> > BR
> > Rongwei
> >
> >> -----Original Message-----
> >> From: Ivan Malov <ivan.malov@arknetworks.am>
> >> Sent: Monday, January 30, 2023 15:40
> >> To: Rongwei Liu <rongweil@nvidia.com>
> >> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> >> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> >> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> >> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> >> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> >> <rasland@nvidia.com>
> >> Subject: RE: [PATCH v7] ethdev: add special flags when creating async
> >> transfer table
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hi Rongwei,
> >>
> >> For my responses, PSB.
> >>
> >> By the way, now you mention things like wasting memory and insertion
> >> optimisastions, are there any comparative figures to see the effect
> >> of this hint on insertion performance / memory footprint?
> >> Some "before" / "after" examples would really be helpful.
> >>
> > Good to hear we reach agreement almost.
>
> Very well.
>
> The key point here is that one may agree that some optimisations are indeed
> needed, yes. I don't deny the fact that some vendors might have issues with
> how the API maps to the HW capabilities.
> Yes, some undesirable resource overhead shall be avoided, but the high level
> hints for that have to be designed with care.
>
Totally agree. That' why we emphasize "optional for PMD" and "application should take care of hint"
> > First, the hint has nothing related to matching, only affects PMD resource
> management.
>
> You say "PMD resource management". For the flow management, that's
> mostly vendor-specific, I take it. Let me explain. The application, for instance,
> can control the number of Tx descriptors in the queue during setup stage.
> Tx descriptors are a common type of HW resource, hence the explicit control
> for it available to applications. For flow library, it's not like that. Different
> vendors have different "underlayer"
> representations, they may vary drastically.
The resource I mentioned is about "steering logic" not SW datapath.
With flow rules offloading, hardware should store the steering logic in its reachable memory no matter embedded in or mapping from host OS.
>
> I take it, in the case of the HW you're working with, this hint indeed maps to
> something that is entirely resource-related and which does not belong in this
> specific vendor's match criteria. I 100% understand that, in your case, these
> are separate. But the point is that, on the high-level programming level
> (vendor-neutral), such a hint is in fact a match criterion. Because it tells the
> driver to limit the scope of matching to just "from net"/"from vport", the same
> way other metadata items do (represented_port).
> The only difference is that it refers to a group of unspecified ports which have
> something in common.
>
" a group of unspecified ports" means dynamic and flexible, right. IMO it's valid and fits sync flow perfectly.
But in async, when allocating resources (table creation), the group info is still unknown. We don't want to scatter it into each rule insertion.
> So, although I don't strongly object having some hints like this one in the
> generic API, I nevertheless disagree with describing this as just "resource-
> specific" and not being a match criterion. It's just not always the case. It might
> not be valid for *all* NIC vendors.
>
Agree, not valid for *all* NIC vendors.
> > In my local test, it can save around 50% memory in the VxLAN encap/decap
> example case.
>
> Forgive me in case this has been already discussed; where's that memory?
> I mean, is it some sort of general-purpose memory? Or some HW-specific
> table capacity overhead? I'm trying to understand how the feature will be
> useful to other vendors, or how common this problem is.
>
See above. HW always needs memory to store offloaded rules no matter embedded in chip or borrowed from OS.
> > Insertion rate has very very few improvements.
> >> After all, I'm not objecting this patch. But I believe that other reviewers'
> >> concerns should nevertheless be addressed anyway.
> > Let me try to show the hint is useful.
> >>
> >> On Mon, 30 Jan 2023, Rongwei Liu wrote:
> >>
> >>> Hi Ivan,
> >>>
> >>> BR
> >>> Rongwei
> >>>
> >>>> -----Original Message-----
> >>>> From: Ivan Malov <ivan.malov@arknetworks.am>
> >>>> Sent: Monday, January 30, 2023 08:00
> >>>> To: Rongwei Liu <rongweil@nvidia.com>
> >>>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> >>>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> >>>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> >>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> >>>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> >>>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> >>>> <rasland@nvidia.com>
> >>>> Subject: Re: [PATCH v7] ethdev: add special flags when creating
> >>>> async transfer table
> >>>>
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>> Hi Rongwei,
> >>>>
> >>>> Thanks for persevering. I have no strong opinion, but, at least,
> >>>> the fact that the new flags are no longer meant for use in
> >>>> rte_flow_attr, which is clearly not the right place for such, is an
> improvement.
> >>>>
> >>> Thanks for the suggestion, move it to rte_flow_table_attr now and it'
> >> dedicated to async API.
> >>>> However, let's take a closer look at the current patch, shall we?
> >>>>
> >>>> But, before we get to that, I'd like to kindly request that you
> >>>> provide a more concrete example of how this feature is supposed to
> >>>> be used. Are there some real-life application examples?
> >>>>
> >>> Sure.
> >>>> Also, to me, it's still unclear how an application can obtain the
> >>>> knowledge of this hint in the first instance. For example, can Open
> >>>> vSwitch somehow tell ethdevs representing physical ports from ones
> >>>> representing "vports" (host endpoints)?
> >>>> How does it know which attribute to specify?
> >>>>
> >>> Hint should be initiated by application and application knows it'
> >>> traffic
> >> pattern which highly relates to deployment.
> >>> Let' use VxLAN encap/decap as an example:
> >>> 1. Traffic from wire should be VxLAN pattern and do the decap, then
> >>> send to
> >> different vports.
> >>> flow pattern_template 0 create transfer relaxed no
> >>> pattern_template_id
> >>> 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp /
> >>> vxlan / tag index is 0 data is 0x33 / end flow actions_template 0
> >>> create transfer actions_template_id 4 template raw_decap index 0 /
> >>> represented_port ethdev_port_id 1 / end mask raw_decap index 0 /
> >>> represented_port ethdev_port_id 1 / end flow template_table 0 create
> >>> group 1 priority 0 transfer wire_orig table_id 4 rules_number 128
> >>> pattern_template 4 actions_template 4
> >>>
> >>> 2. Traffic from vports should be encap with different VxLAN header
> >>> and send
> >> to wire.
> >>> flow actions_template 1 create transfer actions_template_id 5
> >>> template raw_encap index 0 / represented_port ethdev_port_id 0 / end
> >>> mask raw_encap index 0 / represented_port ethdev_port_id 0 / end
> >>> flow template_table 0 create group 1 priority 0 transfer vport_orig
> >>> table_id 5 rules_number 128 pattern_template 4 actions_template 5
> >>>
> >>>> For the rest of my notes, PSB.
> >>>>
> >>>> On Mon, 14 Nov 2022, Rongwei Liu wrote:
> >>>>
> >>>>> In case flow rules match only one kind of traffic in a flow table,
> >>>>> then optimization can be done via allocation of this table.
> >>>>
> >>>> This wording might confuse readers. Consider rephrasing it, please:
> >>>> If multiple flow rules share a common set of match masks, then they
> >>>> might belong in a flow table which can be pre-allocated.
> >>>>
> >>>>> Such optimization is possible only if the application gives a hint
> >>>>> about its usage of the table during initial configuration.
> >>>>>
> >>>>> The transfer domain rules may process traffic from wire or vport,
> >>>>> which may correspond to two kinds of underlayer resources.
> >>>>
> >>>> Why name it a "vport"? Why not "host"?
> >>>>
> >>>> host = packets generated by any of the host's "vport"s wire =
> >>>> packets arriving at the NIC from the network
> >>> Vport is "virtual port" for short and contains "VF/SF" for now.
> >>> Per my thoughts, it' clearer and maps to DPDK port probing/management.
> >>
> >> I understand that "host" might not be a brilliant name.
> >>
> >> If "vport" stands for every port of the NIC that is not a network
> >> port, then this name might be OK to me, but why doesn't it cover PFs?
> >> A PF is clearly not a network / physical port. Why just VF/SF then? Where
> does that "for now"
> >> decision come from? Just wondering.
> >>
> > "For now" stands for my understanding. DPDK is always in evolution, right?
> > You are right, PF should be included in 'vport" concept.
> >>>>
> >>>>> That's why the first two hints introduced in this patch are about
> >>>>> wire and vport traffic specialization.
> >>>>> Wire means traffic arrives from the uplink port while vport means
> >>>>> traffic initiated from VF/SF.
> >>>>
> >>>> By the sound of it, the meaning is confined to just VFs/SFs.
> >>>> What if the user wants to match packets coming from PFs?
> >>>>
> >>> It should be "wire_orig".
> >>
> >> Forgive me, but that does not sound correct. Say, there's an
> >> application and it has a PF plugged into it: ethdev index 0. And the
> >> application transmits packets using rte_eth_tx_burst() from that port.
> >> You say that these packets can be matched via "wire_orig".
> >> But they do not come from the wire. They come from PF...
> > Hmm. My mistake.
> > This may highly depend on PMD implementation. Basically, PFs' traffic
> > may contain "from wire"/"wire_orig" and '"from local"/"vport_orig".
> > That' why we emphasize it' optional for PMD.
> >>
> >>>>>
> >>>>> There are two possible approaches for providing the hints.
> >>>>> Using IPv4 as an example:
> >>>>> 1. Use pattern item in both template table and flow rules.
> >>>>>
> >>>>> 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 flow rule even if it's
> >>>>> just a hint. No value to match because matching is already done by
> >>>>> IPv4 item.
> >>>>
> >>>> Why no value to match on? How does it prevent rogue tenants from
> >>>> spoofing network headers? If the application receives a packet on a
> >>>> particular vport's representor, then it may strictly specify item
> >>>> represented_port pointing to that vport so that only packets from
> >>>> that vport
> >> match.
> >>>>
> >>>> Why isn't security a consideration?
> >>>>
> >>> There is some misunderstanding here. "ANY_VPORT" is the approach
> >>> (new
> >> matching item without value) suggested by you.
> >> I'm not talking about ANY_VPORT in this particular paragraph.
> >>
> >> There's item "represented_port" mentioned over there. I'm just asking
> >> about this "already done by IPv4 item" bit. Yes, it matches on the
> >> header but not on the true origin of the packet (the logical port of
> >> the NIC). If the app knows which logical port the packet ingresses
> >> the NIC, why not match on it for security?
> >>
> > Hint is not a matching and it implies how to manage underlayer steering
> resource.
> > If "vport_orig" is present, PMD will only apply the steering logic to vport
> traffic.
> > The resource is allocated in the async table before each rule. Already cover
> security considerations.
> > Matching on "represented_port" needs to program each rule, considering a
> port range like index "5-10".
> > Hint tells PMD only to take care of traffic from vport regardless the port
> index.
> >
> >>> I was explaining we need to apply it to each flow rule even if it's
> >>> only a flag
> >> and no value.
> >>
> >> That's clear. But PSB.
> >>
> >>>>>
> >>>>> 2. Add special flags into table_attr.
> >>>>>
> >>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>>>
> >>>>> Approach 1 needs to specify the pattern in each flow rule which
> >>>>> wastes memory and is not user friendly.
> >>>>
> >>>> What if the user has to insert a group of rules which not only have
> >>>> the same set of match masks but also share exactly the same match
> >>>> spec values for a limited subset of network items (for example,
> >>>> those of an encap. header)? This way, a subset of network item
> >>>> specs can remain fixed across many rules. Does that count as wasting
> memory?
> >>>>
> >>> Per my understanding, you are talking "multiple spec and mask mixing".
> >>
> >> Say, there's a group of rules, and each of them matches on exactly
> >> the same encap. header (the same in all rules), but different internal match
> field values.
> >> So, why don't these "fixed"
> >> encap. header items deserve being "optimised" somehow, the same way
> >> as this "wire_orig" does?
> > We are back to original point. Async approach is trying to pre-allocate
> resources and speed up the insertion.
> > Resource is allocated in async table stage and we only have mask
> information.
> > In each rule, the matching value passes in. I guess you are saying to optimize
> per different matching values, right?
> > This needs dynamic calculations per each rule and wastes the resource in
> async table(table allocates resource for all possible values).
> >>
> >> If the application knows for sure that there will be packets with
> >> exactly the same encap. header, - that forms this special knowledge
> >> that can be used during init times to help the PMD optimise resource
> allocation.
> >> Isn't that so? Don't these items deserve some form of a "hint"?
> >>
> > It can deserve some kinds of "hint". But see above, these hints are per rule
> and resource allocation happens before rules.
>
> That's not per rule. Perhaps I should've worded it differently.
>
> Suppose, an application has to insert many flow rules, each of which has
> match items A and B. Item A not only has the same mask in *all* rule
> instances, but also the same spec.
> On the other hand, item B only has the same mask in all the rules, but its spec
> is different for each rule.
>
> In this example, the application can allocate a template with items A and B,
> but that only provides a fixed mask for them. And the application will HAVE to
> provide item A with exactly the same spec in all rule instances. The PMD, in
> turn, will HAVE to process this item every time, being unable to see it's in fact
> the same at all times.
>
> To me, this sounds very similar to how you described the need to always
> provide item ANY_VPORT in each rule thus facing some waste of memory and
> parsing difficulties.
>
> If the application knows that a certain item (or a certain fraction of items) is
> going to be entirely the same (mask +
> spec) across all the rules, why shouldn't it be able to express this as a hint to
> the PMD? Why shouldn't it be able to avoid providing such items in every new
> flow rule instance? The same way the "vport_orig" works.
>
> I'm not demanding that you re-implement or re-design this.
> Just trying to find out whether such a problem can indeed be acknowledged.
> Or has it been solved already? If not, then perhaps it pays to just discuss
> whether solving it can be combined with this "vport_orig" solution.
>
> What do you think? What do others think?
>
> >>> We provide a hint in this patch and no assumption on the matching
> patterns.
> >>
> >> So I understand. My point is, certain portions of matching patterns
> >> may be "fixed" = entirely the same (masks and specs) in all rules of
> >> a table. Why not give PMD a "hint" about them, too?
> >>
> >>> I think matching pattern is totally controlled by application layer.
> >>
> >> So is the "direction" spec: the app layer has item represented_port
> >> to control that. But, still, we're here to discuss a hint for that.
> >> Why does the new hint aim exclusively at optimising out this specific
> >> meta item? Why isn't it possible to care about a generic portion of
> >> "know in advance" all-the-same items?
> > " generic portion of know in advance" is some still kind of dynamic approach,
> right?
> > Imagine a situation. DPDK has 10 VFs, each VF may have different VxLAN
> encap headers.
> > This hint approach can work for 10 VFs once.
> > In public cloud deployments, each VF/SF may map to different users, but
> underlay is almost same(GRE/VxLAN... differ in filed values).
> >>
> >>> "wasting memory " because your approach needs to scatter in each
> >>> rule
> >> while this patch only needs to set table_attr once.
> >>> No relation with matching patter totally.
> >>
> >> The slight problem with your proposal is that for some reason only
> >> one type of a match criterion deserves a hint moved to the attrs.
> >> Whilst in reality the applicaction may know in advance that certain
> >> subsets of items will not only have the same masks in all rules but
> >> also totally the same specs. If that is a valid use case, why doesn't
> >> it deserve the same (more
> >> generic) optimisation / a hint? Just wondering...
> >> Or has that been addressed already somehow?
> >>
> > Believe me, the hint helps us to save significant resources already.
>
> I'm not arguing it can be helpful. You're working round the clock to offer a
> solution, - that's fine and is greatly appreciated.
> But what I'm trying to say is that it looks like the problem might manifest itself
> for other type of knowledge that also may deserve a hint. Hence the questions.
> Hence the offer to think of covering more match criteria, not just net/vport.
>
> > Per my view, your proposal is totally valid in sync approach, but
> > please check my response, Async is trying to allocate resources in advance
> and speed up insertion ASAP.
>
> So if it's valid in sync approach, then why can't it be valid in the async one?
> And I guess it can reflect positively on the insertion rate, too. Why limit this
> "hint" approach to just one aspect then?
>
> I'm sure we're close to understanding each other here.
> Yes, "orig_vport" is just a one-bit knowledge, and seems innocent to add as a
> hint, but why not make it possible to have a hint for an arbitrary set of always-
> the-same match criteria?
>
> In this case, nobody will ever argue of whether a hint is a match criterion or if
> it's not. It will be quite a generic instrument, potentially useful to vendors.
> I'm afraid I can't think of an immediate example of such usefulness, but at
> least it will appear as generic as possible from the API perspective.
>
> >>>> If yes, then the problem does not concern just a single pair of
> >>>> attributes, but rather deserves a more versatile solution like some
> >>>> sort of indirect grouping of constant item specs.
> >>>> Have you considered such options?
> >>> See above.
> >>>>
> >>>>> This patch takes the 2nd approach and introduces one new member
> >>>>> "specialize" into rte_flow_table_attr to indicate possible flow
> >>>>> table optimization.
> >>>>
> >>>> The name "specialize" might have some drawbacks:
> >>>> - spelling difference (specialise/specialize)
> >>>> - in grep output, will mix with flows' "spec"
> >>>> - quite long
> >>>> - not a noun
> >>>>
> >>>> Why not "scope"? Or something like that?
> >>>>
> >>> It means special optimization to PMD. "scope" is more rogue.
> >>
> >> Why is it "rogue"? Scope is something limiting the point of view.
> >> So are the suggested flags. Flag "wire_origin" (or whatever it can be
> >> named
> >> eventually) limits the scope of matching. No?
> >>
> > Hint won't interfere with matching. It has no knowledge of matching.
>
> Does specifying "orig_vport" actually provide a *choice* for the packet origin?
> Does it filter out everything else? If yes, then, alas, it *is* matching. Because
> matching is choosing something of interest. Let's face it.
>
> As I said above, I do acknowledge the fact that, for some vendors, this match
> criterion, internally goes to a different HW aspect that is separate from
> matching on, for example, IPv4 addresses.
> That's OK. But for some vendors, this might be just a regular match criterion
> internally. So let's describe it with care.
>
> > Instead, it only controls matching resources. "wire_orig" tells PMD to
> allocate HW resource for traffic from wire only.
>
> If it controls "matching resources", it's indeed affiliated with matching then.
> Look. When the application creates a template, it tells the PMD that it is going
> to match on this, this and this.
> Masks... No exact values; they will come at a later stage. But, with this
> "wire_orig", the application tells the PMD that not only it will match on
> *some* "direction", but it actually provides a SPEC for that. If it indicates bit
> "wire_orig", that equals to setting a "mask" for the "direction enum"
> AND a "spec" (WIRE). Isn't that the case?
>
> If it is, then please see my above concerns about possibly having similar need
> to provide exact-spec hints for other items as well.
>
> > Then traffic from vport is sliently ignored. Hint doesn't know what are
> matched and how many fields are involves.
> >>>>>
> >>>>> By default, there is no hint, so the behavior of the transfer
> >>>>> domain doesn't change.
> >>>>> There is no guarantee that the hint will be used by the PMD.
> >>>>>
> >>>>> 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.
> >>>>> v5: Change specialize type to uint32_t.
> >>>>> v6: Change the flags to macros and re-construct the commit log.
> >>>>> v7: Fix build failure.
> >>>>> ---
> >>>>> 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 | 28 +++++++++++++++++++++
> >>>>> 4 files changed, 71 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/app/test-pmd/cmdline_flow.c
> >>>>> b/app/test-pmd/cmdline_flow.c index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
> >>>>
> >>>> Silently ignoring the field does not sit well with the
> >>>> application's possible intent to drop represented_port match from the
> patterns.
> >>>> From my point of view, if the application sets this attribute, it
> >>>> believes it can rely on it, that is, packets coming from host won't
> >>>> match if the attribute asks to match network only, for instance.
> >>>> Has this
> >> been considered?
> >>>>
> >>>>> +
> >>>>> +If a table is specialized, the application should make sure the
> >>>>> +rules comply with the table attribute.
> >>>>
> >>>> How does the application enforce that? I would appreciate you explain it.
> >>>>
> >>>>> +
> >>>>> 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..c27b48c5c1 100644
> >>>>> --- a/lib/ethdev/rte_flow.h
> >>>>> +++ b/lib/ethdev/rte_flow.h
> >>>>> @@ -5186,6 +5186,29 @@
> rte_flow_actions_template_destroy(uint16_t
> >>>>> port_id, */ struct rte_flow_template_table;
> >>>>>
> >>>>> +/**@{@name Special optional flags for template table attribute
> >>>>> + * Each bit is a hint for table specialization,
> >>>>> + * offering a potential optimization at PMD layer.
> >>>>> + * PMD can ignore the unsupported bits silently.
> >>>>> + */
> >>>>> +/**
> >>>>> + * 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.
> >>>>
> >>>> You intended to spell "criterion", I take it. And still, it *is* a match
> criterion.
> >>>> I'm not denying the possible need to have this criterion at the
> >>>> earliest processing stage. That might be OK, but I still have a
> >>>> hunch that this is too specific.
> >>>> Please see my comment above about wasting memory.
> >>>> I guess this type of criterion is not the only one that may need to
> >>>> be provided as a "hint".
> >>>>
> >>>>> + * Flow rules which match non-wire originated traffic will be
> >>>>> + missed
> >>>>> + * if the hint is supported.
> >>>>
> >>>> And what if it's unsupported? Is it indeed OK to silently ignore it?
> >>>>
> >>>>> + */
> >>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
> >>>> RTE_BIT32(0)
> >>>>
> >>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
> >>>>
> >>>> To me, TRANSFER looks redundant as this bit is already supposed to
> >>>> be ticked in the "struct rte_flow_attr flow_attr" field of the
> >>>> "struct rte_flow_template_table_attr".
> >>>>
> >>>>> +/**
> >>>>> + * Specialize table for transfer flows which come only from vport
> >>>>> +(e.g. VF,
> >>>>> SF).
> >>>>
> >>>> And PF?
> >>>>
> >>>>> + * 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.
> >>>>> + */
> >>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
> >>>> RTE_BIT32(1)
> >>>>
> >>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
> >>>>
> >>>>> +/**@}*/
> >>>>> +
> >>>>> /**
> >>>>> * @warning
> >>>>> * @b EXPERIMENTAL: this API may change without prior notice.
> >>>>> @@ -5201,6 +5224,11 @@ 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.
> >>>>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
> >>>>> + */
> >>>>> + uint32_t specialize;
> >>>>
> >>>> Why not "scope" or something?
> >>>>
> >>>>> };
> >>>>>
> >>>>> /**
> >>>>> --
> >>>>> 2.27.0
> >>>>>
> >>>>
> >>>> Thank you.
> >>>
> >
>
> Thank you.
Hi Rongwei,
OK, I hear ya. Thanks for persevering.
I still hope community will comment on the possibility to
provide a hint mechanism for always-the-same match items,
with the perspective of becoming more versatile. Other
than that, your current patch might be OK, but, again,
I think other reviewers' comments (if any) shall
be addressed. But no strong objections from me.
By the way, for this "specialise" field, in your opinion,
which extra flags could emerge in future / would be nice
to have? I mean, is there any concept of what can be
added to this field's namespace and what can't be?
Thank you.
On Tue, 31 Jan 2023, Rongwei Liu wrote:
> HI Ivan
>
> BR
> Rongwei
>
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@arknetworks.am>
>> Sent: Tuesday, January 31, 2023 07:00
>> To: Rongwei Liu <rongweil@nvidia.com>
>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
>> <rasland@nvidia.com>
>> Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer
>> table
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Rongwei,
>>
>> Thanks for the professional attitude.
>> Hope this discussion gets us on the
>> same page. Please see below.
> Thanks for the suggestion and comments. Hope everything goes well.
>>
>> On Mon, 30 Jan 2023, Rongwei Liu wrote:
>>
>>> HI Ivan
>>>
>>> BR
>>> Rongwei
>>>
>>>> -----Original Message-----
>>>> From: Ivan Malov <ivan.malov@arknetworks.am>
>>>> Sent: Monday, January 30, 2023 15:40
>>>> To: Rongwei Liu <rongweil@nvidia.com>
>>>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>>>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
>>>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
>>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>>>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
>>>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
>>>> <rasland@nvidia.com>
>>>> Subject: RE: [PATCH v7] ethdev: add special flags when creating async
>>>> transfer table
>>>>
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> Hi Rongwei,
>>>>
>>>> For my responses, PSB.
>>>>
>>>> By the way, now you mention things like wasting memory and insertion
>>>> optimisastions, are there any comparative figures to see the effect
>>>> of this hint on insertion performance / memory footprint?
>>>> Some "before" / "after" examples would really be helpful.
>>>>
>>> Good to hear we reach agreement almost.
>>
>> Very well.
>>
>> The key point here is that one may agree that some optimisations are indeed
>> needed, yes. I don't deny the fact that some vendors might have issues with
>> how the API maps to the HW capabilities.
>> Yes, some undesirable resource overhead shall be avoided, but the high level
>> hints for that have to be designed with care.
>>
> Totally agree. That' why we emphasize "optional for PMD" and "application should take care of hint"
>>> First, the hint has nothing related to matching, only affects PMD resource
>> management.
>>
>> You say "PMD resource management". For the flow management, that's
>> mostly vendor-specific, I take it. Let me explain. The application, for instance,
>> can control the number of Tx descriptors in the queue during setup stage.
>> Tx descriptors are a common type of HW resource, hence the explicit control
>> for it available to applications. For flow library, it's not like that. Different
>> vendors have different "underlayer"
>> representations, they may vary drastically.
> The resource I mentioned is about "steering logic" not SW datapath.
> With flow rules offloading, hardware should store the steering logic in its reachable memory no matter embedded in or mapping from host OS.
>>
>> I take it, in the case of the HW you're working with, this hint indeed maps to
>> something that is entirely resource-related and which does not belong in this
>> specific vendor's match criteria. I 100% understand that, in your case, these
>> are separate. But the point is that, on the high-level programming level
>> (vendor-neutral), such a hint is in fact a match criterion. Because it tells the
>> driver to limit the scope of matching to just "from net"/"from vport", the same
>> way other metadata items do (represented_port).
>> The only difference is that it refers to a group of unspecified ports which have
>> something in common.
>>
> " a group of unspecified ports" means dynamic and flexible, right. IMO it's valid and fits sync flow perfectly.
> But in async, when allocating resources (table creation), the group info is still unknown. We don't want to scatter it into each rule insertion.
>> So, although I don't strongly object having some hints like this one in the
>> generic API, I nevertheless disagree with describing this as just "resource-
>> specific" and not being a match criterion. It's just not always the case. It might
>> not be valid for *all* NIC vendors.
>>
> Agree, not valid for *all* NIC vendors.
>>> In my local test, it can save around 50% memory in the VxLAN encap/decap
>> example case.
>>
>> Forgive me in case this has been already discussed; where's that memory?
>> I mean, is it some sort of general-purpose memory? Or some HW-specific
>> table capacity overhead? I'm trying to understand how the feature will be
>> useful to other vendors, or how common this problem is.
>>
> See above. HW always needs memory to store offloaded rules no matter embedded in chip or borrowed from OS.
>>> Insertion rate has very very few improvements.
>>>> After all, I'm not objecting this patch. But I believe that other reviewers'
>>>> concerns should nevertheless be addressed anyway.
>>> Let me try to show the hint is useful.
>>>>
>>>> On Mon, 30 Jan 2023, Rongwei Liu wrote:
>>>>
>>>>> Hi Ivan,
>>>>>
>>>>> BR
>>>>> Rongwei
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ivan Malov <ivan.malov@arknetworks.am>
>>>>>> Sent: Monday, January 30, 2023 08:00
>>>>>> To: Rongwei Liu <rongweil@nvidia.com>
>>>>>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
>>>>>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
>>>>>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
>>>>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>>>>>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
>>>>>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
>>>>>> <rasland@nvidia.com>
>>>>>> Subject: Re: [PATCH v7] ethdev: add special flags when creating
>>>>>> async transfer table
>>>>>>
>>>>>> External email: Use caution opening links or attachments
>>>>>>
>>>>>>
>>>>>> Hi Rongwei,
>>>>>>
>>>>>> Thanks for persevering. I have no strong opinion, but, at least,
>>>>>> the fact that the new flags are no longer meant for use in
>>>>>> rte_flow_attr, which is clearly not the right place for such, is an
>> improvement.
>>>>>>
>>>>> Thanks for the suggestion, move it to rte_flow_table_attr now and it'
>>>> dedicated to async API.
>>>>>> However, let's take a closer look at the current patch, shall we?
>>>>>>
>>>>>> But, before we get to that, I'd like to kindly request that you
>>>>>> provide a more concrete example of how this feature is supposed to
>>>>>> be used. Are there some real-life application examples?
>>>>>>
>>>>> Sure.
>>>>>> Also, to me, it's still unclear how an application can obtain the
>>>>>> knowledge of this hint in the first instance. For example, can Open
>>>>>> vSwitch somehow tell ethdevs representing physical ports from ones
>>>>>> representing "vports" (host endpoints)?
>>>>>> How does it know which attribute to specify?
>>>>>>
>>>>> Hint should be initiated by application and application knows it'
>>>>> traffic
>>>> pattern which highly relates to deployment.
>>>>> Let' use VxLAN encap/decap as an example:
>>>>> 1. Traffic from wire should be VxLAN pattern and do the decap, then
>>>>> send to
>>>> different vports.
>>>>> flow pattern_template 0 create transfer relaxed no
>>>>> pattern_template_id
>>>>> 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp /
>>>>> vxlan / tag index is 0 data is 0x33 / end flow actions_template 0
>>>>> create transfer actions_template_id 4 template raw_decap index 0 /
>>>>> represented_port ethdev_port_id 1 / end mask raw_decap index 0 /
>>>>> represented_port ethdev_port_id 1 / end flow template_table 0 create
>>>>> group 1 priority 0 transfer wire_orig table_id 4 rules_number 128
>>>>> pattern_template 4 actions_template 4
>>>>>
>>>>> 2. Traffic from vports should be encap with different VxLAN header
>>>>> and send
>>>> to wire.
>>>>> flow actions_template 1 create transfer actions_template_id 5
>>>>> template raw_encap index 0 / represented_port ethdev_port_id 0 / end
>>>>> mask raw_encap index 0 / represented_port ethdev_port_id 0 / end
>>>>> flow template_table 0 create group 1 priority 0 transfer vport_orig
>>>>> table_id 5 rules_number 128 pattern_template 4 actions_template 5
>>>>>
>>>>>> For the rest of my notes, PSB.
>>>>>>
>>>>>> On Mon, 14 Nov 2022, Rongwei Liu wrote:
>>>>>>
>>>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>>>> then optimization can be done via allocation of this table.
>>>>>>
>>>>>> This wording might confuse readers. Consider rephrasing it, please:
>>>>>> If multiple flow rules share a common set of match masks, then they
>>>>>> might belong in a flow table which can be pre-allocated.
>>>>>>
>>>>>>> Such optimization is possible only if the application gives a hint
>>>>>>> about its usage of the table during initial configuration.
>>>>>>>
>>>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>>>> which may correspond to two kinds of underlayer resources.
>>>>>>
>>>>>> Why name it a "vport"? Why not "host"?
>>>>>>
>>>>>> host = packets generated by any of the host's "vport"s wire =
>>>>>> packets arriving at the NIC from the network
>>>>> Vport is "virtual port" for short and contains "VF/SF" for now.
>>>>> Per my thoughts, it' clearer and maps to DPDK port probing/management.
>>>>
>>>> I understand that "host" might not be a brilliant name.
>>>>
>>>> If "vport" stands for every port of the NIC that is not a network
>>>> port, then this name might be OK to me, but why doesn't it cover PFs?
>>>> A PF is clearly not a network / physical port. Why just VF/SF then? Where
>> does that "for now"
>>>> decision come from? Just wondering.
>>>>
>>> "For now" stands for my understanding. DPDK is always in evolution, right?
>>> You are right, PF should be included in 'vport" concept.
>>>>>>
>>>>>>> That's why the first two hints introduced in this patch are about
>>>>>>> wire and vport traffic specialization.
>>>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>>>> traffic initiated from VF/SF.
>>>>>>
>>>>>> By the sound of it, the meaning is confined to just VFs/SFs.
>>>>>> What if the user wants to match packets coming from PFs?
>>>>>>
>>>>> It should be "wire_orig".
>>>>
>>>> Forgive me, but that does not sound correct. Say, there's an
>>>> application and it has a PF plugged into it: ethdev index 0. And the
>>>> application transmits packets using rte_eth_tx_burst() from that port.
>>>> You say that these packets can be matched via "wire_orig".
>>>> But they do not come from the wire. They come from PF...
>>> Hmm. My mistake.
>>> This may highly depend on PMD implementation. Basically, PFs' traffic
>>> may contain "from wire"/"wire_orig" and '"from local"/"vport_orig".
>>> That' why we emphasize it' optional for PMD.
>>>>
>>>>>>>
>>>>>>> There are two possible approaches for providing the hints.
>>>>>>> Using IPv4 as an example:
>>>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>>>
>>>>>>> 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 flow rule even if it's
>>>>>>> just a hint. No value to match because matching is already done by
>>>>>>> IPv4 item.
>>>>>>
>>>>>> Why no value to match on? How does it prevent rogue tenants from
>>>>>> spoofing network headers? If the application receives a packet on a
>>>>>> particular vport's representor, then it may strictly specify item
>>>>>> represented_port pointing to that vport so that only packets from
>>>>>> that vport
>>>> match.
>>>>>>
>>>>>> Why isn't security a consideration?
>>>>>>
>>>>> There is some misunderstanding here. "ANY_VPORT" is the approach
>>>>> (new
>>>> matching item without value) suggested by you.
>>>> I'm not talking about ANY_VPORT in this particular paragraph.
>>>>
>>>> There's item "represented_port" mentioned over there. I'm just asking
>>>> about this "already done by IPv4 item" bit. Yes, it matches on the
>>>> header but not on the true origin of the packet (the logical port of
>>>> the NIC). If the app knows which logical port the packet ingresses
>>>> the NIC, why not match on it for security?
>>>>
>>> Hint is not a matching and it implies how to manage underlayer steering
>> resource.
>>> If "vport_orig" is present, PMD will only apply the steering logic to vport
>> traffic.
>>> The resource is allocated in the async table before each rule. Already cover
>> security considerations.
>>> Matching on "represented_port" needs to program each rule, considering a
>> port range like index "5-10".
>>> Hint tells PMD only to take care of traffic from vport regardless the port
>> index.
>>>
>>>>> I was explaining we need to apply it to each flow rule even if it's
>>>>> only a flag
>>>> and no value.
>>>>
>>>> That's clear. But PSB.
>>>>
>>>>>>>
>>>>>>> 2. Add special flags into table_attr.
>>>>>>>
>>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>>>
>>>>>>> Approach 1 needs to specify the pattern in each flow rule which
>>>>>>> wastes memory and is not user friendly.
>>>>>>
>>>>>> What if the user has to insert a group of rules which not only have
>>>>>> the same set of match masks but also share exactly the same match
>>>>>> spec values for a limited subset of network items (for example,
>>>>>> those of an encap. header)? This way, a subset of network item
>>>>>> specs can remain fixed across many rules. Does that count as wasting
>> memory?
>>>>>>
>>>>> Per my understanding, you are talking "multiple spec and mask mixing".
>>>>
>>>> Say, there's a group of rules, and each of them matches on exactly
>>>> the same encap. header (the same in all rules), but different internal match
>> field values.
>>>> So, why don't these "fixed"
>>>> encap. header items deserve being "optimised" somehow, the same way
>>>> as this "wire_orig" does?
>>> We are back to original point. Async approach is trying to pre-allocate
>> resources and speed up the insertion.
>>> Resource is allocated in async table stage and we only have mask
>> information.
>>> In each rule, the matching value passes in. I guess you are saying to optimize
>> per different matching values, right?
>>> This needs dynamic calculations per each rule and wastes the resource in
>> async table(table allocates resource for all possible values).
>>>>
>>>> If the application knows for sure that there will be packets with
>>>> exactly the same encap. header, - that forms this special knowledge
>>>> that can be used during init times to help the PMD optimise resource
>> allocation.
>>>> Isn't that so? Don't these items deserve some form of a "hint"?
>>>>
>>> It can deserve some kinds of "hint". But see above, these hints are per rule
>> and resource allocation happens before rules.
>>
>> That's not per rule. Perhaps I should've worded it differently.
>>
>> Suppose, an application has to insert many flow rules, each of which has
>> match items A and B. Item A not only has the same mask in *all* rule
>> instances, but also the same spec.
>> On the other hand, item B only has the same mask in all the rules, but its spec
>> is different for each rule.
>>
>> In this example, the application can allocate a template with items A and B,
>> but that only provides a fixed mask for them. And the application will HAVE to
>> provide item A with exactly the same spec in all rule instances. The PMD, in
>> turn, will HAVE to process this item every time, being unable to see it's in fact
>> the same at all times.
>>
>> To me, this sounds very similar to how you described the need to always
>> provide item ANY_VPORT in each rule thus facing some waste of memory and
>> parsing difficulties.
>>
>> If the application knows that a certain item (or a certain fraction of items) is
>> going to be entirely the same (mask +
>> spec) across all the rules, why shouldn't it be able to express this as a hint to
>> the PMD? Why shouldn't it be able to avoid providing such items in every new
>> flow rule instance? The same way the "vport_orig" works.
>>
>> I'm not demanding that you re-implement or re-design this.
>> Just trying to find out whether such a problem can indeed be acknowledged.
>> Or has it been solved already? If not, then perhaps it pays to just discuss
>> whether solving it can be combined with this "vport_orig" solution.
>>
>> What do you think? What do others think?
>>
>>>>> We provide a hint in this patch and no assumption on the matching
>> patterns.
>>>>
>>>> So I understand. My point is, certain portions of matching patterns
>>>> may be "fixed" = entirely the same (masks and specs) in all rules of
>>>> a table. Why not give PMD a "hint" about them, too?
>>>>
>>>>> I think matching pattern is totally controlled by application layer.
>>>>
>>>> So is the "direction" spec: the app layer has item represented_port
>>>> to control that. But, still, we're here to discuss a hint for that.
>>>> Why does the new hint aim exclusively at optimising out this specific
>>>> meta item? Why isn't it possible to care about a generic portion of
>>>> "know in advance" all-the-same items?
>>> " generic portion of know in advance" is some still kind of dynamic approach,
>> right?
>>> Imagine a situation. DPDK has 10 VFs, each VF may have different VxLAN
>> encap headers.
>>> This hint approach can work for 10 VFs once.
>>> In public cloud deployments, each VF/SF may map to different users, but
>> underlay is almost same(GRE/VxLAN... differ in filed values).
>>>>
>>>>> "wasting memory " because your approach needs to scatter in each
>>>>> rule
>>>> while this patch only needs to set table_attr once.
>>>>> No relation with matching patter totally.
>>>>
>>>> The slight problem with your proposal is that for some reason only
>>>> one type of a match criterion deserves a hint moved to the attrs.
>>>> Whilst in reality the applicaction may know in advance that certain
>>>> subsets of items will not only have the same masks in all rules but
>>>> also totally the same specs. If that is a valid use case, why doesn't
>>>> it deserve the same (more
>>>> generic) optimisation / a hint? Just wondering...
>>>> Or has that been addressed already somehow?
>>>>
>>> Believe me, the hint helps us to save significant resources already.
>>
>> I'm not arguing it can be helpful. You're working round the clock to offer a
>> solution, - that's fine and is greatly appreciated.
>> But what I'm trying to say is that it looks like the problem might manifest itself
>> for other type of knowledge that also may deserve a hint. Hence the questions.
>> Hence the offer to think of covering more match criteria, not just net/vport.
>>
>>> Per my view, your proposal is totally valid in sync approach, but
>>> please check my response, Async is trying to allocate resources in advance
>> and speed up insertion ASAP.
>>
>> So if it's valid in sync approach, then why can't it be valid in the async one?
>> And I guess it can reflect positively on the insertion rate, too. Why limit this
>> "hint" approach to just one aspect then?
>>
>> I'm sure we're close to understanding each other here.
>> Yes, "orig_vport" is just a one-bit knowledge, and seems innocent to add as a
>> hint, but why not make it possible to have a hint for an arbitrary set of always-
>> the-same match criteria?
>>
>> In this case, nobody will ever argue of whether a hint is a match criterion or if
>> it's not. It will be quite a generic instrument, potentially useful to vendors.
>> I'm afraid I can't think of an immediate example of such usefulness, but at
>> least it will appear as generic as possible from the API perspective.
>>
>>>>>> If yes, then the problem does not concern just a single pair of
>>>>>> attributes, but rather deserves a more versatile solution like some
>>>>>> sort of indirect grouping of constant item specs.
>>>>>> Have you considered such options?
>>>>> See above.
>>>>>>
>>>>>>> This patch takes the 2nd approach and introduces one new member
>>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow
>>>>>>> table optimization.
>>>>>>
>>>>>> The name "specialize" might have some drawbacks:
>>>>>> - spelling difference (specialise/specialize)
>>>>>> - in grep output, will mix with flows' "spec"
>>>>>> - quite long
>>>>>> - not a noun
>>>>>>
>>>>>> Why not "scope"? Or something like that?
>>>>>>
>>>>> It means special optimization to PMD. "scope" is more rogue.
>>>>
>>>> Why is it "rogue"? Scope is something limiting the point of view.
>>>> So are the suggested flags. Flag "wire_origin" (or whatever it can be
>>>> named
>>>> eventually) limits the scope of matching. No?
>>>>
>>> Hint won't interfere with matching. It has no knowledge of matching.
>>
>> Does specifying "orig_vport" actually provide a *choice* for the packet origin?
>> Does it filter out everything else? If yes, then, alas, it *is* matching. Because
>> matching is choosing something of interest. Let's face it.
>>
>> As I said above, I do acknowledge the fact that, for some vendors, this match
>> criterion, internally goes to a different HW aspect that is separate from
>> matching on, for example, IPv4 addresses.
>> That's OK. But for some vendors, this might be just a regular match criterion
>> internally. So let's describe it with care.
>>
>>> Instead, it only controls matching resources. "wire_orig" tells PMD to
>> allocate HW resource for traffic from wire only.
>>
>> If it controls "matching resources", it's indeed affiliated with matching then.
>> Look. When the application creates a template, it tells the PMD that it is going
>> to match on this, this and this.
>> Masks... No exact values; they will come at a later stage. But, with this
>> "wire_orig", the application tells the PMD that not only it will match on
>> *some* "direction", but it actually provides a SPEC for that. If it indicates bit
>> "wire_orig", that equals to setting a "mask" for the "direction enum"
>> AND a "spec" (WIRE). Isn't that the case?
>>
>> If it is, then please see my above concerns about possibly having similar need
>> to provide exact-spec hints for other items as well.
>>
>>> Then traffic from vport is sliently ignored. Hint doesn't know what are
>> matched and how many fields are involves.
>>>>>>>
>>>>>>> By default, there is no hint, so the behavior of the transfer
>>>>>>> domain doesn't change.
>>>>>>> There is no guarantee that the hint will be used by the PMD.
>>>>>>>
>>>>>>> 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.
>>>>>>> v5: Change specialize type to uint32_t.
>>>>>>> v6: Change the flags to macros and re-construct the commit log.
>>>>>>> v7: Fix build failure.
>>>>>>> ---
>>>>>>> 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 | 28 +++++++++++++++++++++
>>>>>>> 4 files changed, 71 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/app/test-pmd/cmdline_flow.c
>>>>>>> b/app/test-pmd/cmdline_flow.c index 88108498e0..62197f2618 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
>>>>>>
>>>>>> Silently ignoring the field does not sit well with the
>>>>>> application's possible intent to drop represented_port match from the
>> patterns.
>>>>>> From my point of view, if the application sets this attribute, it
>>>>>> believes it can rely on it, that is, packets coming from host won't
>>>>>> match if the attribute asks to match network only, for instance.
>>>>>> Has this
>>>> been considered?
>>>>>>
>>>>>>> +
>>>>>>> +If a table is specialized, the application should make sure the
>>>>>>> +rules comply with the table attribute.
>>>>>>
>>>>>> How does the application enforce that? I would appreciate you explain it.
>>>>>>
>>>>>>> +
>>>>>>> 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..c27b48c5c1 100644
>>>>>>> --- a/lib/ethdev/rte_flow.h
>>>>>>> +++ b/lib/ethdev/rte_flow.h
>>>>>>> @@ -5186,6 +5186,29 @@
>> rte_flow_actions_template_destroy(uint16_t
>>>>>>> port_id, */ struct rte_flow_template_table;
>>>>>>>
>>>>>>> +/**@{@name Special optional flags for template table attribute
>>>>>>> + * Each bit is a hint for table specialization,
>>>>>>> + * offering a potential optimization at PMD layer.
>>>>>>> + * PMD can ignore the unsupported bits silently.
>>>>>>> + */
>>>>>>> +/**
>>>>>>> + * 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.
>>>>>>
>>>>>> You intended to spell "criterion", I take it. And still, it *is* a match
>> criterion.
>>>>>> I'm not denying the possible need to have this criterion at the
>>>>>> earliest processing stage. That might be OK, but I still have a
>>>>>> hunch that this is too specific.
>>>>>> Please see my comment above about wasting memory.
>>>>>> I guess this type of criterion is not the only one that may need to
>>>>>> be provided as a "hint".
>>>>>>
>>>>>>> + * Flow rules which match non-wire originated traffic will be
>>>>>>> + missed
>>>>>>> + * if the hint is supported.
>>>>>>
>>>>>> And what if it's unsupported? Is it indeed OK to silently ignore it?
>>>>>>
>>>>>>> + */
>>>>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
>>>>>> RTE_BIT32(0)
>>>>>>
>>>>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
>>>>>>
>>>>>> To me, TRANSFER looks redundant as this bit is already supposed to
>>>>>> be ticked in the "struct rte_flow_attr flow_attr" field of the
>>>>>> "struct rte_flow_template_table_attr".
>>>>>>
>>>>>>> +/**
>>>>>>> + * Specialize table for transfer flows which come only from vport
>>>>>>> +(e.g. VF,
>>>>>>> SF).
>>>>>>
>>>>>> And PF?
>>>>>>
>>>>>>> + * 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.
>>>>>>> + */
>>>>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
>>>>>> RTE_BIT32(1)
>>>>>>
>>>>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
>>>>>>
>>>>>>> +/**@}*/
>>>>>>> +
>>>>>>> /**
>>>>>>> * @warning
>>>>>>> * @b EXPERIMENTAL: this API may change without prior notice.
>>>>>>> @@ -5201,6 +5224,11 @@ 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.
>>>>>>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
>>>>>>> + */
>>>>>>> + uint32_t specialize;
>>>>>>
>>>>>> Why not "scope" or something?
>>>>>>
>>>>>>> };
>>>>>>>
>>>>>>> /**
>>>>>>> --
>>>>>>> 2.27.0
>>>>>>>
>>>>>>
>>>>>> Thank you.
>>>>>
>>>
>>
>> Thank you.
>
HI Ivan:
BR
Rongwei
> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Tuesday, January 31, 2023 13:30
> To: Rongwei Liu <rongweil@nvidia.com>
> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> <rasland@nvidia.com>
> Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer
> table
>
> External email: Use caution opening links or attachments
>
>
> Hi Rongwei,
>
> OK, I hear ya. Thanks for persevering.
>
> I still hope community will comment on the possibility to provide a hint
> mechanism for always-the-same match items, with the perspective of
> becoming more versatile. Other than that, your current patch might be OK,
> but, again, I think other reviewers' comments (if any) shall be addressed. But
> no strong objections from me.
>
Good to hear the confirmation from you.
> By the way, for this "specialise" field, in your opinion, which extra flags could
> emerge in future / would be nice to have? I mean, is there any concept of
> what can be added to this field's namespace and what can't be?
>
IMO, flags that helps to reduce the resource or speed up insertion/deletion should be considered as a candidate.
> Thank you.
>
> On Tue, 31 Jan 2023, Rongwei Liu wrote:
>
> > HI Ivan
> >
> > BR
> > Rongwei
> >
> >> -----Original Message-----
> >> From: Ivan Malov <ivan.malov@arknetworks.am>
> >> Sent: Tuesday, January 31, 2023 07:00
> >> To: Rongwei Liu <rongweil@nvidia.com>
> >> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> >> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> >> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> >> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> >> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> >> <rasland@nvidia.com>
> >> Subject: RE: [PATCH v7] ethdev: add special flags when creating async
> >> transfer table
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Hi Rongwei,
> >>
> >> Thanks for the professional attitude.
> >> Hope this discussion gets us on the
> >> same page. Please see below.
> > Thanks for the suggestion and comments. Hope everything goes well.
> >>
> >> On Mon, 30 Jan 2023, Rongwei Liu wrote:
> >>
> >>> HI Ivan
> >>>
> >>> BR
> >>> Rongwei
> >>>
> >>>> -----Original Message-----
> >>>> From: Ivan Malov <ivan.malov@arknetworks.am>
> >>>> Sent: Monday, January 30, 2023 15:40
> >>>> To: Rongwei Liu <rongweil@nvidia.com>
> >>>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> >>>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-
> >>>> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Aman Singh
> >>>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> >>>> Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko
> >>>> <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org; Raslan Darawsheh
> >>>> <rasland@nvidia.com>
> >>>> Subject: RE: [PATCH v7] ethdev: add special flags when creating
> >>>> async transfer table
> >>>>
> >>>> External email: Use caution opening links or attachments
> >>>>
> >>>>
> >>>> Hi Rongwei,
> >>>>
> >>>> For my responses, PSB.
> >>>>
> >>>> By the way, now you mention things like wasting memory and
> >>>> insertion optimisastions, are there any comparative figures to see
> >>>> the effect of this hint on insertion performance / memory footprint?
> >>>> Some "before" / "after" examples would really be helpful.
> >>>>
> >>> Good to hear we reach agreement almost.
> >>
> >> Very well.
> >>
> >> The key point here is that one may agree that some optimisations are
> >> indeed needed, yes. I don't deny the fact that some vendors might
> >> have issues with how the API maps to the HW capabilities.
> >> Yes, some undesirable resource overhead shall be avoided, but the
> >> high level hints for that have to be designed with care.
> >>
> > Totally agree. That' why we emphasize "optional for PMD" and "application
> should take care of hint"
> >>> First, the hint has nothing related to matching, only affects PMD
> >>> resource
> >> management.
> >>
> >> You say "PMD resource management". For the flow management, that's
> >> mostly vendor-specific, I take it. Let me explain. The application,
> >> for instance, can control the number of Tx descriptors in the queue during
> setup stage.
> >> Tx descriptors are a common type of HW resource, hence the explicit
> >> control for it available to applications. For flow library, it's not
> >> like that. Different vendors have different "underlayer"
> >> representations, they may vary drastically.
> > The resource I mentioned is about "steering logic" not SW datapath.
> > With flow rules offloading, hardware should store the steering logic in its
> reachable memory no matter embedded in or mapping from host OS.
> >>
> >> I take it, in the case of the HW you're working with, this hint
> >> indeed maps to something that is entirely resource-related and which
> >> does not belong in this specific vendor's match criteria. I 100%
> >> understand that, in your case, these are separate. But the point is
> >> that, on the high-level programming level (vendor-neutral), such a
> >> hint is in fact a match criterion. Because it tells the driver to
> >> limit the scope of matching to just "from net"/"from vport", the same way
> other metadata items do (represented_port).
> >> The only difference is that it refers to a group of unspecified ports
> >> which have something in common.
> >>
> > " a group of unspecified ports" means dynamic and flexible, right. IMO it's
> valid and fits sync flow perfectly.
> > But in async, when allocating resources (table creation), the group info is
> still unknown. We don't want to scatter it into each rule insertion.
> >> So, although I don't strongly object having some hints like this one
> >> in the generic API, I nevertheless disagree with describing this as
> >> just "resource- specific" and not being a match criterion. It's just
> >> not always the case. It might not be valid for *all* NIC vendors.
> >>
> > Agree, not valid for *all* NIC vendors.
> >>> In my local test, it can save around 50% memory in the VxLAN
> >>> encap/decap
> >> example case.
> >>
> >> Forgive me in case this has been already discussed; where's that memory?
> >> I mean, is it some sort of general-purpose memory? Or some
> >> HW-specific table capacity overhead? I'm trying to understand how the
> >> feature will be useful to other vendors, or how common this problem is.
> >>
> > See above. HW always needs memory to store offloaded rules no matter
> embedded in chip or borrowed from OS.
> >>> Insertion rate has very very few improvements.
> >>>> After all, I'm not objecting this patch. But I believe that other reviewers'
> >>>> concerns should nevertheless be addressed anyway.
> >>> Let me try to show the hint is useful.
> >>>>
> >>>> On Mon, 30 Jan 2023, Rongwei Liu wrote:
> >>>>
> >>>>> Hi Ivan,
> >>>>>
> >>>>> BR
> >>>>> Rongwei
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Ivan Malov <ivan.malov@arknetworks.am>
> >>>>>> Sent: Monday, January 30, 2023 08:00
> >>>>>> To: Rongwei Liu <rongweil@nvidia.com>
> >>>>>> Cc: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> >>>>>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>;
> >>>>>> NBU-Contact- Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> >>>>>> Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> >>>>>> <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>;
> >>>>>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org;
> >>>>>> Raslan Darawsheh <rasland@nvidia.com>
> >>>>>> Subject: Re: [PATCH v7] ethdev: add special flags when creating
> >>>>>> async transfer table
> >>>>>>
> >>>>>> External email: Use caution opening links or attachments
> >>>>>>
> >>>>>>
> >>>>>> Hi Rongwei,
> >>>>>>
> >>>>>> Thanks for persevering. I have no strong opinion, but, at least,
> >>>>>> the fact that the new flags are no longer meant for use in
> >>>>>> rte_flow_attr, which is clearly not the right place for such, is
> >>>>>> an
> >> improvement.
> >>>>>>
> >>>>> Thanks for the suggestion, move it to rte_flow_table_attr now and it'
> >>>> dedicated to async API.
> >>>>>> However, let's take a closer look at the current patch, shall we?
> >>>>>>
> >>>>>> But, before we get to that, I'd like to kindly request that you
> >>>>>> provide a more concrete example of how this feature is supposed
> >>>>>> to be used. Are there some real-life application examples?
> >>>>>>
> >>>>> Sure.
> >>>>>> Also, to me, it's still unclear how an application can obtain the
> >>>>>> knowledge of this hint in the first instance. For example, can
> >>>>>> Open vSwitch somehow tell ethdevs representing physical ports
> >>>>>> from ones representing "vports" (host endpoints)?
> >>>>>> How does it know which attribute to specify?
> >>>>>>
> >>>>> Hint should be initiated by application and application knows it'
> >>>>> traffic
> >>>> pattern which highly relates to deployment.
> >>>>> Let' use VxLAN encap/decap as an example:
> >>>>> 1. Traffic from wire should be VxLAN pattern and do the decap,
> >>>>> then send to
> >>>> different vports.
> >>>>> flow pattern_template 0 create transfer relaxed no
> >>>>> pattern_template_id
> >>>>> 4 template represented_port ethdev_port_id is 0 / eth / ipv4 / udp
> >>>>> / vxlan / tag index is 0 data is 0x33 / end flow actions_template
> >>>>> 0 create transfer actions_template_id 4 template raw_decap index 0
> >>>>> / represented_port ethdev_port_id 1 / end mask raw_decap index 0 /
> >>>>> represented_port ethdev_port_id 1 / end flow template_table 0
> >>>>> create group 1 priority 0 transfer wire_orig table_id 4
> >>>>> rules_number 128 pattern_template 4 actions_template 4
> >>>>>
> >>>>> 2. Traffic from vports should be encap with different VxLAN header
> >>>>> and send
> >>>> to wire.
> >>>>> flow actions_template 1 create transfer actions_template_id 5
> >>>>> template raw_encap index 0 / represented_port ethdev_port_id 0 /
> >>>>> end mask raw_encap index 0 / represented_port ethdev_port_id 0 /
> >>>>> end flow template_table 0 create group 1 priority 0 transfer
> >>>>> vport_orig table_id 5 rules_number 128 pattern_template 4
> >>>>> actions_template 5
> >>>>>
> >>>>>> For the rest of my notes, PSB.
> >>>>>>
> >>>>>> On Mon, 14 Nov 2022, Rongwei Liu wrote:
> >>>>>>
> >>>>>>> In case flow rules match only one kind of traffic in a flow
> >>>>>>> table, then optimization can be done via allocation of this table.
> >>>>>>
> >>>>>> This wording might confuse readers. Consider rephrasing it, please:
> >>>>>> If multiple flow rules share a common set of match masks, then
> >>>>>> they might belong in a flow table which can be pre-allocated.
> >>>>>>
> >>>>>>> Such optimization is possible only if the application gives a
> >>>>>>> hint about its usage of the table during initial configuration.
> >>>>>>>
> >>>>>>> The transfer domain rules may process traffic from wire or
> >>>>>>> vport, which may correspond to two kinds of underlayer resources.
> >>>>>>
> >>>>>> Why name it a "vport"? Why not "host"?
> >>>>>>
> >>>>>> host = packets generated by any of the host's "vport"s wire =
> >>>>>> packets arriving at the NIC from the network
> >>>>> Vport is "virtual port" for short and contains "VF/SF" for now.
> >>>>> Per my thoughts, it' clearer and maps to DPDK port
> probing/management.
> >>>>
> >>>> I understand that "host" might not be a brilliant name.
> >>>>
> >>>> If "vport" stands for every port of the NIC that is not a network
> >>>> port, then this name might be OK to me, but why doesn't it cover PFs?
> >>>> A PF is clearly not a network / physical port. Why just VF/SF then?
> >>>> Where
> >> does that "for now"
> >>>> decision come from? Just wondering.
> >>>>
> >>> "For now" stands for my understanding. DPDK is always in evolution, right?
> >>> You are right, PF should be included in 'vport" concept.
> >>>>>>
> >>>>>>> That's why the first two hints introduced in this patch are
> >>>>>>> about wire and vport traffic specialization.
> >>>>>>> Wire means traffic arrives from the uplink port while vport
> >>>>>>> means traffic initiated from VF/SF.
> >>>>>>
> >>>>>> By the sound of it, the meaning is confined to just VFs/SFs.
> >>>>>> What if the user wants to match packets coming from PFs?
> >>>>>>
> >>>>> It should be "wire_orig".
> >>>>
> >>>> Forgive me, but that does not sound correct. Say, there's an
> >>>> application and it has a PF plugged into it: ethdev index 0. And
> >>>> the application transmits packets using rte_eth_tx_burst() from that port.
> >>>> You say that these packets can be matched via "wire_orig".
> >>>> But they do not come from the wire. They come from PF...
> >>> Hmm. My mistake.
> >>> This may highly depend on PMD implementation. Basically, PFs'
> >>> traffic may contain "from wire"/"wire_orig" and '"from
> local"/"vport_orig".
> >>> That' why we emphasize it' optional for PMD.
> >>>>
> >>>>>>>
> >>>>>>> There are two possible approaches for providing the hints.
> >>>>>>> Using IPv4 as an example:
> >>>>>>> 1. Use pattern item in both template table and flow rules.
> >>>>>>>
> >>>>>>> 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 flow rule even if it's
> >>>>>>> just a hint. No value to match because matching is already done
> >>>>>>> by
> >>>>>>> IPv4 item.
> >>>>>>
> >>>>>> Why no value to match on? How does it prevent rogue tenants from
> >>>>>> spoofing network headers? If the application receives a packet on
> >>>>>> a particular vport's representor, then it may strictly specify
> >>>>>> item represented_port pointing to that vport so that only packets
> >>>>>> from that vport
> >>>> match.
> >>>>>>
> >>>>>> Why isn't security a consideration?
> >>>>>>
> >>>>> There is some misunderstanding here. "ANY_VPORT" is the approach
> >>>>> (new
> >>>> matching item without value) suggested by you.
> >>>> I'm not talking about ANY_VPORT in this particular paragraph.
> >>>>
> >>>> There's item "represented_port" mentioned over there. I'm just
> >>>> asking about this "already done by IPv4 item" bit. Yes, it matches
> >>>> on the header but not on the true origin of the packet (the logical
> >>>> port of the NIC). If the app knows which logical port the packet
> >>>> ingresses the NIC, why not match on it for security?
> >>>>
> >>> Hint is not a matching and it implies how to manage underlayer
> >>> steering
> >> resource.
> >>> If "vport_orig" is present, PMD will only apply the steering logic
> >>> to vport
> >> traffic.
> >>> The resource is allocated in the async table before each rule.
> >>> Already cover
> >> security considerations.
> >>> Matching on "represented_port" needs to program each rule,
> >>> considering a
> >> port range like index "5-10".
> >>> Hint tells PMD only to take care of traffic from vport regardless
> >>> the port
> >> index.
> >>>
> >>>>> I was explaining we need to apply it to each flow rule even if
> >>>>> it's only a flag
> >>>> and no value.
> >>>>
> >>>> That's clear. But PSB.
> >>>>
> >>>>>>>
> >>>>>>> 2. Add special flags into table_attr.
> >>>>>>>
> >>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>>>>>
> >>>>>>> Approach 1 needs to specify the pattern in each flow rule which
> >>>>>>> wastes memory and is not user friendly.
> >>>>>>
> >>>>>> What if the user has to insert a group of rules which not only
> >>>>>> have the same set of match masks but also share exactly the same
> >>>>>> match spec values for a limited subset of network items (for
> >>>>>> example, those of an encap. header)? This way, a subset of
> >>>>>> network item specs can remain fixed across many rules. Does that
> >>>>>> count as wasting
> >> memory?
> >>>>>>
> >>>>> Per my understanding, you are talking "multiple spec and mask mixing".
> >>>>
> >>>> Say, there's a group of rules, and each of them matches on exactly
> >>>> the same encap. header (the same in all rules), but different
> >>>> internal match
> >> field values.
> >>>> So, why don't these "fixed"
> >>>> encap. header items deserve being "optimised" somehow, the same way
> >>>> as this "wire_orig" does?
> >>> We are back to original point. Async approach is trying to
> >>> pre-allocate
> >> resources and speed up the insertion.
> >>> Resource is allocated in async table stage and we only have mask
> >> information.
> >>> In each rule, the matching value passes in. I guess you are saying
> >>> to optimize
> >> per different matching values, right?
> >>> This needs dynamic calculations per each rule and wastes the
> >>> resource in
> >> async table(table allocates resource for all possible values).
> >>>>
> >>>> If the application knows for sure that there will be packets with
> >>>> exactly the same encap. header, - that forms this special knowledge
> >>>> that can be used during init times to help the PMD optimise
> >>>> resource
> >> allocation.
> >>>> Isn't that so? Don't these items deserve some form of a "hint"?
> >>>>
> >>> It can deserve some kinds of "hint". But see above, these hints are
> >>> per rule
> >> and resource allocation happens before rules.
> >>
> >> That's not per rule. Perhaps I should've worded it differently.
> >>
> >> Suppose, an application has to insert many flow rules, each of which
> >> has match items A and B. Item A not only has the same mask in *all*
> >> rule instances, but also the same spec.
> >> On the other hand, item B only has the same mask in all the rules,
> >> but its spec is different for each rule.
> >>
> >> In this example, the application can allocate a template with items A
> >> and B, but that only provides a fixed mask for them. And the
> >> application will HAVE to provide item A with exactly the same spec in
> >> all rule instances. The PMD, in turn, will HAVE to process this item
> >> every time, being unable to see it's in fact the same at all times.
> >>
> >> To me, this sounds very similar to how you described the need to
> >> always provide item ANY_VPORT in each rule thus facing some waste of
> >> memory and parsing difficulties.
> >>
> >> If the application knows that a certain item (or a certain fraction
> >> of items) is going to be entirely the same (mask +
> >> spec) across all the rules, why shouldn't it be able to express this
> >> as a hint to the PMD? Why shouldn't it be able to avoid providing
> >> such items in every new flow rule instance? The same way the "vport_orig"
> works.
> >>
> >> I'm not demanding that you re-implement or re-design this.
> >> Just trying to find out whether such a problem can indeed be acknowledged.
> >> Or has it been solved already? If not, then perhaps it pays to just
> >> discuss whether solving it can be combined with this "vport_orig" solution.
> >>
> >> What do you think? What do others think?
> >>
> >>>>> We provide a hint in this patch and no assumption on the matching
> >> patterns.
> >>>>
> >>>> So I understand. My point is, certain portions of matching patterns
> >>>> may be "fixed" = entirely the same (masks and specs) in all rules
> >>>> of a table. Why not give PMD a "hint" about them, too?
> >>>>
> >>>>> I think matching pattern is totally controlled by application layer.
> >>>>
> >>>> So is the "direction" spec: the app layer has item represented_port
> >>>> to control that. But, still, we're here to discuss a hint for that.
> >>>> Why does the new hint aim exclusively at optimising out this
> >>>> specific meta item? Why isn't it possible to care about a generic
> >>>> portion of "know in advance" all-the-same items?
> >>> " generic portion of know in advance" is some still kind of dynamic
> >>> approach,
> >> right?
> >>> Imagine a situation. DPDK has 10 VFs, each VF may have different
> >>> VxLAN
> >> encap headers.
> >>> This hint approach can work for 10 VFs once.
> >>> In public cloud deployments, each VF/SF may map to different users,
> >>> but
> >> underlay is almost same(GRE/VxLAN... differ in filed values).
> >>>>
> >>>>> "wasting memory " because your approach needs to scatter in each
> >>>>> rule
> >>>> while this patch only needs to set table_attr once.
> >>>>> No relation with matching patter totally.
> >>>>
> >>>> The slight problem with your proposal is that for some reason only
> >>>> one type of a match criterion deserves a hint moved to the attrs.
> >>>> Whilst in reality the applicaction may know in advance that certain
> >>>> subsets of items will not only have the same masks in all rules but
> >>>> also totally the same specs. If that is a valid use case, why
> >>>> doesn't it deserve the same (more
> >>>> generic) optimisation / a hint? Just wondering...
> >>>> Or has that been addressed already somehow?
> >>>>
> >>> Believe me, the hint helps us to save significant resources already.
> >>
> >> I'm not arguing it can be helpful. You're working round the clock to
> >> offer a solution, - that's fine and is greatly appreciated.
> >> But what I'm trying to say is that it looks like the problem might
> >> manifest itself for other type of knowledge that also may deserve a hint.
> Hence the questions.
> >> Hence the offer to think of covering more match criteria, not just net/vport.
> >>
> >>> Per my view, your proposal is totally valid in sync approach, but
> >>> please check my response, Async is trying to allocate resources in
> >>> advance
> >> and speed up insertion ASAP.
> >>
> >> So if it's valid in sync approach, then why can't it be valid in the async one?
> >> And I guess it can reflect positively on the insertion rate, too. Why
> >> limit this "hint" approach to just one aspect then?
> >>
> >> I'm sure we're close to understanding each other here.
> >> Yes, "orig_vport" is just a one-bit knowledge, and seems innocent to
> >> add as a hint, but why not make it possible to have a hint for an
> >> arbitrary set of always- the-same match criteria?
> >>
> >> In this case, nobody will ever argue of whether a hint is a match
> >> criterion or if it's not. It will be quite a generic instrument, potentially
> useful to vendors.
> >> I'm afraid I can't think of an immediate example of such usefulness,
> >> but at least it will appear as generic as possible from the API perspective.
> >>
> >>>>>> If yes, then the problem does not concern just a single pair of
> >>>>>> attributes, but rather deserves a more versatile solution like
> >>>>>> some sort of indirect grouping of constant item specs.
> >>>>>> Have you considered such options?
> >>>>> See above.
> >>>>>>
> >>>>>>> This patch takes the 2nd approach and introduces one new member
> >>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow
> >>>>>>> table optimization.
> >>>>>>
> >>>>>> The name "specialize" might have some drawbacks:
> >>>>>> - spelling difference (specialise/specialize)
> >>>>>> - in grep output, will mix with flows' "spec"
> >>>>>> - quite long
> >>>>>> - not a noun
> >>>>>>
> >>>>>> Why not "scope"? Or something like that?
> >>>>>>
> >>>>> It means special optimization to PMD. "scope" is more rogue.
> >>>>
> >>>> Why is it "rogue"? Scope is something limiting the point of view.
> >>>> So are the suggested flags. Flag "wire_origin" (or whatever it can
> >>>> be named
> >>>> eventually) limits the scope of matching. No?
> >>>>
> >>> Hint won't interfere with matching. It has no knowledge of matching.
> >>
> >> Does specifying "orig_vport" actually provide a *choice* for the packet
> origin?
> >> Does it filter out everything else? If yes, then, alas, it *is*
> >> matching. Because matching is choosing something of interest. Let's face it.
> >>
> >> As I said above, I do acknowledge the fact that, for some vendors,
> >> this match criterion, internally goes to a different HW aspect that
> >> is separate from matching on, for example, IPv4 addresses.
> >> That's OK. But for some vendors, this might be just a regular match
> >> criterion internally. So let's describe it with care.
> >>
> >>> Instead, it only controls matching resources. "wire_orig" tells PMD
> >>> to
> >> allocate HW resource for traffic from wire only.
> >>
> >> If it controls "matching resources", it's indeed affiliated with matching then.
> >> Look. When the application creates a template, it tells the PMD that
> >> it is going to match on this, this and this.
> >> Masks... No exact values; they will come at a later stage. But, with
> >> this "wire_orig", the application tells the PMD that not only it will
> >> match on
> >> *some* "direction", but it actually provides a SPEC for that. If it
> >> indicates bit "wire_orig", that equals to setting a "mask" for the "direction
> enum"
> >> AND a "spec" (WIRE). Isn't that the case?
> >>
> >> If it is, then please see my above concerns about possibly having
> >> similar need to provide exact-spec hints for other items as well.
> >>
> >>> Then traffic from vport is sliently ignored. Hint doesn't know what
> >>> are
> >> matched and how many fields are involves.
> >>>>>>>
> >>>>>>> By default, there is no hint, so the behavior of the transfer
> >>>>>>> domain doesn't change.
> >>>>>>> There is no guarantee that the hint will be used by the PMD.
> >>>>>>>
> >>>>>>> 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.
> >>>>>>> v5: Change specialize type to uint32_t.
> >>>>>>> v6: Change the flags to macros and re-construct the commit log.
> >>>>>>> v7: Fix build failure.
> >>>>>>> ---
> >>>>>>> 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 | 28 +++++++++++++++++++++
> >>>>>>> 4 files changed, 71 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/app/test-pmd/cmdline_flow.c
> >>>>>>> b/app/test-pmd/cmdline_flow.c index 88108498e0..62197f2618
> >>>>>>> 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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_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.
> >>>>>>
> >>>>>> Silently ignoring the field does not sit well with the
> >>>>>> application's possible intent to drop represented_port match from
> >>>>>> the
> >> patterns.
> >>>>>> From my point of view, if the application sets this attribute, it
> >>>>>> believes it can rely on it, that is, packets coming from host
> >>>>>> won't match if the attribute asks to match network only, for instance.
> >>>>>> Has this
> >>>> been considered?
> >>>>>>
> >>>>>>> +
> >>>>>>> +If a table is specialized, the application should make sure the
> >>>>>>> +rules comply with the table attribute.
> >>>>>>
> >>>>>> How does the application enforce that? I would appreciate you explain
> it.
> >>>>>>
> >>>>>>> +
> >>>>>>> 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..c27b48c5c1 100644
> >>>>>>> --- a/lib/ethdev/rte_flow.h
> >>>>>>> +++ b/lib/ethdev/rte_flow.h
> >>>>>>> @@ -5186,6 +5186,29 @@
> >> rte_flow_actions_template_destroy(uint16_t
> >>>>>>> port_id, */ struct rte_flow_template_table;
> >>>>>>>
> >>>>>>> +/**@{@name Special optional flags for template table attribute
> >>>>>>> + * Each bit is a hint for table specialization,
> >>>>>>> + * offering a potential optimization at PMD layer.
> >>>>>>> + * PMD can ignore the unsupported bits silently.
> >>>>>>> + */
> >>>>>>> +/**
> >>>>>>> + * 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.
> >>>>>>
> >>>>>> You intended to spell "criterion", I take it. And still, it *is*
> >>>>>> a match
> >> criterion.
> >>>>>> I'm not denying the possible need to have this criterion at the
> >>>>>> earliest processing stage. That might be OK, but I still have a
> >>>>>> hunch that this is too specific.
> >>>>>> Please see my comment above about wasting memory.
> >>>>>> I guess this type of criterion is not the only one that may need
> >>>>>> to be provided as a "hint".
> >>>>>>
> >>>>>>> + * Flow rules which match non-wire originated traffic will be
> >>>>>>> + missed
> >>>>>>> + * if the hint is supported.
> >>>>>>
> >>>>>> And what if it's unsupported? Is it indeed OK to silently ignore it?
> >>>>>>
> >>>>>>> + */
> >>>>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_WIRE_ORIG
> >>>>>> RTE_BIT32(0)
> >>>>>>
> >>>>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_WIRE ?
> >>>>>>
> >>>>>> To me, TRANSFER looks redundant as this bit is already supposed
> >>>>>> to be ticked in the "struct rte_flow_attr flow_attr" field of the
> >>>>>> "struct rte_flow_template_table_attr".
> >>>>>>
> >>>>>>> +/**
> >>>>>>> + * Specialize table for transfer flows which come only from
> >>>>>>> +vport (e.g. VF,
> >>>>>>> SF).
> >>>>>>
> >>>>>> And PF?
> >>>>>>
> >>>>>>> + * 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.
> >>>>>>> + */
> >>>>>>> +#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG
> >>>>>> RTE_BIT32(1)
> >>>>>>
> >>>>>> Why not RTE_FLOW_TABLE_SCOPE_FROM_HOST ?
> >>>>>>
> >>>>>>> +/**@}*/
> >>>>>>> +
> >>>>>>> /**
> >>>>>>> * @warning
> >>>>>>> * @b EXPERIMENTAL: this API may change without prior notice.
> >>>>>>> @@ -5201,6 +5224,11 @@ 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.
> >>>>>>> + * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
> >>>>>>> + */
> >>>>>>> + uint32_t specialize;
> >>>>>>
> >>>>>> Why not "scope" or something?
> >>>>>>
> >>>>>>> };
> >>>>>>>
> >>>>>>> /**
> >>>>>>> --
> >>>>>>> 2.27.0
> >>>>>>>
> >>>>>>
> >>>>>> Thank you.
> >>>>>
> >>>
> >>
> >> Thank you.
> >
31/01/2023 06:30, Ivan Malov:
> Hi Rongwei,
>
> OK, I hear ya. Thanks for persevering.
>
> I still hope community will comment on the possibility to
> provide a hint mechanism for always-the-same match items,
> with the perspective of becoming more versatile.
Any hint could be imagined.
But please keep this in mind: a hint is *not* a matching criteria,
for the simple reason that a hint can be ignored by the PMD.
So you cannot use a hint to avoid specifying a match item,
but you could use a hint to specify that an item is the same
for all the rules of a table.
> Other
> than that, your current patch might be OK, but, again,
> I think other reviewers' comments (if any) shall
> be addressed. But no strong objections from me.
>
> By the way, for this "specialise" field, in your opinion,
> which extra flags could emerge in future / would be nice
> to have? I mean, is there any concept of what can be
> added to this field's namespace and what can't be?
I think there is no limit with hint flags to be added.
I repeat it again: hints can be ignored by the PMDs.
On 1/18/23 19:18, Thomas Monjalon wrote:
> 18/01/2023 08:28, Andrew Rybchenko:
>> On 11/14/22 14:59, Rongwei Liu wrote:
>>> In case flow rules match only one kind of traffic in a flow table,
>>> then optimization can be done via allocation of this table.
>>> Such optimization is possible only if the application gives a hint
>>> about its usage of the table during initial configuration.
>>>
>>> The transfer domain rules may process traffic from wire or vport,
>>> which may correspond to two kinds of underlayer resources.
>>> That's why the first two hints introduced in this patch are about
>>> wire and vport traffic specialization.
>>> Wire means traffic arrives from the uplink port while vport means
>>> traffic initiated from VF/SF.
>>>
>>> There are two possible approaches for providing the hints.
>>> Using IPv4 as an example:
>>> 1. Use pattern item in both template table and flow rules.
>>>
>>> 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 flow rule even if it's
>>> just a hint. No value to match because matching is already done by
>>> IPv4 item.
>>>
>>> 2. Add special flags into table_attr.
>>>
>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>
>>> Approach 1 needs to specify the pattern in each flow rule which wastes
>>> memory and is not user friendly.
>>> This patch takes the 2nd approach and introduces one new member
>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>> optimization.
>>
>> The above description is misleading. It alternates options (1)
>> and (2), but in fact (2) requires (1) as well.
>
> Yes the above description may be misleading
> and it seems you are misleaded :)
It is not my intention. If it is only my problem, I'm OK to
step back.
> I will explain below why the option (2) doesn't require (1).
> I think we should apply the same example to both cases to make it clear:
>
> 1. Use pattern item in both template table and flow rules:
>
> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
>
> The pattern template 3 will be used only to match flows coming from vports.
> ANY_VPORT needs to be present in each flow rule.
It looks like I lost something here. Why do we need to specify
it in each flow rule if the matching is already fixed in
template table?
> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> the user knows 1.1.1.1 is the IP of a vport.
What should happen if a packet with src IP 1.1.1.1 comes from
the wire? Almost anything could come from network.
>
> 2. Add specialization flag into template table attribute:
>
> template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is 255.255.255.255 / end
> flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
>
> The pattern template 3 can be used only to match flows coming from vports.
In this case it is interesting how it will behave on:
a NIC which does not support VPORT_ORIG and just ignores it
VS
a NIC which support VPORT_ORIG and takes it into account.
>
>> (2) is simply done on different level - much earlier, before
>> flow rules creation. Since resources allocation is assumed to
>> be done on table creation, we need to know the purpose of the
>> table in advance to optimize resources allocation.
>
> Actually in both cases we get the hint at template table creation.
> But in solution 2 we are not creating a redundant pattern matching,
> and we don't need to check it in flow rules, so it is more efficient.
>
>> Since (2) is *not a matching criteria*, but just a hint, (1)
>> flow rules must have matching criteria anyway.
>
> No we don't need the matching criteria ANY_VPORT with solution (2)
> because we are already matching on an IP src which is a vport.
>
>>> +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.
>>
>> If a table is specialized, the application must make sure that
>> all flow rules added to the table have pattern which implies
>> corresponding matching criteria. For example if a table is
>> specialized to be wire-origin only, pattern should have
>> represented port item with ethdev which corresponds to a
>> physical port (or any other item which matches packets
>> coming from wire only).
>
> No need of a matching criteria strictly mapping the hint.
> Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
> and the rules can match on an IP src which is assigned to a vport.
> So there is no need to strictly match the vport itself in the rule.
If so, the problem is that the same rules will behave in a different way
on different NICs.
> Hope it make thinks clear.
> We can improve the commit log as I wrote above.
>
>
01/02/2023 11:17, Andrew Rybchenko:
> On 1/18/23 19:18, Thomas Monjalon wrote:
> > 18/01/2023 08:28, Andrew Rybchenko:
> >> On 11/14/22 14:59, Rongwei Liu wrote:
> >>> In case flow rules match only one kind of traffic in a flow table,
> >>> then optimization can be done via allocation of this table.
> >>> Such optimization is possible only if the application gives a hint
> >>> about its usage of the table during initial configuration.
> >>>
> >>> The transfer domain rules may process traffic from wire or vport,
> >>> which may correspond to two kinds of underlayer resources.
> >>> That's why the first two hints introduced in this patch are about
> >>> wire and vport traffic specialization.
> >>> Wire means traffic arrives from the uplink port while vport means
> >>> traffic initiated from VF/SF.
> >>>
> >>> There are two possible approaches for providing the hints.
> >>> Using IPv4 as an example:
> >>> 1. Use pattern item in both template table and flow rules.
> >>>
> >>> 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 flow rule even if it's
> >>> just a hint. No value to match because matching is already done by
> >>> IPv4 item.
> >>>
> >>> 2. Add special flags into table_attr.
> >>>
> >>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>
> >>> Approach 1 needs to specify the pattern in each flow rule which wastes
> >>> memory and is not user friendly.
> >>> This patch takes the 2nd approach and introduces one new member
> >>> "specialize" into rte_flow_table_attr to indicate possible flow table
> >>> optimization.
> >>
> >> The above description is misleading. It alternates options (1)
> >> and (2), but in fact (2) requires (1) as well.
> >
> > Yes the above description may be misleading
> > and it seems you are misleaded :)
>
> It is not my intention. If it is only my problem, I'm OK to
> step back.
It's OK to explain and check everything is OK, no worries.
Thanks for reviewing.
> > I will explain below why the option (2) doesn't require (1).
> > I think we should apply the same example to both cases to make it clear:
> >
> > 1. Use pattern item in both template table and flow rules:
> >
> > template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
> > flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
> >
> > The pattern template 3 will be used only to match flows coming from vports.
> > ANY_VPORT needs to be present in each flow rule.
>
> It looks like I lost something here. Why do we need to specify
> it in each flow rule if the matching is already fixed in
> template table?
I think that's how template tables are designed.
Ori, please could you point us to the relevant documentation?
> > ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> > the user knows 1.1.1.1 is the IP of a vport.
>
> What should happen if a packet with src IP 1.1.1.1 comes from
> the wire? Almost anything could come from network.
It a packet comes from a wired port AND
the PMD did an optimization based on this hint,
then the packet could be not matched.
> > 2. Add specialization flag into template table attribute:
> >
> > template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is 255.255.255.255 / end
> > flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
> >
> > The pattern template 3 can be used only to match flows coming from vports.
>
> In this case it is interesting how it will behave on:
> a NIC which does not support VPORT_ORIG and just ignores it
If the hint is ignored, all packets will hit the rule.
> VS
> a NIC which support VPORT_ORIG and takes it into account.
If the hint is taken into account,
it is possible that wire packets do not hit this rule,
but we don't really know, it is an internal driver optimization.
> >> (2) is simply done on different level - much earlier, before
> >> flow rules creation. Since resources allocation is assumed to
> >> be done on table creation, we need to know the purpose of the
> >> table in advance to optimize resources allocation.
> >
> > Actually in both cases we get the hint at template table creation.
> > But in solution 2 we are not creating a redundant pattern matching,
> > and we don't need to check it in flow rules, so it is more efficient.
> >
> >> Since (2) is *not a matching criteria*, but just a hint, (1)
> >> flow rules must have matching criteria anyway.
> >
> > No we don't need the matching criteria ANY_VPORT with solution (2)
> > because we are already matching on an IP src which is a vport.
> >
> >>> +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.
> >>
> >> If a table is specialized, the application must make sure that
> >> all flow rules added to the table have pattern which implies
> >> corresponding matching criteria. For example if a table is
> >> specialized to be wire-origin only, pattern should have
> >> represented port item with ethdev which corresponds to a
> >> physical port (or any other item which matches packets
> >> coming from wire only).
> >
> > No need of a matching criteria strictly mapping the hint.
> > Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
> > and the rules can match on an IP src which is assigned to a vport.
> > So there is no need to strictly match the vport itself in the rule.
>
> If so, the problem is that the same rules will behave in a different way
> on different NICs.
Not exactly.
If the assumption made by the application is wrong,
yes there may be some differences for the unexpected packets.
But it would be a user mistake somewhere.
In general, an application should have the same functional result,
no matter the hint is used or not by the driver.
> > Hope it make thinks clear.
> > We can improve the commit log as I wrote above.
On 2/1/23 13:58, Thomas Monjalon wrote:
> 01/02/2023 11:17, Andrew Rybchenko:
>> On 1/18/23 19:18, Thomas Monjalon wrote:
>>> 18/01/2023 08:28, Andrew Rybchenko:
>>>> On 11/14/22 14:59, Rongwei Liu wrote:
>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>> then optimization can be done via allocation of this table.
>>>>> Such optimization is possible only if the application gives a hint
>>>>> about its usage of the table during initial configuration.
>>>>>
>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>> which may correspond to two kinds of underlayer resources.
>>>>> That's why the first two hints introduced in this patch are about
>>>>> wire and vport traffic specialization.
>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>> traffic initiated from VF/SF.
>>>>>
>>>>> There are two possible approaches for providing the hints.
>>>>> Using IPv4 as an example:
>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>
>>>>> 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 flow rule even if it's
>>>>> just a hint. No value to match because matching is already done by
>>>>> IPv4 item.
>>>>>
>>>>> 2. Add special flags into table_attr.
>>>>>
>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>
>>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
>>>>> memory and is not user friendly.
>>>>> This patch takes the 2nd approach and introduces one new member
>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>>>> optimization.
>>>>
>>>> The above description is misleading. It alternates options (1)
>>>> and (2), but in fact (2) requires (1) as well.
>>>
>>> Yes the above description may be misleading
>>> and it seems you are misleaded :)
>>
>> It is not my intention. If it is only my problem, I'm OK to
>> step back.
>
> It's OK to explain and check everything is OK, no worries.
> Thanks for reviewing.
>
>>> I will explain below why the option (2) doesn't require (1).
>>> I think we should apply the same example to both cases to make it clear:
>>>
>>> 1. Use pattern item in both template table and flow rules:
>>>
>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
>>>
>>> The pattern template 3 will be used only to match flows coming from vports.
>>> ANY_VPORT needs to be present in each flow rule.
>>
>> It looks like I lost something here. Why do we need to specify
>> it in each flow rule if the matching is already fixed in
>> template table?
>
> I think that's how template tables are designed.
> Ori, please could you point us to the relevant documentation?
>
>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
>>> the user knows 1.1.1.1 is the IP of a vport.
>>
>> What should happen if a packet with src IP 1.1.1.1 comes from
>> the wire? Almost anything could come from network.
>
> It a packet comes from a wired port AND
> the PMD did an optimization based on this hint,
> then the packet could be not matched.
So, the hint changes matching results and therefore becomes
a strange (extra) matching criteria under specific
circumstance. It sounds bad. So, good application must use
real (always) matching criteria when composing flow rules.
So, RTE flow API should provide a way to write a good
application without extra pain.
That's why I'm saying that (2) requires (1) anyway.
It does not say that hint is not required at all.
It is still useful for resources usage optimization if
application knows how it is going to use particular table.
>
>>> 2. Add specialization flag into template table attribute:
>>>
>>> template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is 255.255.255.255 / end
>>> flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
>>>
>>> The pattern template 3 can be used only to match flows coming from vports.
>>
>> In this case it is interesting how it will behave on:
>> a NIC which does not support VPORT_ORIG and just ignores it
>
> If the hint is ignored, all packets will hit the rule.
>
>> VS
>> a NIC which support VPORT_ORIG and takes it into account.
>
> If the hint is taken into account,
> it is possible that wire packets do not hit this rule,
> but we don't really know, it is an internal driver optimization.
>
>
>>>> (2) is simply done on different level - much earlier, before
>>>> flow rules creation. Since resources allocation is assumed to
>>>> be done on table creation, we need to know the purpose of the
>>>> table in advance to optimize resources allocation.
>>>
>>> Actually in both cases we get the hint at template table creation.
>>> But in solution 2 we are not creating a redundant pattern matching,
>>> and we don't need to check it in flow rules, so it is more efficient.
>>>
>>>> Since (2) is *not a matching criteria*, but just a hint, (1)
>>>> flow rules must have matching criteria anyway.
>>>
>>> No we don't need the matching criteria ANY_VPORT with solution (2)
>>> because we are already matching on an IP src which is a vport.
>>>
>>>>> +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.
>>>>
>>>> If a table is specialized, the application must make sure that
>>>> all flow rules added to the table have pattern which implies
>>>> corresponding matching criteria. For example if a table is
>>>> specialized to be wire-origin only, pattern should have
>>>> represented port item with ethdev which corresponds to a
>>>> physical port (or any other item which matches packets
>>>> coming from wire only).
>>>
>>> No need of a matching criteria strictly mapping the hint.
>>> Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
>>> and the rules can match on an IP src which is assigned to a vport.
>>> So there is no need to strictly match the vport itself in the rule.
>>
>> If so, the problem is that the same rules will behave in a different way
>> on different NICs.
>
> Not exactly.
> If the assumption made by the application is wrong,
> yes there may be some differences for the unexpected packets.
> But it would be a user mistake somewhere.
>
> In general, an application should have the same functional result,
> no matter the hint is used or not by the driver.
>
>>> Hope it make thinks clear.
>>> We can improve the commit log as I wrote above.
>
>
>
Hi
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, 1 February 2023 12:59
>
> 01/02/2023 11:17, Andrew Rybchenko:
> > On 1/18/23 19:18, Thomas Monjalon wrote:
> > > 18/01/2023 08:28, Andrew Rybchenko:
> > >> On 11/14/22 14:59, Rongwei Liu wrote:
> > >>> In case flow rules match only one kind of traffic in a flow table,
> > >>> then optimization can be done via allocation of this table.
> > >>> Such optimization is possible only if the application gives a hint
> > >>> about its usage of the table during initial configuration.
> > >>>
> > >>> The transfer domain rules may process traffic from wire or vport,
> > >>> which may correspond to two kinds of underlayer resources.
> > >>> That's why the first two hints introduced in this patch are about
> > >>> wire and vport traffic specialization.
> > >>> Wire means traffic arrives from the uplink port while vport means
> > >>> traffic initiated from VF/SF.
> > >>>
> > >>> There are two possible approaches for providing the hints.
> > >>> Using IPv4 as an example:
> > >>> 1. Use pattern item in both template table and flow rules.
> > >>>
> > >>> 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 flow rule even if it's
> > >>> just a hint. No value to match because matching is already done by
> > >>> IPv4 item.
> > >>>
> > >>> 2. Add special flags into table_attr.
> > >>>
> > >>> template_table 0 create table_id 0 group 1 transfer vport_orig
> > >>>
> > >>> Approach 1 needs to specify the pattern in each flow rule which wastes
> > >>> memory and is not user friendly.
> > >>> This patch takes the 2nd approach and introduces one new member
> > >>> "specialize" into rte_flow_table_attr to indicate possible flow table
> > >>> optimization.
> > >>
> > >> The above description is misleading. It alternates options (1)
> > >> and (2), but in fact (2) requires (1) as well.
> > >
> > > Yes the above description may be misleading
> > > and it seems you are misleaded :)
> >
> > It is not my intention. If it is only my problem, I'm OK to
> > step back.
>
> It's OK to explain and check everything is OK, no worries.
> Thanks for reviewing.
>
> > > I will explain below why the option (2) doesn't require (1).
> > > I think we should apply the same example to both cases to make it clear:
> > >
> > > 1. Use pattern item in both template table and flow rules:
> > >
> > > template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is
> 255.255.255.255 / end
> > > flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is
> 1.1.1.1 / end
> > >
> > > The pattern template 3 will be used only to match flows coming from
> vports.
> > > ANY_VPORT needs to be present in each flow rule.
> >
> > It looks like I lost something here. Why do we need to specify
> > it in each flow rule if the matching is already fixed in
> > template table?
>
> I think that's how template tables are designed.
> Ori, please could you point us to the relevant documentation?
>
Two things:
1. we don't have at this point a way in the rte_flow template tables a way to set a fix match.
Meaning to say all rules should match ip src = x
When we create a template we are just saying on which fields/items we want to match
not the value.
So if ip src is masked it mean that we will want to match on the ip src but each rule
will specify the exact value.
Side note In the action we do have a way to set fixed action for this template.
2. I think in the above example, the idea is to show that the src marks the VM (vport)
This is why we need to match only on traffic coming from VM and not wire
since if the src is coming from uplink it doesn't have anything to do with the requested rule.
> > > ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> > > the user knows 1.1.1.1 is the IP of a vport.
> >
> > What should happen if a packet with src IP 1.1.1.1 comes from
> > the wire? Almost anything could come from network.
>
> It a packet comes from a wired port AND
> the PMD did an optimization based on this hint,
> then the packet could be not matched.
This is the idea the rule will not match on it. See my above comment.
>
> > > 2. Add specialization flag into template table attribute:
> > >
> > > template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is
> 255.255.255.255 / end
> > > flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
> > >
> > > The pattern template 3 can be used only to match flows coming from
> vports.
> >
> > In this case it is interesting how it will behave on:
> > a NIC which does not support VPORT_ORIG and just ignores it
>
> If the hint is ignored, all packets will hit the rule.
>
+1
> > VS
> > a NIC which support VPORT_ORIG and takes it into account.
>
> If the hint is taken into account,
> it is possible that wire packets do not hit this rule,
> but we don't really know, it is an internal driver optimization.
>
>
> > >> (2) is simply done on different level - much earlier, before
> > >> flow rules creation. Since resources allocation is assumed to
> > >> be done on table creation, we need to know the purpose of the
> > >> table in advance to optimize resources allocation.
> > >
> > > Actually in both cases we get the hint at template table creation.
> > > But in solution 2 we are not creating a redundant pattern matching,
> > > and we don't need to check it in flow rules, so it is more efficient.
> > >
> > >> Since (2) is *not a matching criteria*, but just a hint, (1)
> > >> flow rules must have matching criteria anyway.
> > >
> > > No we don't need the matching criteria ANY_VPORT with solution (2)
> > > because we are already matching on an IP src which is a vport.
> > >
> > >>> +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.
> > >>
> > >> If a table is specialized, the application must make sure that
> > >> all flow rules added to the table have pattern which implies
> > >> corresponding matching criteria. For example if a table is
> > >> specialized to be wire-origin only, pattern should have
> > >> represented port item with ethdev which corresponds to a
> > >> physical port (or any other item which matches packets
> > >> coming from wire only).
> > >
> > > No need of a matching criteria strictly mapping the hint.
> > > Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
> > > and the rules can match on an IP src which is assigned to a vport.
> > > So there is no need to strictly match the vport itself in the rule.
> >
> > If so, the problem is that the same rules will behave in a different way
> > on different NICs.
>
> Not exactly.
> If the assumption made by the application is wrong,
> yes there may be some differences for the unexpected packets.
> But it would be a user mistake somewhere.
>
> In general, an application should have the same functional result,
> no matter the hint is used or not by the driver.
>
> > > Hope it make thinks clear.
> > > We can improve the commit log as I wrote above.
>
>
01/02/2023 12:10, Andrew Rybchenko:
> On 2/1/23 13:58, Thomas Monjalon wrote:
> > 01/02/2023 11:17, Andrew Rybchenko:
> >> On 1/18/23 19:18, Thomas Monjalon wrote:
> >>> 18/01/2023 08:28, Andrew Rybchenko:
> >>>> On 11/14/22 14:59, Rongwei Liu wrote:
> >>>>> In case flow rules match only one kind of traffic in a flow table,
> >>>>> then optimization can be done via allocation of this table.
> >>>>> Such optimization is possible only if the application gives a hint
> >>>>> about its usage of the table during initial configuration.
> >>>>>
> >>>>> The transfer domain rules may process traffic from wire or vport,
> >>>>> which may correspond to two kinds of underlayer resources.
> >>>>> That's why the first two hints introduced in this patch are about
> >>>>> wire and vport traffic specialization.
> >>>>> Wire means traffic arrives from the uplink port while vport means
> >>>>> traffic initiated from VF/SF.
> >>>>>
> >>>>> There are two possible approaches for providing the hints.
> >>>>> Using IPv4 as an example:
> >>>>> 1. Use pattern item in both template table and flow rules.
> >>>>>
> >>>>> 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 flow rule even if it's
> >>>>> just a hint. No value to match because matching is already done by
> >>>>> IPv4 item.
> >>>>>
> >>>>> 2. Add special flags into table_attr.
> >>>>>
> >>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>>>
> >>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
> >>>>> memory and is not user friendly.
> >>>>> This patch takes the 2nd approach and introduces one new member
> >>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
> >>>>> optimization.
> >>>>
> >>>> The above description is misleading. It alternates options (1)
> >>>> and (2), but in fact (2) requires (1) as well.
> >>>
> >>> Yes the above description may be misleading
> >>> and it seems you are misleaded :)
> >>
> >> It is not my intention. If it is only my problem, I'm OK to
> >> step back.
> >
> > It's OK to explain and check everything is OK, no worries.
> > Thanks for reviewing.
> >
> >>> I will explain below why the option (2) doesn't require (1).
> >>> I think we should apply the same example to both cases to make it clear:
> >>>
> >>> 1. Use pattern item in both template table and flow rules:
> >>>
> >>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
> >>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
> >>>
> >>> The pattern template 3 will be used only to match flows coming from vports.
> >>> ANY_VPORT needs to be present in each flow rule.
> >>
> >> It looks like I lost something here. Why do we need to specify
> >> it in each flow rule if the matching is already fixed in
> >> template table?
> >
> > I think that's how template tables are designed.
> > Ori, please could you point us to the relevant documentation?
> >
> >>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> >>> the user knows 1.1.1.1 is the IP of a vport.
> >>
> >> What should happen if a packet with src IP 1.1.1.1 comes from
> >> the wire? Almost anything could come from network.
> >
> > It a packet comes from a wired port AND
> > the PMD did an optimization based on this hint,
> > then the packet could be not matched.
>
> So, the hint changes matching results and therefore becomes
> a strange (extra) matching criteria under specific
> circumstance. It sounds bad.
In this case, the user made a wrong assumption.
If the user does not do a mistake, the behavior should be the same
whether the hint is used or ignored.
> So, good application must use
> real (always) matching criteria when composing flow rules.
Of course, nothing replaces matching criteria.
> So, RTE flow API should provide a way to write a good
> application without extra pain.
> That's why I'm saying that (2) requires (1) anyway.
I don't follow this sentence.
If you mean with hint, flow matching is still required, then yes,
this is what I emphasized in my rewrite of the case (2) below.
> It does not say that hint is not required at all.
> It is still useful for resources usage optimization if
> application knows how it is going to use particular table.
Yes, that's an optional optimization.
It should not change the rules,
and it should not change the functional behavior
if the user does not do mistakes.
> >>> 2. Add specialization flag into template table attribute:
> >>>
> >>> template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is 255.255.255.255 / end
> >>> flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
> >>>
> >>> The pattern template 3 can be used only to match flows coming from vports.
> >>
> >> In this case it is interesting how it will behave on:
> >> a NIC which does not support VPORT_ORIG and just ignores it
> >
> > If the hint is ignored, all packets will hit the rule.
> >
> >> VS
> >> a NIC which support VPORT_ORIG and takes it into account.
> >
> > If the hint is taken into account,
> > it is possible that wire packets do not hit this rule,
> > but we don't really know, it is an internal driver optimization.
> >
> >
> >>>> (2) is simply done on different level - much earlier, before
> >>>> flow rules creation. Since resources allocation is assumed to
> >>>> be done on table creation, we need to know the purpose of the
> >>>> table in advance to optimize resources allocation.
> >>>
> >>> Actually in both cases we get the hint at template table creation.
> >>> But in solution 2 we are not creating a redundant pattern matching,
> >>> and we don't need to check it in flow rules, so it is more efficient.
> >>>
> >>>> Since (2) is *not a matching criteria*, but just a hint, (1)
> >>>> flow rules must have matching criteria anyway.
> >>>
> >>> No we don't need the matching criteria ANY_VPORT with solution (2)
> >>> because we are already matching on an IP src which is a vport.
> >>>
> >>>>> +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.
> >>>>
> >>>> If a table is specialized, the application must make sure that
> >>>> all flow rules added to the table have pattern which implies
> >>>> corresponding matching criteria. For example if a table is
> >>>> specialized to be wire-origin only, pattern should have
> >>>> represented port item with ethdev which corresponds to a
> >>>> physical port (or any other item which matches packets
> >>>> coming from wire only).
> >>>
> >>> No need of a matching criteria strictly mapping the hint.
> >>> Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
> >>> and the rules can match on an IP src which is assigned to a vport.
> >>> So there is no need to strictly match the vport itself in the rule.
> >>
> >> If so, the problem is that the same rules will behave in a different way
> >> on different NICs.
> >
> > Not exactly.
> > If the assumption made by the application is wrong,
> > yes there may be some differences for the unexpected packets.
> > But it would be a user mistake somewhere.
> >
> > In general, an application should have the same functional result,
> > no matter the hint is used or not by the driver.
> >
> >>> Hope it make thinks clear.
> >>> We can improve the commit log as I wrote above.
01/02/2023 12:12, Ori Kam:
> 1. we don't have at this point a way in the rte_flow template tables a way to set a fix match.
> Meaning to say all rules should match ip src = x
> When we create a template we are just saying on which fields/items we want to match
> not the value.
> So if ip src is masked it mean that we will want to match on the ip src but each rule
> will specify the exact value.
> Side note In the action we do have a way to set fixed action for this template.
I think we need to improve the documentation to make it clear.
Hi
Please also see my previous mail.
> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, 1 February 2023 13:11
>
> On 2/1/23 13:58, Thomas Monjalon wrote:
> > 01/02/2023 11:17, Andrew Rybchenko:
> >> On 1/18/23 19:18, Thomas Monjalon wrote:
> >>> 18/01/2023 08:28, Andrew Rybchenko:
> >>>> On 11/14/22 14:59, Rongwei Liu wrote:
> >>>>> In case flow rules match only one kind of traffic in a flow table,
> >>>>> then optimization can be done via allocation of this table.
> >>>>> Such optimization is possible only if the application gives a hint
> >>>>> about its usage of the table during initial configuration.
> >>>>>
> >>>>> The transfer domain rules may process traffic from wire or vport,
> >>>>> which may correspond to two kinds of underlayer resources.
> >>>>> That's why the first two hints introduced in this patch are about
> >>>>> wire and vport traffic specialization.
> >>>>> Wire means traffic arrives from the uplink port while vport means
> >>>>> traffic initiated from VF/SF.
> >>>>>
> >>>>> There are two possible approaches for providing the hints.
> >>>>> Using IPv4 as an example:
> >>>>> 1. Use pattern item in both template table and flow rules.
> >>>>>
> >>>>> 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 flow rule even if it's
> >>>>> just a hint. No value to match because matching is already done by
> >>>>> IPv4 item.
> >>>>>
> >>>>> 2. Add special flags into table_attr.
> >>>>>
> >>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>>>
> >>>>> Approach 1 needs to specify the pattern in each flow rule which
> wastes
> >>>>> memory and is not user friendly.
> >>>>> This patch takes the 2nd approach and introduces one new member
> >>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
> >>>>> optimization.
> >>>>
> >>>> The above description is misleading. It alternates options (1)
> >>>> and (2), but in fact (2) requires (1) as well.
> >>>
> >>> Yes the above description may be misleading
> >>> and it seems you are misleaded :)
> >>
> >> It is not my intention. If it is only my problem, I'm OK to
> >> step back.
> >
> > It's OK to explain and check everything is OK, no worries.
> > Thanks for reviewing.
> >
> >>> I will explain below why the option (2) doesn't require (1).
> >>> I think we should apply the same example to both cases to make it clear:
> >>>
> >>> 1. Use pattern item in both template table and flow rules:
> >>>
> >>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is
> 255.255.255.255 / end
> >>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is
> 1.1.1.1 / end
> >>>
> >>> The pattern template 3 will be used only to match flows coming from
> vports.
> >>> ANY_VPORT needs to be present in each flow rule.
> >>
> >> It looks like I lost something here. Why do we need to specify
> >> it in each flow rule if the matching is already fixed in
> >> template table?
> >
> > I think that's how template tables are designed.
> > Ori, please could you point us to the relevant documentation?
> >
> >>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> >>> the user knows 1.1.1.1 is the IP of a vport.
> >>
> >> What should happen if a packet with src IP 1.1.1.1 comes from
> >> the wire? Almost anything could come from network.
> >
> > It a packet comes from a wired port AND
> > the PMD did an optimization based on this hint,
> > then the packet could be not matched.
>
> So, the hint changes matching results and therefore becomes
> a strange (extra) matching criteria under specific
> circumstance. It sounds bad. So, good application must use
> real (always) matching criteria when composing flow rules.
> So, RTE flow API should provide a way to write a good
> application without extra pain.
> That's why I'm saying that (2) requires (1) anyway.
>
I'm sorry I don't see why.
This feature is about where to place the rule in the HW.
This feature can be useful for any HW that as different pipelines for ingress
and egress traffic. It can be used to save resources or if some actions can be done
only on one direction then the PMD can allow them and not block the this rule.
> It does not say that hint is not required at all.
> It is still useful for resources usage optimization if
> application knows how it is going to use particular table.
>
So we agree that the hint is good?
> >
> >>> 2. Add specialization flag into template table attribute:
> >>>
> >>> template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is
> 255.255.255.255 / end
> >>> flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
> >>>
> >>> The pattern template 3 can be used only to match flows coming from
> vports.
> >>
> >> In this case it is interesting how it will behave on:
> >> a NIC which does not support VPORT_ORIG and just ignores it
> >
> > If the hint is ignored, all packets will hit the rule.
> >
> >> VS
> >> a NIC which support VPORT_ORIG and takes it into account.
> >
> > If the hint is taken into account,
> > it is possible that wire packets do not hit this rule,
> > but we don't really know, it is an internal driver optimization.
> >
> >
> >>>> (2) is simply done on different level - much earlier, before
> >>>> flow rules creation. Since resources allocation is assumed to
> >>>> be done on table creation, we need to know the purpose of the
> >>>> table in advance to optimize resources allocation.
> >>>
> >>> Actually in both cases we get the hint at template table creation.
> >>> But in solution 2 we are not creating a redundant pattern matching,
> >>> and we don't need to check it in flow rules, so it is more efficient.
> >>>
> >>>> Since (2) is *not a matching criteria*, but just a hint, (1)
> >>>> flow rules must have matching criteria anyway.
> >>>
> >>> No we don't need the matching criteria ANY_VPORT with solution (2)
> >>> because we are already matching on an IP src which is a vport.
> >>>
> >>>>> +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.
> >>>>
> >>>> If a table is specialized, the application must make sure that
> >>>> all flow rules added to the table have pattern which implies
> >>>> corresponding matching criteria. For example if a table is
> >>>> specialized to be wire-origin only, pattern should have
> >>>> represented port item with ethdev which corresponds to a
> >>>> physical port (or any other item which matches packets
> >>>> coming from wire only).
> >>>
> >>> No need of a matching criteria strictly mapping the hint.
> >>> Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
> >>> and the rules can match on an IP src which is assigned to a vport.
> >>> So there is no need to strictly match the vport itself in the rule.
> >>
> >> If so, the problem is that the same rules will behave in a different way
> >> on different NICs.
> >
> > Not exactly.
> > If the assumption made by the application is wrong,
> > yes there may be some differences for the unexpected packets.
> > But it would be a user mistake somewhere.
> >
> > In general, an application should have the same functional result,
> > no matter the hint is used or not by the driver.
> >
> >>> Hope it make thinks clear.
> >>> We can improve the commit log as I wrote above.
> >
> >
> >
On 2/1/23 14:22, Ori Kam wrote:
> Hi
>
> Please also see my previous mail.
>
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Wednesday, 1 February 2023 13:11
>>
>> On 2/1/23 13:58, Thomas Monjalon wrote:
>>> 01/02/2023 11:17, Andrew Rybchenko:
>>>> On 1/18/23 19:18, Thomas Monjalon wrote:
>>>>> 18/01/2023 08:28, Andrew Rybchenko:
>>>>>> On 11/14/22 14:59, Rongwei Liu wrote:
>>>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>>>> then optimization can be done via allocation of this table.
>>>>>>> Such optimization is possible only if the application gives a hint
>>>>>>> about its usage of the table during initial configuration.
>>>>>>>
>>>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>>>> which may correspond to two kinds of underlayer resources.
>>>>>>> That's why the first two hints introduced in this patch are about
>>>>>>> wire and vport traffic specialization.
>>>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>>>> traffic initiated from VF/SF.
>>>>>>>
>>>>>>> There are two possible approaches for providing the hints.
>>>>>>> Using IPv4 as an example:
>>>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>>>
>>>>>>> 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 flow rule even if it's
>>>>>>> just a hint. No value to match because matching is already done by
>>>>>>> IPv4 item.
>>>>>>>
>>>>>>> 2. Add special flags into table_attr.
>>>>>>>
>>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>>>
>>>>>>> Approach 1 needs to specify the pattern in each flow rule which
>> wastes
>>>>>>> memory and is not user friendly.
>>>>>>> This patch takes the 2nd approach and introduces one new member
>>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>>>>>> optimization.
>>>>>>
>>>>>> The above description is misleading. It alternates options (1)
>>>>>> and (2), but in fact (2) requires (1) as well.
>>>>>
>>>>> Yes the above description may be misleading
>>>>> and it seems you are misleaded :)
>>>>
>>>> It is not my intention. If it is only my problem, I'm OK to
>>>> step back.
>>>
>>> It's OK to explain and check everything is OK, no worries.
>>> Thanks for reviewing.
>>>
>>>>> I will explain below why the option (2) doesn't require (1).
>>>>> I think we should apply the same example to both cases to make it clear:
>>>>>
>>>>> 1. Use pattern item in both template table and flow rules:
>>>>>
>>>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is
>> 255.255.255.255 / end
>>>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is
>> 1.1.1.1 / end
>>>>>
>>>>> The pattern template 3 will be used only to match flows coming from
>> vports.
>>>>> ANY_VPORT needs to be present in each flow rule.
>>>>
>>>> It looks like I lost something here. Why do we need to specify
>>>> it in each flow rule if the matching is already fixed in
>>>> template table?
>>>
>>> I think that's how template tables are designed.
>>> Ori, please could you point us to the relevant documentation?
>>>
>>>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
>>>>> the user knows 1.1.1.1 is the IP of a vport.
>>>>
>>>> What should happen if a packet with src IP 1.1.1.1 comes from
>>>> the wire? Almost anything could come from network.
>>>
>>> It a packet comes from a wired port AND
>>> the PMD did an optimization based on this hint,
>>> then the packet could be not matched.
>>
>> So, the hint changes matching results and therefore becomes
>> a strange (extra) matching criteria under specific
>> circumstance. It sounds bad. So, good application must use
>> real (always) matching criteria when composing flow rules.
>> So, RTE flow API should provide a way to write a good
>> application without extra pain.
>> That's why I'm saying that (2) requires (1) anyway.
>>
>
> I'm sorry I don't see why.
Will be in reply to Thomas
>
> This feature is about where to place the rule in the HW.
> This feature can be useful for any HW that as different pipelines for ingress
> and egress traffic. It can be used to save resources or if some actions can be done
> only on one direction then the PMD can allow them and not block the this rule.
>
>
>> It does not say that hint is not required at all.
>> It is still useful for resources usage optimization if
>> application knows how it is going to use particular table.
>>
>
> So we agree that the hint is good?
Acceptable since we need to optimize resources.
It would be better without it, but we need it.
On 2/1/23 14:18, Thomas Monjalon wrote:
> 01/02/2023 12:10, Andrew Rybchenko:
>> On 2/1/23 13:58, Thomas Monjalon wrote:
>>> 01/02/2023 11:17, Andrew Rybchenko:
>>>> On 1/18/23 19:18, Thomas Monjalon wrote:
>>>>> 18/01/2023 08:28, Andrew Rybchenko:
>>>>>> On 11/14/22 14:59, Rongwei Liu wrote:
>>>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>>>> then optimization can be done via allocation of this table.
>>>>>>> Such optimization is possible only if the application gives a hint
>>>>>>> about its usage of the table during initial configuration.
>>>>>>>
>>>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>>>> which may correspond to two kinds of underlayer resources.
>>>>>>> That's why the first two hints introduced in this patch are about
>>>>>>> wire and vport traffic specialization.
>>>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>>>> traffic initiated from VF/SF.
>>>>>>>
>>>>>>> There are two possible approaches for providing the hints.
>>>>>>> Using IPv4 as an example:
>>>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>>>
>>>>>>> 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 flow rule even if it's
>>>>>>> just a hint. No value to match because matching is already done by
>>>>>>> IPv4 item.
>>>>>>>
>>>>>>> 2. Add special flags into table_attr.
>>>>>>>
>>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>>>
>>>>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
>>>>>>> memory and is not user friendly.
>>>>>>> This patch takes the 2nd approach and introduces one new member
>>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>>>>>> optimization.
>>>>>>
>>>>>> The above description is misleading. It alternates options (1)
>>>>>> and (2), but in fact (2) requires (1) as well.
>>>>>
>>>>> Yes the above description may be misleading
>>>>> and it seems you are misleaded :)
>>>>
>>>> It is not my intention. If it is only my problem, I'm OK to
>>>> step back.
>>>
>>> It's OK to explain and check everything is OK, no worries.
>>> Thanks for reviewing.
>>>
>>>>> I will explain below why the option (2) doesn't require (1).
>>>>> I think we should apply the same example to both cases to make it clear:
>>>>>
>>>>> 1. Use pattern item in both template table and flow rules:
>>>>>
>>>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
>>>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
>>>>>
>>>>> The pattern template 3 will be used only to match flows coming from vports.
>>>>> ANY_VPORT needs to be present in each flow rule.
>>>>
>>>> It looks like I lost something here. Why do we need to specify
>>>> it in each flow rule if the matching is already fixed in
>>>> template table?
>>>
>>> I think that's how template tables are designed.
>>> Ori, please could you point us to the relevant documentation?
>>>
>>>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
>>>>> the user knows 1.1.1.1 is the IP of a vport.
>>>>
>>>> What should happen if a packet with src IP 1.1.1.1 comes from
>>>> the wire? Almost anything could come from network.
>>>
>>> It a packet comes from a wired port AND
>>> the PMD did an optimization based on this hint,
>>> then the packet could be not matched.
>>
>> So, the hint changes matching results and therefore becomes
>> a strange (extra) matching criteria under specific
>> circumstance. It sounds bad.
>
> In this case, the user made a wrong assumption.
> If the user does not do a mistake, the behavior should be the same
> whether the hint is used or ignored.
>
>> So, good application must use
>> real (always) matching criteria when composing flow rules.
>
> Of course, nothing replaces matching criteria.
>
>> So, RTE flow API should provide a way to write a good
>> application without extra pain.
>> That's why I'm saying that (2) requires (1) anyway.
>
> I don't follow this sentence.
> If you mean with hint, flow matching is still required, then yes,
> this is what I emphasized in my rewrite of the case (2) below.
>
>> It does not say that hint is not required at all.
>> It is still useful for resources usage optimization if
>> application knows how it is going to use particular table.
>
> Yes, that's an optional optimization.
> It should not change the rules,
> and it should not change the functional behavior
> if the user does not do mistakes.
So, we basically agree on the topic, but my goal here is a bit
bigger. Make it easier for a user to avoid mistakes. May be it
is stupid goal :) and all efforts are vain.
If we have a match item with similar functionality it would be
easy to just put it into a pattern. Otherwise, it could be
complicated, have high chances to be skipped and rely on
implicit matching criteria imposed by the hint on the HW
which takes it into account.
Hi Thomas,
On Wed, 1 Feb 2023, Thomas Monjalon wrote:
> 31/01/2023 06:30, Ivan Malov:
>> Hi Rongwei,
>>
>> OK, I hear ya. Thanks for persevering.
>>
>> I still hope community will comment on the possibility to
>> provide a hint mechanism for always-the-same match items,
>> with the perspective of becoming more versatile.
>
> Any hint could be imagined.
> But please keep this in mind: a hint is *not* a matching criteria,
> for the simple reason that a hint can be ignored by the PMD.
> So you cannot use a hint to avoid specifying a match item,
> but you could use a hint to specify that an item is the same
> for all the rules of a table.
Reading the same thought expressed in your words, the penny drops.
So, a hint then. But even not being a match criterion itself, it
is still confined to knowledge about a too special particularity.
If one needs to add similar hints for other aspects of matching,
they will have to add more and more bits to this namespace.
So why at all detach the namespace of hints from such of
the match items? A more generic solution might be needed.
In another email of yours, [1], you suggest that documentation be
improved. But it seems that addressing the "fixed match" issue
described by Ori (in the quote) could be that "more generic"
approach. For example, if one added "always_fixed_spec" bit
to struct rte_flow_item, this bit could be taken into
account by PMD in rte_flow_pattern_template_create().
When it has spotted this bit for item ANY_VPORT,
it will treat it the way this "specialise" hint
does, collecting the same upfront knowledge.
Yes, I do acknowledge that encountering such a bit in
a regular/sync flow parsing is irrelevant, but this
is just a general idea and not the final proposal.
Also, in mail [2], Ori talks about separate pipelines
for ingress and egress. That sheds some light on this
hint, thanks. On the one hand, yes, vendors do tend
to have separate pipelines for this, this and this,
but, on the other hand, assuming this particular
separation of pipelines and making a customised
hint for it might not be quite generic. It is
that special particularity which I am talking
about in the first paragraph of my response.
So why not combine addressing "fixed match" items
and solving the problem of this "direction" hint?
Again, I can't come up with an immediate example
of how precisely this could be useful, but since
DPDK strives to being as much generic/neutral as
possible, then why not consider this?
[1] https://mails.dpdk.org/archives/dev/2023-February/260667.html
[2] https://mails.dpdk.org/archives/dev/2023-February/260668.html
>
>
>> Other
>> than that, your current patch might be OK, but, again,
>> I think other reviewers' comments (if any) shall
>> be addressed. But no strong objections from me.
>>
>> By the way, for this "specialise" field, in your opinion,
>> which extra flags could emerge in future / would be nice
>> to have? I mean, is there any concept of what can be
>> added to this field's namespace and what can't be?
>
> I think there is no limit with hint flags to be added.
> I repeat it again: hints can be ignored by the PMDs.
>
>
>
Thank you.
01/02/2023 12:50, Ivan Malov:
> On Wed, 1 Feb 2023, Thomas Monjalon wrote:
> > 31/01/2023 06:30, Ivan Malov:
> >> I still hope community will comment on the possibility to
> >> provide a hint mechanism for always-the-same match items,
> >> with the perspective of becoming more versatile.
> >
> > Any hint could be imagined.
> > But please keep this in mind: a hint is *not* a matching criteria,
> > for the simple reason that a hint can be ignored by the PMD.
> > So you cannot use a hint to avoid specifying a match item,
> > but you could use a hint to specify that an item is the same
> > for all the rules of a table.
>
> Reading the same thought expressed in your words, the penny drops.
> So, a hint then. But even not being a match criterion itself, it
> is still confined to knowledge about a too special particularity.
> If one needs to add similar hints for other aspects of matching,
> they will have to add more and more bits to this namespace.
> So why at all detach the namespace of hints from such of
> the match items? A more generic solution might be needed.
The hints are not necessarily related to the matching.
That's why it is more flexible to have separate definitions.
> In another email of yours, [1], you suggest that documentation be
> improved. But it seems that addressing the "fixed match" issue
> described by Ori (in the quote) could be that "more generic"
> approach. For example, if one added "always_fixed_spec" bit
> to struct rte_flow_item, this bit could be taken into
> account by PMD in rte_flow_pattern_template_create().
> When it has spotted this bit for item ANY_VPORT,
> it will treat it the way this "specialise" hint
> does, collecting the same upfront knowledge.
>
> Yes, I do acknowledge that encountering such a bit in
> a regular/sync flow parsing is irrelevant, but this
> is just a general idea and not the final proposal.
Yes it does not have sense outside of template table.
> Also, in mail [2], Ori talks about separate pipelines
> for ingress and egress. That sheds some light on this
> hint, thanks. On the one hand, yes, vendors do tend
> to have separate pipelines for this, this and this,
> but, on the other hand, assuming this particular
> separation of pipelines and making a customised
> hint for it might not be quite generic. It is
> that special particularity which I am talking
> about in the first paragraph of my response.
>
> So why not combine addressing "fixed match" items
> and solving the problem of this "direction" hint?
I agree we should try to better address templates
with some fixed matching items, but we could still need
to have some hints for other kind of optimizations.
> Again, I can't come up with an immediate example
> of how precisely this could be useful, but since
> DPDK strives to being as much generic/neutral as
> possible, then why not consider this?
I agree the hints may be quite vendor-specifics,
but they are optional and does not hurt vendors not implementing them.
For applications supporting many drivers,
they can use some hints without losing portability.
So I don't think such approach is against genericity or neutrality,
it is just bringing some flexibility for the best performances.
And in DPDK, the first priority is the performance.
> [1] https://mails.dpdk.org/archives/dev/2023-February/260667.html
> [2] https://mails.dpdk.org/archives/dev/2023-February/260668.html
>
> >> Other
> >> than that, your current patch might be OK, but, again,
> >> I think other reviewers' comments (if any) shall
> >> be addressed. But no strong objections from me.
> >>
> >> By the way, for this "specialise" field, in your opinion,
> >> which extra flags could emerge in future / would be nice
> >> to have? I mean, is there any concept of what can be
> >> added to this field's namespace and what can't be?
> >
> > I think there is no limit with hint flags to be added.
> > I repeat it again: hints can be ignored by the PMDs.
>
> Thank you.
The template flow API is experimental and will probably remain as such
for a long time, so if you find a more elegant approach,
we will consider it.
But given we don't know how to make it better today,
and there is no real problem with its definition,
I don't see a reason to postpone its integration as experimental.
In my opinion, having hint is good.
The real discussion is on the flags.
If we find how to manage the same optimization without these flags,
we could drop them, but the hint flexibility should remain.
01/02/2023 12:38, Andrew Rybchenko:
> On 2/1/23 14:18, Thomas Monjalon wrote:
> > 01/02/2023 12:10, Andrew Rybchenko:
> >> On 2/1/23 13:58, Thomas Monjalon wrote:
> >>> 01/02/2023 11:17, Andrew Rybchenko:
> >>>> On 1/18/23 19:18, Thomas Monjalon wrote:
> >>>>> 18/01/2023 08:28, Andrew Rybchenko:
> >>>>>> On 11/14/22 14:59, Rongwei Liu wrote:
> >>>>>>> In case flow rules match only one kind of traffic in a flow table,
> >>>>>>> then optimization can be done via allocation of this table.
> >>>>>>> Such optimization is possible only if the application gives a hint
> >>>>>>> about its usage of the table during initial configuration.
> >>>>>>>
> >>>>>>> The transfer domain rules may process traffic from wire or vport,
> >>>>>>> which may correspond to two kinds of underlayer resources.
> >>>>>>> That's why the first two hints introduced in this patch are about
> >>>>>>> wire and vport traffic specialization.
> >>>>>>> Wire means traffic arrives from the uplink port while vport means
> >>>>>>> traffic initiated from VF/SF.
> >>>>>>>
> >>>>>>> There are two possible approaches for providing the hints.
> >>>>>>> Using IPv4 as an example:
> >>>>>>> 1. Use pattern item in both template table and flow rules.
> >>>>>>>
> >>>>>>> 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 flow rule even if it's
> >>>>>>> just a hint. No value to match because matching is already done by
> >>>>>>> IPv4 item.
> >>>>>>>
> >>>>>>> 2. Add special flags into table_attr.
> >>>>>>>
> >>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>>>>>
> >>>>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
> >>>>>>> memory and is not user friendly.
> >>>>>>> This patch takes the 2nd approach and introduces one new member
> >>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
> >>>>>>> optimization.
> >>>>>>
> >>>>>> The above description is misleading. It alternates options (1)
> >>>>>> and (2), but in fact (2) requires (1) as well.
> >>>>>
> >>>>> Yes the above description may be misleading
> >>>>> and it seems you are misleaded :)
> >>>>
> >>>> It is not my intention. If it is only my problem, I'm OK to
> >>>> step back.
> >>>
> >>> It's OK to explain and check everything is OK, no worries.
> >>> Thanks for reviewing.
> >>>
> >>>>> I will explain below why the option (2) doesn't require (1).
> >>>>> I think we should apply the same example to both cases to make it clear:
> >>>>>
> >>>>> 1. Use pattern item in both template table and flow rules:
> >>>>>
> >>>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
> >>>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
> >>>>>
> >>>>> The pattern template 3 will be used only to match flows coming from vports.
> >>>>> ANY_VPORT needs to be present in each flow rule.
> >>>>
> >>>> It looks like I lost something here. Why do we need to specify
> >>>> it in each flow rule if the matching is already fixed in
> >>>> template table?
> >>>
> >>> I think that's how template tables are designed.
> >>> Ori, please could you point us to the relevant documentation?
> >>>
> >>>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> >>>>> the user knows 1.1.1.1 is the IP of a vport.
> >>>>
> >>>> What should happen if a packet with src IP 1.1.1.1 comes from
> >>>> the wire? Almost anything could come from network.
> >>>
> >>> It a packet comes from a wired port AND
> >>> the PMD did an optimization based on this hint,
> >>> then the packet could be not matched.
> >>
> >> So, the hint changes matching results and therefore becomes
> >> a strange (extra) matching criteria under specific
> >> circumstance. It sounds bad.
> >
> > In this case, the user made a wrong assumption.
> > If the user does not do a mistake, the behavior should be the same
> > whether the hint is used or ignored.
> >
> >> So, good application must use
> >> real (always) matching criteria when composing flow rules.
> >
> > Of course, nothing replaces matching criteria.
> >
> >> So, RTE flow API should provide a way to write a good
> >> application without extra pain.
> >> That's why I'm saying that (2) requires (1) anyway.
> >
> > I don't follow this sentence.
> > If you mean with hint, flow matching is still required, then yes,
> > this is what I emphasized in my rewrite of the case (2) below.
> >
> >> It does not say that hint is not required at all.
> >> It is still useful for resources usage optimization if
> >> application knows how it is going to use particular table.
> >
> > Yes, that's an optional optimization.
> > It should not change the rules,
> > and it should not change the functional behavior
> > if the user does not do mistakes.
>
> So, we basically agree on the topic, but my goal here is a bit
> bigger. Make it easier for a user to avoid mistakes. May be it
> is stupid goal :) and all efforts are vain.
> If we have a match item with similar functionality it would be
> easy to just put it into a pattern. Otherwise, it could be
> complicated, have high chances to be skipped and rely on
> implicit matching criteria imposed by the hint on the HW
> which takes it into account.
We may highlight in the doc that the functional behaviour must not rely
on the hints. It is only optional optimization and effects may vary
with differents driver.
What do you think? I don't know what else to do about user mistakes :)
On Wed, 1 Feb 2023, Thomas Monjalon wrote:
> 01/02/2023 12:50, Ivan Malov:
>> On Wed, 1 Feb 2023, Thomas Monjalon wrote:
>>> 31/01/2023 06:30, Ivan Malov:
>>>> I still hope community will comment on the possibility to
>>>> provide a hint mechanism for always-the-same match items,
>>>> with the perspective of becoming more versatile.
>>>
>>> Any hint could be imagined.
>>> But please keep this in mind: a hint is *not* a matching criteria,
>>> for the simple reason that a hint can be ignored by the PMD.
>>> So you cannot use a hint to avoid specifying a match item,
>>> but you could use a hint to specify that an item is the same
>>> for all the rules of a table.
>>
>> Reading the same thought expressed in your words, the penny drops.
>> So, a hint then. But even not being a match criterion itself, it
>> is still confined to knowledge about a too special particularity.
>> If one needs to add similar hints for other aspects of matching,
>> they will have to add more and more bits to this namespace.
>> So why at all detach the namespace of hints from such of
>> the match items? A more generic solution might be needed.
>
> The hints are not necessarily related to the matching.
> That's why it is more flexible to have separate definitions.
I hear ya.
>
>> In another email of yours, [1], you suggest that documentation be
>> improved. But it seems that addressing the "fixed match" issue
>> described by Ori (in the quote) could be that "more generic"
>> approach. For example, if one added "always_fixed_spec" bit
>> to struct rte_flow_item, this bit could be taken into
>> account by PMD in rte_flow_pattern_template_create().
>> When it has spotted this bit for item ANY_VPORT,
>> it will treat it the way this "specialise" hint
>> does, collecting the same upfront knowledge.
>>
>> Yes, I do acknowledge that encountering such a bit in
>> a regular/sync flow parsing is irrelevant, but this
>> is just a general idea and not the final proposal.
>
> Yes it does not have sense outside of template table.
Yes, but something similar can be devised to attach
some "always the same exact spec" meaning to given
items in the pattern. Not necessarily via this bit.
>
>> Also, in mail [2], Ori talks about separate pipelines
>> for ingress and egress. That sheds some light on this
>> hint, thanks. On the one hand, yes, vendors do tend
>> to have separate pipelines for this, this and this,
>> but, on the other hand, assuming this particular
>> separation of pipelines and making a customised
>> hint for it might not be quite generic. It is
>> that special particularity which I am talking
>> about in the first paragraph of my response.
>>
>> So why not combine addressing "fixed match" items
>> and solving the problem of this "direction" hint?
>
> I agree we should try to better address templates
> with some fixed matching items, but we could still need
> to have some hints for other kind of optimizations.
Agreed.
>
>> Again, I can't come up with an immediate example
>> of how precisely this could be useful, but since
>> DPDK strives to being as much generic/neutral as
>> possible, then why not consider this?
>
> I agree the hints may be quite vendor-specifics,
> but they are optional and does not hurt vendors not implementing them.
> For applications supporting many drivers,
> they can use some hints without losing portability.
>
> So I don't think such approach is against genericity or neutrality,
> it is just bringing some flexibility for the best performances.
> And in DPDK, the first priority is the performance.
I'm questioning this because I suspect that, had the original
flow design had more flexibility / better decomposition,
then perhaps it would have been unneeded to add these
extra hints in the first instance. I don't mean to
criticise them too much, though.
>
>> [1] https://mails.dpdk.org/archives/dev/2023-February/260667.html
>> [2] https://mails.dpdk.org/archives/dev/2023-February/260668.html
>>
>>>> Other
>>>> than that, your current patch might be OK, but, again,
>>>> I think other reviewers' comments (if any) shall
>>>> be addressed. But no strong objections from me.
>>>>
>>>> By the way, for this "specialise" field, in your opinion,
>>>> which extra flags could emerge in future / would be nice
>>>> to have? I mean, is there any concept of what can be
>>>> added to this field's namespace and what can't be?
>>>
>>> I think there is no limit with hint flags to be added.
>>> I repeat it again: hints can be ignored by the PMDs.
>>
>> Thank you.
>
> The template flow API is experimental and will probably remain as such
> for a long time, so if you find a more elegant approach,
> we will consider it.
Thanks for explaining this.
You know, now you mention it, are there non-debug app
examples available that make use of this template API?
Back in the day, I reviewed the template API, but
since then, I've never come across any real-life
examples. I'd appreciate you point something out.
> But given we don't know how to make it better today,
> and there is no real problem with its definition,
> I don't see a reason to postpone its integration as experimental.
>
> In my opinion, having hint is good.
> The real discussion is on the flags.
> If we find how to manage the same optimization without these flags,
> we could drop them, but the hint flexibility should remain.
>
>
>
Thank you.
01/02/2023 15:04, Ivan Malov:
> On Wed, 1 Feb 2023, Thomas Monjalon wrote:
> > The template flow API is experimental and will probably remain as such
> > for a long time, so if you find a more elegant approach,
> > we will consider it.
>
> Thanks for explaining this.
>
> You know, now you mention it, are there non-debug app
> examples available that make use of this template API?
> Back in the day, I reviewed the template API, but
> since then, I've never come across any real-life
> examples. I'd appreciate you point something out.
I'm not aware of any Open Source project using the flow template API.
But I can tell you that some companies are using it
and it is the only way for them to reach their flow performance target.
I would love to have an open router application truly based on DPDK.
Hi Ivan,
> -----Original Message-----
> From: Ivan Malov <ivan.malov@arknetworks.am>
> Sent: Wednesday, 1 February 2023 16:04
>
> On Wed, 1 Feb 2023, Thomas Monjalon wrote:
>
> > 01/02/2023 12:50, Ivan Malov:
> >> On Wed, 1 Feb 2023, Thomas Monjalon wrote:
> >>> 31/01/2023 06:30, Ivan Malov:
> >>>> I still hope community will comment on the possibility to
> >>>> provide a hint mechanism for always-the-same match items,
> >>>> with the perspective of becoming more versatile.
> >>>
> >>> Any hint could be imagined.
> >>> But please keep this in mind: a hint is *not* a matching criteria,
> >>> for the simple reason that a hint can be ignored by the PMD.
> >>> So you cannot use a hint to avoid specifying a match item,
> >>> but you could use a hint to specify that an item is the same
> >>> for all the rules of a table.
> >>
> >> Reading the same thought expressed in your words, the penny drops.
> >> So, a hint then. But even not being a match criterion itself, it
> >> is still confined to knowledge about a too special particularity.
> >> If one needs to add similar hints for other aspects of matching,
> >> they will have to add more and more bits to this namespace.
> >> So why at all detach the namespace of hints from such of
> >> the match items? A more generic solution might be needed.
> >
> > The hints are not necessarily related to the matching.
> > That's why it is more flexible to have separate definitions.
>
> I hear ya.
>
> >
> >> In another email of yours, [1], you suggest that documentation be
> >> improved. But it seems that addressing the "fixed match" issue
> >> described by Ori (in the quote) could be that "more generic"
> >> approach. For example, if one added "always_fixed_spec" bit
> >> to struct rte_flow_item, this bit could be taken into
> >> account by PMD in rte_flow_pattern_template_create().
> >> When it has spotted this bit for item ANY_VPORT,
> >> it will treat it the way this "specialise" hint
> >> does, collecting the same upfront knowledge.
> >>
> >> Yes, I do acknowledge that encountering such a bit in
> >> a regular/sync flow parsing is irrelevant, but this
> >> is just a general idea and not the final proposal.
> >
> > Yes it does not have sense outside of template table.
>
> Yes, but something similar can be devised to attach
> some "always the same exact spec" meaning to given
> items in the pattern. Not necessarily via this bit.
>
We thought about it but it made the api to complex but we can revisit it.
I'm open to suggestions.
> >
> >> Also, in mail [2], Ori talks about separate pipelines
> >> for ingress and egress. That sheds some light on this
> >> hint, thanks. On the one hand, yes, vendors do tend
> >> to have separate pipelines for this, this and this,
> >> but, on the other hand, assuming this particular
> >> separation of pipelines and making a customised
> >> hint for it might not be quite generic. It is
> >> that special particularity which I am talking
> >> about in the first paragraph of my response.
> >>
> >> So why not combine addressing "fixed match" items
> >> and solving the problem of this "direction" hint?
> >
> > I agree we should try to better address templates
> > with some fixed matching items, but we could still need
> > to have some hints for other kind of optimizations.
>
> Agreed.
>
+1
> >
> >> Again, I can't come up with an immediate example
> >> of how precisely this could be useful, but since
> >> DPDK strives to being as much generic/neutral as
> >> possible, then why not consider this?
> >
> > I agree the hints may be quite vendor-specifics,
> > but they are optional and does not hurt vendors not implementing them.
> > For applications supporting many drivers,
> > they can use some hints without losing portability.
> >
> > So I don't think such approach is against genericity or neutrality,
> > it is just bringing some flexibility for the best performances.
> > And in DPDK, the first priority is the performance.
>
> I'm questioning this because I suspect that, had the original
> flow design had more flexibility / better decomposition,
> then perhaps it would have been unneeded to add these
> extra hints in the first instance. I don't mean to
> criticise them too much, though.
>
One of the problems is that the API is too flexible this means that
PMD must assume everything so it can't optimize, this is why we
need hints .
> >
> >> [1] https://mails.dpdk.org/archives/dev/2023-February/260667.html
> >> [2] https://mails.dpdk.org/archives/dev/2023-February/260668.html
> >>
> >>>> Other
> >>>> than that, your current patch might be OK, but, again,
> >>>> I think other reviewers' comments (if any) shall
> >>>> be addressed. But no strong objections from me.
> >>>>
> >>>> By the way, for this "specialise" field, in your opinion,
> >>>> which extra flags could emerge in future / would be nice
> >>>> to have? I mean, is there any concept of what can be
> >>>> added to this field's namespace and what can't be?
> >>>
> >>> I think there is no limit with hint flags to be added.
> >>> I repeat it again: hints can be ignored by the PMDs.
> >>
> >> Thank you.
> >
> > The template flow API is experimental and will probably remain as such
> > for a long time, so if you find a more elegant approach,
> > we will consider it.
>
> Thanks for explaining this.
>
> You know, now you mention it, are there non-debug app
> examples available that make use of this template API?
> Back in the day, I reviewed the template API, but
> since then, I've never come across any real-life
> examples. I'd appreciate you point something out.
>
> > But given we don't know how to make it better today,
> > and there is no real problem with its definition,
> > I don't see a reason to postpone its integration as experimental.
> >
> > In my opinion, having hint is good.
> > The real discussion is on the flags.
> > If we find how to manage the same optimization without these flags,
> > we could drop them, but the hint flexibility should remain.
> >
> >
> >
>
> Thank you.
On 2/1/23 16:48, Thomas Monjalon wrote:
> 01/02/2023 12:38, Andrew Rybchenko:
>> On 2/1/23 14:18, Thomas Monjalon wrote:
>>> 01/02/2023 12:10, Andrew Rybchenko:
>>>> On 2/1/23 13:58, Thomas Monjalon wrote:
>>>>> 01/02/2023 11:17, Andrew Rybchenko:
>>>>>> On 1/18/23 19:18, Thomas Monjalon wrote:
>>>>>>> 18/01/2023 08:28, Andrew Rybchenko:
>>>>>>>> On 11/14/22 14:59, Rongwei Liu wrote:
>>>>>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>>>>>> then optimization can be done via allocation of this table.
>>>>>>>>> Such optimization is possible only if the application gives a hint
>>>>>>>>> about its usage of the table during initial configuration.
>>>>>>>>>
>>>>>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>>>>>> which may correspond to two kinds of underlayer resources.
>>>>>>>>> That's why the first two hints introduced in this patch are about
>>>>>>>>> wire and vport traffic specialization.
>>>>>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>>>>>> traffic initiated from VF/SF.
>>>>>>>>>
>>>>>>>>> There are two possible approaches for providing the hints.
>>>>>>>>> Using IPv4 as an example:
>>>>>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>>>>>
>>>>>>>>> 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 flow rule even if it's
>>>>>>>>> just a hint. No value to match because matching is already done by
>>>>>>>>> IPv4 item.
>>>>>>>>>
>>>>>>>>> 2. Add special flags into table_attr.
>>>>>>>>>
>>>>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>>>>>
>>>>>>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
>>>>>>>>> memory and is not user friendly.
>>>>>>>>> This patch takes the 2nd approach and introduces one new member
>>>>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>>>>>>>> optimization.
>>>>>>>>
>>>>>>>> The above description is misleading. It alternates options (1)
>>>>>>>> and (2), but in fact (2) requires (1) as well.
>>>>>>>
>>>>>>> Yes the above description may be misleading
>>>>>>> and it seems you are misleaded :)
>>>>>>
>>>>>> It is not my intention. If it is only my problem, I'm OK to
>>>>>> step back.
>>>>>
>>>>> It's OK to explain and check everything is OK, no worries.
>>>>> Thanks for reviewing.
>>>>>
>>>>>>> I will explain below why the option (2) doesn't require (1).
>>>>>>> I think we should apply the same example to both cases to make it clear:
>>>>>>>
>>>>>>> 1. Use pattern item in both template table and flow rules:
>>>>>>>
>>>>>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
>>>>>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
>>>>>>>
>>>>>>> The pattern template 3 will be used only to match flows coming from vports.
>>>>>>> ANY_VPORT needs to be present in each flow rule.
>>>>>>
>>>>>> It looks like I lost something here. Why do we need to specify
>>>>>> it in each flow rule if the matching is already fixed in
>>>>>> template table?
>>>>>
>>>>> I think that's how template tables are designed.
>>>>> Ori, please could you point us to the relevant documentation?
>>>>>
>>>>>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
>>>>>>> the user knows 1.1.1.1 is the IP of a vport.
>>>>>>
>>>>>> What should happen if a packet with src IP 1.1.1.1 comes from
>>>>>> the wire? Almost anything could come from network.
>>>>>
>>>>> It a packet comes from a wired port AND
>>>>> the PMD did an optimization based on this hint,
>>>>> then the packet could be not matched.
>>>>
>>>> So, the hint changes matching results and therefore becomes
>>>> a strange (extra) matching criteria under specific
>>>> circumstance. It sounds bad.
>>>
>>> In this case, the user made a wrong assumption.
>>> If the user does not do a mistake, the behavior should be the same
>>> whether the hint is used or ignored.
>>>
>>>> So, good application must use
>>>> real (always) matching criteria when composing flow rules.
>>>
>>> Of course, nothing replaces matching criteria.
>>>
>>>> So, RTE flow API should provide a way to write a good
>>>> application without extra pain.
>>>> That's why I'm saying that (2) requires (1) anyway.
>>>
>>> I don't follow this sentence.
>>> If you mean with hint, flow matching is still required, then yes,
>>> this is what I emphasized in my rewrite of the case (2) below.
>>>
>>>> It does not say that hint is not required at all.
>>>> It is still useful for resources usage optimization if
>>>> application knows how it is going to use particular table.
>>>
>>> Yes, that's an optional optimization.
>>> It should not change the rules,
>>> and it should not change the functional behavior
>>> if the user does not do mistakes.
>>
>> So, we basically agree on the topic, but my goal here is a bit
>> bigger. Make it easier for a user to avoid mistakes. May be it
>> is stupid goal :) and all efforts are vain.
>> If we have a match item with similar functionality it would be
>> easy to just put it into a pattern. Otherwise, it could be
>> complicated, have high chances to be skipped and rely on
>> implicit matching criteria imposed by the hint on the HW
>> which takes it into account.
>
> We may highlight in the doc that the functional behaviour must not rely
> on the hints. It is only optional optimization and effects may vary
> with differents driver.
> What do you think? I don't know what else to do about user mistakes :)
As I said - add corresponding pattern items.
Anyway, hint itself is OK and makes sense. Hopefully
documentation highlights that pattern match is required.
If so,
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
02/02/2023 10:21, Andrew Rybchenko:
> On 2/1/23 16:48, Thomas Monjalon wrote:
> > 01/02/2023 12:38, Andrew Rybchenko:
> >> On 2/1/23 14:18, Thomas Monjalon wrote:
> >>> 01/02/2023 12:10, Andrew Rybchenko:
> >>>> On 2/1/23 13:58, Thomas Monjalon wrote:
> >>>>> 01/02/2023 11:17, Andrew Rybchenko:
> >>>>>> On 1/18/23 19:18, Thomas Monjalon wrote:
> >>>>>>> 18/01/2023 08:28, Andrew Rybchenko:
> >>>>>>>> On 11/14/22 14:59, Rongwei Liu wrote:
> >>>>>>>>> In case flow rules match only one kind of traffic in a flow table,
> >>>>>>>>> then optimization can be done via allocation of this table.
> >>>>>>>>> Such optimization is possible only if the application gives a hint
> >>>>>>>>> about its usage of the table during initial configuration.
> >>>>>>>>>
> >>>>>>>>> The transfer domain rules may process traffic from wire or vport,
> >>>>>>>>> which may correspond to two kinds of underlayer resources.
> >>>>>>>>> That's why the first two hints introduced in this patch are about
> >>>>>>>>> wire and vport traffic specialization.
> >>>>>>>>> Wire means traffic arrives from the uplink port while vport means
> >>>>>>>>> traffic initiated from VF/SF.
> >>>>>>>>>
> >>>>>>>>> There are two possible approaches for providing the hints.
> >>>>>>>>> Using IPv4 as an example:
> >>>>>>>>> 1. Use pattern item in both template table and flow rules.
> >>>>>>>>>
> >>>>>>>>> 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 flow rule even if it's
> >>>>>>>>> just a hint. No value to match because matching is already done by
> >>>>>>>>> IPv4 item.
> >>>>>>>>>
> >>>>>>>>> 2. Add special flags into table_attr.
> >>>>>>>>>
> >>>>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
> >>>>>>>>>
> >>>>>>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
> >>>>>>>>> memory and is not user friendly.
> >>>>>>>>> This patch takes the 2nd approach and introduces one new member
> >>>>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
> >>>>>>>>> optimization.
> >>>>>>>>
> >>>>>>>> The above description is misleading. It alternates options (1)
> >>>>>>>> and (2), but in fact (2) requires (1) as well.
> >>>>>>>
> >>>>>>> Yes the above description may be misleading
> >>>>>>> and it seems you are misleaded :)
> >>>>>>
> >>>>>> It is not my intention. If it is only my problem, I'm OK to
> >>>>>> step back.
> >>>>>
> >>>>> It's OK to explain and check everything is OK, no worries.
> >>>>> Thanks for reviewing.
> >>>>>
> >>>>>>> I will explain below why the option (2) doesn't require (1).
> >>>>>>> I think we should apply the same example to both cases to make it clear:
> >>>>>>>
> >>>>>>> 1. Use pattern item in both template table and flow rules:
> >>>>>>>
> >>>>>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
> >>>>>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
> >>>>>>>
> >>>>>>> The pattern template 3 will be used only to match flows coming from vports.
> >>>>>>> ANY_VPORT needs to be present in each flow rule.
> >>>>>>
> >>>>>> It looks like I lost something here. Why do we need to specify
> >>>>>> it in each flow rule if the matching is already fixed in
> >>>>>> template table?
> >>>>>
> >>>>> I think that's how template tables are designed.
> >>>>> Ori, please could you point us to the relevant documentation?
> >>>>>
> >>>>>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> >>>>>>> the user knows 1.1.1.1 is the IP of a vport.
> >>>>>>
> >>>>>> What should happen if a packet with src IP 1.1.1.1 comes from
> >>>>>> the wire? Almost anything could come from network.
> >>>>>
> >>>>> It a packet comes from a wired port AND
> >>>>> the PMD did an optimization based on this hint,
> >>>>> then the packet could be not matched.
> >>>>
> >>>> So, the hint changes matching results and therefore becomes
> >>>> a strange (extra) matching criteria under specific
> >>>> circumstance. It sounds bad.
> >>>
> >>> In this case, the user made a wrong assumption.
> >>> If the user does not do a mistake, the behavior should be the same
> >>> whether the hint is used or ignored.
> >>>
> >>>> So, good application must use
> >>>> real (always) matching criteria when composing flow rules.
> >>>
> >>> Of course, nothing replaces matching criteria.
> >>>
> >>>> So, RTE flow API should provide a way to write a good
> >>>> application without extra pain.
> >>>> That's why I'm saying that (2) requires (1) anyway.
> >>>
> >>> I don't follow this sentence.
> >>> If you mean with hint, flow matching is still required, then yes,
> >>> this is what I emphasized in my rewrite of the case (2) below.
> >>>
> >>>> It does not say that hint is not required at all.
> >>>> It is still useful for resources usage optimization if
> >>>> application knows how it is going to use particular table.
> >>>
> >>> Yes, that's an optional optimization.
> >>> It should not change the rules,
> >>> and it should not change the functional behavior
> >>> if the user does not do mistakes.
> >>
> >> So, we basically agree on the topic, but my goal here is a bit
> >> bigger. Make it easier for a user to avoid mistakes. May be it
> >> is stupid goal :) and all efforts are vain.
> >> If we have a match item with similar functionality it would be
> >> easy to just put it into a pattern. Otherwise, it could be
> >> complicated, have high chances to be skipped and rely on
> >> implicit matching criteria imposed by the hint on the HW
> >> which takes it into account.
> >
> > We may highlight in the doc that the functional behaviour must not rely
> > on the hints. It is only optional optimization and effects may vary
> > with differents driver.
> > What do you think? I don't know what else to do about user mistakes :)
>
> As I said - add corresponding pattern items.
I think I get it now.
You suggest to have pattern items for VPORT and PHY_PORT,
so the user won't be tempted to use hint for such matching?
We used to have RTE_FLOW_ITEM_TYPE_PHY_PORT, we could think about it.
> Anyway, hint itself is OK and makes sense. Hopefully
> documentation highlights that pattern match is required.
Yes we did an effort to highlight what are hints in the last version.
> If so,
>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
On 2/2/23 14:29, Thomas Monjalon wrote:
> 02/02/2023 10:21, Andrew Rybchenko:
>> On 2/1/23 16:48, Thomas Monjalon wrote:
>>> 01/02/2023 12:38, Andrew Rybchenko:
>>>> On 2/1/23 14:18, Thomas Monjalon wrote:
>>>>> 01/02/2023 12:10, Andrew Rybchenko:
>>>>>> On 2/1/23 13:58, Thomas Monjalon wrote:
>>>>>>> 01/02/2023 11:17, Andrew Rybchenko:
>>>>>>>> On 1/18/23 19:18, Thomas Monjalon wrote:
>>>>>>>>> 18/01/2023 08:28, Andrew Rybchenko:
>>>>>>>>>> On 11/14/22 14:59, Rongwei Liu wrote:
>>>>>>>>>>> In case flow rules match only one kind of traffic in a flow table,
>>>>>>>>>>> then optimization can be done via allocation of this table.
>>>>>>>>>>> Such optimization is possible only if the application gives a hint
>>>>>>>>>>> about its usage of the table during initial configuration.
>>>>>>>>>>>
>>>>>>>>>>> The transfer domain rules may process traffic from wire or vport,
>>>>>>>>>>> which may correspond to two kinds of underlayer resources.
>>>>>>>>>>> That's why the first two hints introduced in this patch are about
>>>>>>>>>>> wire and vport traffic specialization.
>>>>>>>>>>> Wire means traffic arrives from the uplink port while vport means
>>>>>>>>>>> traffic initiated from VF/SF.
>>>>>>>>>>>
>>>>>>>>>>> There are two possible approaches for providing the hints.
>>>>>>>>>>> Using IPv4 as an example:
>>>>>>>>>>> 1. Use pattern item in both template table and flow rules.
>>>>>>>>>>>
>>>>>>>>>>> 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 flow rule even if it's
>>>>>>>>>>> just a hint. No value to match because matching is already done by
>>>>>>>>>>> IPv4 item.
>>>>>>>>>>>
>>>>>>>>>>> 2. Add special flags into table_attr.
>>>>>>>>>>>
>>>>>>>>>>> template_table 0 create table_id 0 group 1 transfer vport_orig
>>>>>>>>>>>
>>>>>>>>>>> Approach 1 needs to specify the pattern in each flow rule which wastes
>>>>>>>>>>> memory and is not user friendly.
>>>>>>>>>>> This patch takes the 2nd approach and introduces one new member
>>>>>>>>>>> "specialize" into rte_flow_table_attr to indicate possible flow table
>>>>>>>>>>> optimization.
>>>>>>>>>>
>>>>>>>>>> The above description is misleading. It alternates options (1)
>>>>>>>>>> and (2), but in fact (2) requires (1) as well.
>>>>>>>>>
>>>>>>>>> Yes the above description may be misleading
>>>>>>>>> and it seems you are misleaded :)
>>>>>>>>
>>>>>>>> It is not my intention. If it is only my problem, I'm OK to
>>>>>>>> step back.
>>>>>>>
>>>>>>> It's OK to explain and check everything is OK, no worries.
>>>>>>> Thanks for reviewing.
>>>>>>>
>>>>>>>>> I will explain below why the option (2) doesn't require (1).
>>>>>>>>> I think we should apply the same example to both cases to make it clear:
>>>>>>>>>
>>>>>>>>> 1. Use pattern item in both template table and flow rules:
>>>>>>>>>
>>>>>>>>> template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is 255.255.255.255 / end
>>>>>>>>> flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is 1.1.1.1 / end
>>>>>>>>>
>>>>>>>>> The pattern template 3 will be used only to match flows coming from vports.
>>>>>>>>> ANY_VPORT needs to be present in each flow rule.
>>>>>>>>
>>>>>>>> It looks like I lost something here. Why do we need to specify
>>>>>>>> it in each flow rule if the matching is already fixed in
>>>>>>>> template table?
>>>>>>>
>>>>>>> I think that's how template tables are designed.
>>>>>>> Ori, please could you point us to the relevant documentation?
>>>>>>>
>>>>>>>>> ANY_VPORT matching is redundant with IP src 1.1.1.1 because
>>>>>>>>> the user knows 1.1.1.1 is the IP of a vport.
>>>>>>>>
>>>>>>>> What should happen if a packet with src IP 1.1.1.1 comes from
>>>>>>>> the wire? Almost anything could come from network.
>>>>>>>
>>>>>>> It a packet comes from a wired port AND
>>>>>>> the PMD did an optimization based on this hint,
>>>>>>> then the packet could be not matched.
>>>>>>
>>>>>> So, the hint changes matching results and therefore becomes
>>>>>> a strange (extra) matching criteria under specific
>>>>>> circumstance. It sounds bad.
>>>>>
>>>>> In this case, the user made a wrong assumption.
>>>>> If the user does not do a mistake, the behavior should be the same
>>>>> whether the hint is used or ignored.
>>>>>
>>>>>> So, good application must use
>>>>>> real (always) matching criteria when composing flow rules.
>>>>>
>>>>> Of course, nothing replaces matching criteria.
>>>>>
>>>>>> So, RTE flow API should provide a way to write a good
>>>>>> application without extra pain.
>>>>>> That's why I'm saying that (2) requires (1) anyway.
>>>>>
>>>>> I don't follow this sentence.
>>>>> If you mean with hint, flow matching is still required, then yes,
>>>>> this is what I emphasized in my rewrite of the case (2) below.
>>>>>
>>>>>> It does not say that hint is not required at all.
>>>>>> It is still useful for resources usage optimization if
>>>>>> application knows how it is going to use particular table.
>>>>>
>>>>> Yes, that's an optional optimization.
>>>>> It should not change the rules,
>>>>> and it should not change the functional behavior
>>>>> if the user does not do mistakes.
>>>>
>>>> So, we basically agree on the topic, but my goal here is a bit
>>>> bigger. Make it easier for a user to avoid mistakes. May be it
>>>> is stupid goal :) and all efforts are vain.
>>>> If we have a match item with similar functionality it would be
>>>> easy to just put it into a pattern. Otherwise, it could be
>>>> complicated, have high chances to be skipped and rely on
>>>> implicit matching criteria imposed by the hint on the HW
>>>> which takes it into account.
>>>
>>> We may highlight in the doc that the functional behaviour must not rely
>>> on the hints. It is only optional optimization and effects may vary
>>> with differents driver.
>>> What do you think? I don't know what else to do about user mistakes :)
>>
>> As I said - add corresponding pattern items.
>
> I think I get it now.
> You suggest to have pattern items for VPORT and PHY_PORT,
> so the user won't be tempted to use hint for such matching?
> We used to have RTE_FLOW_ITEM_TYPE_PHY_PORT, we could think about it.
Naming is bad since we had PHY_PORT before, but you get the
idea right.
>
>> Anyway, hint itself is OK and makes sense. Hopefully
>> documentation highlights that pattern match is required.
>
> Yes we did an effort to highlight what are hints in the last version.
>
>> If so,
>>
>> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>
>
>
@@ -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_TABLE_SPECIALIZE_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_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG;
+ return len;
default:
return -1;
}
@@ -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
-----------------------
@@ -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}
@@ -5186,6 +5186,29 @@ rte_flow_actions_template_destroy(uint16_t port_id,
*/
struct rte_flow_template_table;
+/**@{@name Special optional flags for template table attribute
+ * Each bit is a hint for table specialization,
+ * offering a potential optimization at PMD layer.
+ * PMD can ignore the unsupported bits silently.
+ */
+/**
+ * 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.
+ */
+#define RTE_FLOW_TABLE_SPECIALIZE_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.
+ */
+#define RTE_FLOW_TABLE_SPECIALIZE_TRANSFER_VPORT_ORIG RTE_BIT32(1)
+/**@}*/
+
/**
* @warning
* @b EXPERIMENTAL: this API may change without prior notice.
@@ -5201,6 +5224,11 @@ 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.
+ * Value is composed with RTE_FLOW_TABLE_SPECIALIZE_*.
+ */
+ uint32_t specialize;
};
/**