From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0D14941B89; Tue, 31 Jan 2023 06:30:19 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DF68340E28; Tue, 31 Jan 2023 06:30:18 +0100 (CET) Received: from agw.arknetworks.am (agw.arknetworks.am [79.141.165.80]) by mails.dpdk.org (Postfix) with ESMTP id 40F4E40DFB for ; Tue, 31 Jan 2023 06:30:17 +0100 (CET) Received: from debian (unknown [78.109.72.147]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by agw.arknetworks.am (Postfix) with ESMTPSA id 5793CE004B; Tue, 31 Jan 2023 09:30:16 +0400 (+04) Date: Tue, 31 Jan 2023 09:30:21 +0400 (+04) From: Ivan Malov To: Rongwei Liu cc: Matan Azrad , Slava Ovsiienko , Ori Kam , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Aman Singh , Yuying Zhang , Ferruh Yigit , Andrew Rybchenko , "dev@dpdk.org" , Raslan Darawsheh Subject: RE: [PATCH v7] ethdev: add special flags when creating async transfer table In-Reply-To: Message-ID: References: <20221114115946.1074787-1-rongweil@nvidia.com> <2cd321-40eb-799f-abe-d0258f92e6de@arknetworks.am> <2f5f4559-b650-e085-3364-68886cc9b3d6@arknetworks.am> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 >> Sent: Tuesday, January 31, 2023 07:00 >> To: Rongwei Liu >> Cc: Matan Azrad ; Slava Ovsiienko >> ; Ori Kam ; NBU-Contact- >> Thomas Monjalon (EXTERNAL) ; Aman Singh >> ; Yuying Zhang ; >> Ferruh Yigit ; Andrew Rybchenko >> ; dev@dpdk.org; Raslan Darawsheh >> >> 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 >>>> Sent: Monday, January 30, 2023 15:40 >>>> To: Rongwei Liu >>>> Cc: Matan Azrad ; Slava Ovsiienko >>>> ; Ori Kam ; NBU-Contact- >>>> Thomas Monjalon (EXTERNAL) ; Aman Singh >>>> ; Yuying Zhang ; >>>> Ferruh Yigit ; Andrew Rybchenko >>>> ; dev@dpdk.org; Raslan Darawsheh >>>> >>>> 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 >>>>>> Sent: Monday, January 30, 2023 08:00 >>>>>> To: Rongwei Liu >>>>>> Cc: Matan Azrad ; Slava Ovsiienko >>>>>> ; Ori Kam ; NBU-Contact- >>>>>> Thomas Monjalon (EXTERNAL) ; Aman Singh >>>>>> ; Yuying Zhang ; >>>>>> Ferruh Yigit ; Andrew Rybchenko >>>>>> ; dev@dpdk.org; Raslan Darawsheh >>>>>> >>>>>> 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 >>>>>>> Acked-by: Ori Kam >>>>>>> >>>>>>> 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. >