From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Nambiar, Amritha" Subject: Re: [net-next PATCH] net: sched: cls_flower: Classify packets using port ranges Date: Thu, 18 Oct 2018 00:22:15 -0700 Message-ID: <9100e57f-0520-0ff8-b091-e81d5aeb9a27@intel.com> References: <153935241037.11051.5334451030083154425.stgit@anamhost.jf.intel.com> <20181017.214233.695288699109520404.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Linux Kernel Network Developers , Jakub Kicinski , sridhar.samudrala@intel.com, Jamal Hadi Salim , Jiri Pirko To: Cong Wang , David Miller Return-path: Received: from mga03.intel.com ([134.134.136.65]:62776 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727538AbeJRPVy (ORCPT ); Thu, 18 Oct 2018 11:21:54 -0400 In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 10/17/2018 10:41 PM, Cong Wang wrote: > On Wed, Oct 17, 2018 at 9:42 PM David Miller wrote: >> >> From: Amritha Nambiar >> Date: Fri, 12 Oct 2018 06:53:30 -0700 >> >>> Added support in tc flower for filtering based on port ranges. >>> This is a rework of the RFC patch at: >>> https://patchwork.ozlabs.org/patch/969595/ >> >> You never addressed Cong's feedback asking you to explain why this >> can't be simply built using existing generic filtering facilities that >> exist already. >> >> I appreciate that you addressed Jiri's feedback, but Cong's feedback is >> just as, if not more, important. >> > > My objection is against introducing a new filter just for port range, now > it is built on top of flower filter, so it is much better now. > > u32 filter can do the nearly same, but requires a power-of-two, so it is > not completely duplicated. > > Therefore, I think the idea of building it on top of flower is fine. But I don't > read into any code, only the description. > > Thanks! > Sorry for not clarifying it out, this reworked patch addresses both Jiri's and Cong's concerns. The previous RFC patch introduced a new special-purpose classifier called 'range' for port-range based filtering, that as Cong pointed out had overlaps with other existing classifiers. The reason I added a new classifier was because u32 does not support ranges that are not power-of-2 and flower uses mask-key based rhashtable lookup which was not suited for range based keys. Based on the feedback for the RFC, this patch adds port-range support to cls_flower by separating out range comparison from the rhashtable lookup. Since this adds to cls_flower, overlaps with other general-purpose classifiers are avoided. -Amritha