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 0F3F1A0543; Thu, 22 Sep 2022 12:31:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 283E440C35; Thu, 22 Sep 2022 12:31:17 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 9E811400D7 for ; Thu, 22 Sep 2022 12:31:16 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 1F56683; Thu, 22 Sep 2022 13:31:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 1F56683 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1663842676; bh=fph3gLB7gPuOVBc8wlU3JROplQNO1ukrBRFySOyqyn0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=E5DBif6m58mE4XOdRTtGgnKtYM6kznPbBrc9iql4wCRbg9lRBBlRSF5yII6Ou0OFm r59AyDo2zy2YDEeV+KCuEFMRBZREOt5r3+rsIp+4uestg6ReL880Pz/X05BnVkd5cl +b29+wIeXfXydcouTieHA77sItJG5qq7rYaHPn3I= Message-ID: <36d3dc2d-e54f-8b2d-da5a-3d7ea66f34be@oktetlabs.ru> Date: Thu, 22 Sep 2022 13:31:15 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Subject: Re: [PATCH v1] ethdev: add direction info when creating the transfer table Content-Language: en-US To: Ori Kam , =?UTF-8?Q?Morten_Br=c3=b8rup?= , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Ivan Malov Cc: Rongwei Liu , Matan Azrad , Slava Ovsiienko , Aman Singh , Yuying Zhang , "dev@dpdk.org" , Raslan Darawsheh , "jerinj@marvell.com" , "ajit.khaparde@broadcom.com" References: <20220907024020.2474860-1-rongweil@nvidia.com> <4733483b-effe-1eac-cbf-d238e1ec2b8@oktetlabs.ru> <13527775.RDIVbhacDa@thomas> <98CBD80474FA8B44BF855DF32C47DC35D87342@smartserver.smartshare.dk> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 On 9/22/22 13:06, Ori Kam wrote: > Hi Andrew, > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Thursday, 22 September 2022 10:39 >> >> On 9/21/22 15:51, Morten Brørup wrote: >>>> From: Ori Kam [mailto:orika@nvidia.com] >>>> Sent: Wednesday, 21 September 2022 14.41 >>>> >>>>> From: Andrew Rybchenko >>>>> >>>>> On 9/21/22 12:40, Thomas Monjalon wrote: >>>>>> 21/09/2022 11:04, Ivan Malov: >>>>>>> Now it's clear to me that your intention is to match on exact >>>> ports, >>>>>>> as usual, but this time with a hint for the flow table. Got it. >>>>>>> >>>>>>> In your response, you say that matching on ALL vports is not what >>>>>>> the use case needs. OK, I understood. But please note that the >>>>>>> item name does not say "ALL", it says "ANY". >>>>>>> >>>>>>> OK. Say, "ANY" is also confusing. Let's then name it "VPORTS_ONLY" >>>>>>> and "PHY_PORTS_ONLY". This way, if user provides item >> VPORTS_ONLY >>>>>>> and then provides item REPRESENTED_PORT, these two items do not >>>>>>> contradict each other. Item VPORTS_ONLY defines the scope of some >>>>>>> kind, then the following item, REPRESENTED_PORT, makes it >>>> narrower. >>>>>>> >>>>>>> And, in documentation, one can say clearly that the user *may* >>>>>>> omit item VPORTS_ONLY in the exact rule pattern provided that >>>>>>> they have already submitted this item as part of the template. >>>>>> >>>>>> I think the problem that Rongwei & Ori are trying to solve >>>>>> is to allocate resources for the templates table in the right >>>> place. >>>>>> A table can have multiple templates. >>>>>> If all rules/templates for this table are dedicated to virtual >>>> ports, >>>>>> then the table will be allocated in a place managing only virtual >>>> ports. >>>>>> This allocation decision must be taken at table creation, >>>>>> whereas rules will be created later. >>>>>> In order to do this specific table allocation for vports, >>>>>> we need to restrict all templates of the table to be "vports only". >>>>>> >>>>>> I hope it makes things clearer. >>>>>> Now the question is how to achieve this? Solutions are: >>>>>> >>>>>> 1/ give a hint to the table allocation >>>>>> 2/ insert a pattern item in all templates of the table >>>>>> >>>>>> I don't see any other solution. Please propose if there are more >>>> options. >>>>>> >>>>>> >>>>> >>>>> See my mail >>>>> >>>>> 3/ use jump rule which ensures that all traffic meets out >>>>> expectations >>>>> >>>>> It means that the table creation could be postponed. Or the >>>>> table could be per-configured at the point of creation and >>>>> finalized when we know that all traffic will be from wires >>>>> or from vports. Yes, it complicates internals to achieve >>>>> the optimization. >>>> >>>> Sorry Andrew your suggestion is not a valid one for the following >>>> reasons: >>>> 1. table creation can't be postponed this is a key idea of the rte_flow >>>> template API. >> >> I guess nobody cares if it delays insertion on the first rule >> only. Anyway, see below. >> >>>> 2. we can never know what rules will be inserted if the application >>>> doesn't tell us. >>>> how can we know this is the last rule? What do we do with the >>>> first rule? >>>> 3. I don't see how jumping helps since it worsens the issue when you >>>> jump to a table, >>>> how does the PMD know if this table should have only wire or only >>>> vports? >> >> Jump rules say so. PMD can analyze there rules. >> May be just need an attribute saying that all jump rules >> to the table are configured and further attempts to reconfigure >> will be rejected? >> > > The idea is the PMD will not analyze rules. That is why we have the table > and template. > Sorry, I don't understand what attribute can be in jump? The jump is just > to table. It can't say anything about the table destination table. > This is all this patch adds the attribute to a table to say where this > table should be located. > >>>> >>>> I agree with Thomas, there are two valid options, I vote for the hint >>>> since this is the >>>> feature idea to tell the PMD where this resource should be allocated. >>> >>> This is an optimization; I agree with Ori that a hint is appropriate, like the >> MBUF_FAST_FREE hint on TX queues. >>> >>> No need to add more complexity by requiring the driver to recognize that >> the pattern is present in all templates. (And perhaps also remove that >> pattern when applying the templates.) >> >> What does the part of the matching criteria so special >> that it is allowed to have dedicated hint attribute? >> >> May be we can have really generic solution when any >> part of the matching criteria could provide such hints? > > That is the point I keep returning to, it is not matching! > This is on which HW resource the table should be allocated. Sorry, but it is just your HW details that you have different location/resources for rules which apply on packets coming from wire and coming from host (vports). > Think about ingress/egress/transfer why are they not in the pattern? We have no ingress/egress in transfer domain any more because it is ambiguous. Transfer itself is really a different domain. Logically and from privileges point of view. That's why it is important to distinguish it. Ingress and egress in non-transfer case are natively bound to two main functions of the driver: transmit (egress rules) and receive (ingress rules). In general, it is a matching criteria as well, but because of its nature (explained above) it is simply handy to distinguish it from the very beginning. > They are where rules should be offloaded, they are different domain. We have just two domains: transfer and non-transfer. > Like we have elsewhere for example in action create we can state on which > domain the action should be created. If the application selects a number of domains > it may mean that extra resources will be allocated.> Two more points: 1/ If it is just a hint, it is optional for PMD to support/handle it. It means that it MUST NOT impose any limitations on matching. If so, if you want a rule to be applied on packets coming from wire, you still MUST specify it in the pattern. So, it does not sound like a hint in your case. 2/ struct rte_flow_attr is used for really all rules. How a new attribute should be interpreted in non-transfer rules? Similar to ingress/egress? Duplication? Or even harder (if it is NOT a hint): should it really enforce matching of packets coming from wire (i.e. not a different vport)? Not sure that it is doable or even make sense. We can say that the attribute may be used for the transfer rules only. If so, it MUST be checked on ethdev level since it is a generic rule. 3/ struct rte_flow_attr is used for sync and async rules. As I understand you're using it for async rules only. Does it make sense for sync rules?