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

[Ivan Malov <ivan.malov@arknetworks.am>]

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.