From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f45.google.com (mail-wm0-f45.google.com [74.125.82.45]) by dpdk.org (Postfix) with ESMTP id 9215D2BE1 for ; Fri, 24 Mar 2017 10:46:44 +0100 (CET) Received: by mail-wm0-f45.google.com with SMTP id u132so8484131wmg.0 for ; Fri, 24 Mar 2017 02:46:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=lwjQZh2FVLreMRDaBdiiJaj4l6mMVAlaVl8EmeBf6WU=; b=rEKjZHYsKKJnax3ZySf12bvaaDYuGeqK1vcDdj3AUBbKhL4Db808pX6xgfk3CIhD9Q 09D8TZEXU4YIwNQR99byYJ40KIT7jIf7bSeeln30YWNGW1uPXlWvbEV+nY5C6NsPMCrs KWZch6I6WUb3QPlBioUu45iOS938ylEpA73Xmk/0Kw2BCZIyrfTKor1smfS8vrUuCk1+ 5QuJetie0yioZj/HzajMsutbSo/UtfNs4ta1ycIMz0vZdn1Qx5lVbIz9pGTSG8sVmza3 fagIy+1bc8Yhmgt7jaSxIXDJQvjYyHai5BY5N07J0UYDh7/2JdyjVsQfwxvL1yLivJbI XfyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=lwjQZh2FVLreMRDaBdiiJaj4l6mMVAlaVl8EmeBf6WU=; b=h89AhrGh5+AF5dGgLVoRXh/qk6Fx4qTvPNsIO/OTIv/5GndsVtSRciJoV5Q0IyAo74 0eS4ZSSVf9LQe4SPUYz7xQ8rpTt1Nw2hn8zvx6SuoyMMOB/Vyde4LUHfj2pSqc3tnrTF maBdjSBy2nb1D9XxMcvpm5ZH2Tzsm4NHtlfMybjlaZs3ULYXyksxyK8pCXUmhL+KyNqo +wZmbcAL6DeYCah0wXrEgINgp3rAk3GAdDdd3yEJ4PV64lNFgGPBUmdryg3RjfOCqcyg vD34KZ9VsZUaJVil1YdeyfHO0+9ogfDPyvf5kcEcMlub0iE05KO4ca95l8n8bwie1c3f QArA== X-Gm-Message-State: AFeK/H13ol8NWUlV9EDzHqj0XVYqHy/bIJSUgTIr7pKaarYiE8ScUsE/kC7UeegemMBZJYb9 X-Received: by 10.28.16.143 with SMTP id 137mr1643206wmq.114.1490348803851; Fri, 24 Mar 2017 02:46:43 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id o15sm1753570wmd.10.2017.03.24.02.46.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 Mar 2017 02:46:42 -0700 (PDT) Date: Fri, 24 Mar 2017 10:46:33 +0100 From: Adrien Mazarguil To: John Daley Cc: dev@dpdk.org Message-ID: <20170324094633.GS3790@6wind.com> References: <20170324023659.28099-1-johndale@cisco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170324023659.28099-1-johndale@cisco.com> Subject: Re: [dpdk-dev] [PATCH 0/1] proposed minor change in rte_flow_validate semantics X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Mar 2017 09:46:44 -0000 Hi John, On Thu, Mar 23, 2017 at 07:36:58PM -0700, John Daley wrote: > Hi, > > In implementing rte_flow_validate() for the Cisco enic, I got to wondering > if the semantics might be slightly off given how I see apps using it. > > Please forgive me if this has already been discussed, but during runtime, I > can't see any reason why rte_flow_validate() would be called when > rte_flow_create() returns the same errors as validate, but also creates > the filter if all is good (calling validate first is just extra overhead). > What are the use cases for calling validate when the app is up and running? In short (I'll elaborate below), to get a rough idea of the capabilities of a given port at initialization time. One should use rte_flow_validate() with nonspecific rules to determine if the PMD understands them at all *given the circumstances* (the current configuration state of the device). Nothing prevents one from using it just before calling rte_flow_create(), however doing so is indeed redundant. > I see rte_flow_validate() being run at startup for answering these 2 questions: > 1.) Given enough resources, is the device capable of handling flows > a, b, c, .., which the app may want to create during runtime? Yes, breaking this down means all the following: 1. Recognize all pattern items and actions. 2. Make sure items are properly stacked and actions can be combined. 3. Check for pattern item specification structures (e.g. limits, masks, ranges, and so on). 4. Check for actions configuration structures (e.g. target queue exists). Depending on the flow rule, 3. and 4. may have to check the current device configuration in order to validate the rule. > 2.) How many concurrent flows of type a, b, c, .. can the device handle? To > answer this app needs to do a bunch of rte_flow_create()'s (followed by a bunch of rte_flow_destroy()s). Hehe, this is the brute force approach, actually the rte_flow API in its current form is not supposed to answer this question, and I don't think we should document it that way (note you could use rte_flow_flush() to destroy all flows at once faster). Some devices (like mlx5) do not really have an upper limit on the number of possible flows one can configure, they're basically limited by the available memory on the host system. > The answer to these questions will allow the app to decide beforehand which > filters to offload (if any) and set up function pointers or whatever to run > as efficiently as possible on the target device. > > rte_flow_validate() has comments that imply it should be run on the fly,like > "the flow rule is validated against its current configuration state and the > returned value should be considered valid by the caller for that state only". > The only real way to implement this is for enic (and I think other nics) is > to actually do a flow create/destroy within rte_flow_validate(). But, see > paragraph 2 above- why bother? I looked at the i40e and mlx5 implementations > and I didn't see any calls to the nics to check a flow against current state > so they might not be implemented as the comment imply they should be either. Then you're probably right there is an issue with the documentation, it's not the intent (more below). > If there are no use cases for calling validate at runtime, I think we ought > to change the semantics of rte_flow_validate() a little to define success to > mean that the device will accept the flow IF there are enough resources, > rather than it will accept the flow given the current state. We can safely > remove the -EEXIST return code since no other drivers set them yet. This > makes the function easier to implement correctly without trying to do > something that's not useful anyway. > > The following patch just changes comments but I think that is all that is > required. rte_flow_validate() and rte_flow_create() share the same error codes to make their implementation simpler. rte_flow_validate() basically exposes the validation part necessarily present in rte_flow_create(). EEXIST and other error codes *may* be returned by rte_flow_validate(), but this does not force PMDs to check possible collisions during the validation step, I'll take documentation is not clear enough regarding this. So to be completely clear, rte_flow_validate() already does *not* guarantee a rule can be created afterward, only that rte_flow_create() will understand that rule. The other reason "configuration state" is mentioned in the documentation (besides target queues should exist) is that existing rules may have switched the device in a mode that does not allow different rule types. If after initialization, matching TCP, UDP and ICMP is possible, creating a UDP rule might subsequently prevent the creation of otherwise valid TCP and ICMP rules. rte_flow_validate() should (but is not forced to) check for that. What do you think about keeping the defined error codes as is and merging somehow my above statements in the documentation instead? -- Adrien Mazarguil 6WIND