On 10/17/2018 10:41 PM, Cong Wang wrote: > On Wed, Oct 17, 2018 at 9:42 PM David Miller <da...@davemloft.net> wrote: >> >> From: Amritha Nambiar <amritha.namb...@intel.com> >> 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
