From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Fastabend Subject: Re: [RFC] Generic flow director/filtering/classification API Date: Sat, 23 Jul 2016 14:10:22 -0700 Message-ID: <5793DD3E.3080605@gmail.com> References: <20160705181646.GO7621@6wind.com> <20160711104141.GA10172@localhost.localdomain> <20160721192023.GU7621@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Jerin Jacob , dev@dpdk.org, Thomas Monjalon , Helin Zhang , Jingjing Wu , Rasesh Mody , Ajit Khaparde , Rahul Lakkireddy , Wenzhuo Lu , Jan Medala , John Daley , Jing Chen , Konstantin Ananyev , Matej Vido , Alejandro Lucero , Sony Chacko , Pablo de Lara , Olga Shern Return-path: Received: from mail-pa0-f68.google.com (mail-pa0-f68.google.com [209.85.220.68]) by dpdk.org (Postfix) with ESMTP id 825DC58C5 for ; Sat, 23 Jul 2016 23:10:47 +0200 (CEST) Received: by mail-pa0-f68.google.com with SMTP id hh10so8935102pac.1 for ; Sat, 23 Jul 2016 14:10:47 -0700 (PDT) In-Reply-To: <20160721192023.GU7621@6wind.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 16-07-21 12:20 PM, Adrien Mazarguil wrote: > Hi Jerin, > > Sorry, looks like I missed your reply. Please see below. > Hi Adrian, Sorry for a bit delay but a few comments that may be worth considering. To start with completely agree on the general problem statement and the nice summary of all the current models. Also good start on this. > > Considering that allowed pattern/actions combinations cannot be known in > advance and would result in an unpractically large number of capabilities to > expose, a method is provided to validate a given rule from the current > device configuration state without actually adding it (akin to a "dry run" > mode). Rather than have a query/validate process why did we jump over having an intermediate representation of the capabilities? Here you state it is unpractical but we know how to represent parse graphs and the drivers could report their supported parse graph via a single query to a middle layer. This will actually reduce the msg chatter imagine many applications at init time or in boundary cases where a large set of applications come online at once and start banging on the interface all at once seems less than ideal. Worse in my opinion it requires all drivers to write mostly duplicating validation code where a common layer could easily do this if every driver reported a common data structure representing its parse graph instead. The nice fallout of this initial effort upfront is the driver no longer needs to do error handling/checking/etc and can assume all rules are correct and valid. It makes driver code much simpler to support. And IMO at least by doing this we get some other nice benefits described below. Another related question is about performance. > Creation > ~~~~~~~~ > > Creating a flow rule is similar to validating one, except the rule is > actually created. > > :: > > struct rte_flow * > rte_flow_create(uint8_t port_id, > const struct rte_flow_pattern *pattern, > const struct rte_flow_actions *actions); I gather this implies that each driver must parse the pattern/action block and map this onto the hardware. How many rules per second can this support? I've run into systems that expect a level of service somewhere around 50k cmds per second. So bulking will help at the message level but it seems like a lot of overhead to unpack the pattern/action section. One strategy I've used in other systems that worked relatively well is if the query for the parse graph above returns a key for each node in the graph then a single lookup can map the key to a node. Its unambiguous and then these operations simply become a table lookup. So to be a bit more concrete this changes the pattern structure in rte_flow_create() into a tuple where the key is known by the initial parse graph query. If you reserve a set of well-defined key values for well known protocols like ethernet, ip, etc. then the query model also works but the middle layer catches errors in this case and again the driver only gets known good flows. So something like this, struct rte_flow_pattern { uint32_t priority; uint32_t key; uint32_t value_length; u8 *value; } Also if we have multiple tables what do you think about adding a table_id to the signature. Probably not needed in the first generation but is likely useful for hardware with multiple tables so that it would be, rte_flow_create(uint8_t port_id, uint8_t table_id, ...); Finally one other problem we've had which would be great to address if we are doing a rewrite of the API is adding new protocols to already deployed DPDK stacks. This is mostly a Linux distribution problem where you can't easily update DPDK. In the prototype header linked in this document it seems to add new headers requires adding a new enum in the rte_flow_item_type but there is at least an attempt at a catch all here, > /** > * Matches a string of a given length at a given offset (in bytes), > * or anywhere in the payload of the current protocol layer > * (including L2 header if used as the first item in the stack). > * > * See struct rte_flow_item_raw. > */ > RTE_FLOW_ITEM_TYPE_RAW, Actually this is a nice implementation because it works after the previous item in the stack correct? So you can put it after "known" variable length headers like IP. The limitation is it can't get past undefined variable length headers. However if you use the above parse graph reporting from the driver mechanism and the driver always reports its largest supported graph then we don't have this issue where a new hardware sku/ucode/etc added support for new headers but we have no way to deploy it to existing software users without recompiling and redeploying. I looked at the git repo but I only saw the header definition I guess the implementation is TBD after there is enough agreement on the interface? Thanks, John