From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from stargate3.asicdesigners.com (stargate.chelsio.com [12.32.117.8]) by dpdk.org (Postfix) with ESMTP id 788742931 for ; Thu, 25 Feb 2016 10:33:49 +0100 (CET) Received: from localhost (scalar.blr.asicdesigners.com [10.193.185.94]) by stargate3.asicdesigners.com (8.13.8/8.13.8) with ESMTP id u1P9Xg4v008320; Thu, 25 Feb 2016 01:33:43 -0800 Date: Thu, 25 Feb 2016 15:03:24 +0530 From: Rahul Lakkireddy To: Thomas Monjalon Message-ID: <20160225093322.GB10077@chelsio.com> References: <13893420.AuWBYNZ43L@xps13> <20160224184006.GB8856@chelsio.com> <3102616.xnBgmsEaQs@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3102616.xnBgmsEaQs@xps13> User-Agent: Mutt/1.5.24 (2015-08-30) Cc: "dev@dpdk.org" , Kumar A S , Nirranjan Kirubaharan Subject: Re: [dpdk-dev] [PATCH 01/10] ethdev: add a generic flow and new behavior switch to fdir X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Feb 2016 09:33:50 -0000 Hi Thomas, On Wednesday, February 02/24/16, 2016 at 14:17:58 -0800, Thomas Monjalon wrote: > Caution: I truly respect the work done by Chelsio on DPDK. Thank you. And Chelsio will continue to support DPDK community and will continue to contribute. > And I'm sure you can help to build a good filtering API, which > was mainly designed with Intel needs in mind because it was > difficult to have opinions of other vendors some time ago. > That's why it's a chance to have new needs and it would be a shame > to let it go through a vendor specific backdoor. I agree that new needs should be raised. RFC v1 was submitted at: http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/29986 RFC v2 was submitted at: http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/30732 I tried to accomodate as many fields as possible to make this as generic as possible. Also, I followed the review comments given by Jingjing. I also waited for more review comments before posting this series to see if there were any objections with the approach. I have been trying to make this generic for all vendors and not favour any one over the other. > > 2016-02-25 00:10, Rahul Lakkireddy: > > Hi Thomas, > > > > On Wednesday, February 02/24/16, 2016 at 07:02:42 -0800, Thomas Monjalon wrote: > > > 2016-02-24 14:43, Bruce Richardson: > > > > On Wed, Feb 03, 2016 at 02:02:22PM +0530, Rahul Lakkireddy wrote: > > > > > Add a new raw packet flow that allows specifying generic flow input. > > > > > > > > > > Add the ability to provide masks for fields in flow to allow range of > > > > > values. > > > > > > > > > > Add a new behavior switch. > > > > > > > > > > Add the ability to provide behavior arguments to allow rewriting matched > > > > > fields with new values. Ex: allows to provide new ip and port addresses > > > > > to rewrite the fields of packets matching a filter rule before NAT'ing. > > > > > > > > > Thomas, any comments as ethdev maintainer? > > > > > > Yes, some comments. > > > First, there are several different changes in the same patch. It must be split. > > > > Should each structure change be split into a separate patch? > > A patch = a feature. > The switch action and the flow rule are different things. Ok. I will split this into separate patches. > > > > Then I don't understand at all the raw flow filter. What is a raw flow? > > > How behavior_arg must be used? > > > > This was discussed with Jingjing at > > > > http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/31471 > > Thanks, I missed it. > > > A raw flow provides a generic way for vendors to add their vendor > > specific input flow. > > Please, "generic" and "vendor specific" in the same sentence. > It's obviously wrong. I think this sentence is being mis-interpreted. What I intended to say is: the fields are generic so that any vendor can hook-in. The fields themselves are not vendor specific. > > > In our case, it is possible to match several flows > > in a single rule. For example, it's possible to set an ethernet, vlan, > > ip and tcp/udp flows all in a single rule. We can specify all of these > > flows in a single raw input flow, which can then be passed to cxgbe flow > > director to set the corresponding filter. > > I feel we need to define what is an API. > If the application wants to call something specific to the NIC, why using > the ethdev API? You just have to include cxgbe.h. Well, in that sense, flow-director is also very intel specific, no ? What we are trying to do is make flow-director generic and, we have been following the review comments on this. If there are better ideas on how to achieve this, we are open to suggestions/comments and are ready to re-do the series and re-submit also. > > > On similar lines, behavior_arg provides a generic way to pass extra > > action arguments for matched flows. For example, in our case, to > > perform NAT, the new src/dst ip and src/dst port addresses to be > > re-written for a matched rule can be passed in behavior_arg. > > Yes a kind of void* to give what you want to the driver without the > convenience of a documented function. void* approach was taken based on review comments from Jingjing. And we didn't receive any further comments/objections on that thread. > > I know the support of filters among NICs is really heterogeneous. > And the DPDK API are not yet generic enough. But please do not give up! > If the filtering API can be improved to support your cases, please do it. I am not giving up. If there are better suggestions then, I am willing to re-do and re-submit the series. If the approach taken in RFC v1 series looks more promising then, I can re-surrect that also. However, I will need some direction over here so that it becomes generic and doesn't remain intel specific as it is now. Thanks, Rahul.