From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamal Hadi Salim Subject: Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw Date: Thu, 28 Jun 2018 09:13:30 -0400 Message-ID: References: <20180626080000.12964-1-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, xiyou.wangcong@gmail.com, jakub.kicinski@netronome.com, simon.horman@netronome.com, john.hurley@netronome.com, dsahern@gmail.com, mlxsw@mellanox.com To: Jiri Pirko , netdev@vger.kernel.org Return-path: Received: from mail-io0-f196.google.com ([209.85.223.196]:36163 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752170AbeF1NNd (ORCPT ); Thu, 28 Jun 2018 09:13:33 -0400 Received: by mail-io0-f196.google.com with SMTP id k3-v6so5189352iog.3 for ; Thu, 28 Jun 2018 06:13:33 -0700 (PDT) In-Reply-To: <20180626080000.12964-1-jiri@resnulli.us> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 26/06/18 03:59 AM, Jiri Pirko wrote: > From: Jiri Pirko > > For the TC clsact offload these days, some of HW drivers need > to hold a magic ball. The reason is, with the first inserted rule inside > HW they need to guess what fields will be used for the matching. If > later on this guess proves to be wrong and user adds a filter with a > different field to match, there's a problem. Mlxsw resolves it now with > couple of patterns. Those try to cover as many match fields as possible. > This aproach is far from optimal, both performance-wise and scale-wise. > Also, there is a combination of filters that in certain order won't > succeed. > > > Most of the time, when user inserts filters in chain, he knows right away > how the filters are going to look like - what type and option will they > have. For example, he knows that he will only insert filters of type > flower matching destination IP address. He can specify a template that > would cover all the filters in the chain. > Is this just restricted to hardware offload? Example it will make sense for u32 in s/ware as well (i.e flexible TCAM like TCAM based classification). i.e it is possible that rules the user enters end up being worst case a linked list lookup, yes? And allocating space for a tuple that is not in use is a waste of space. If yes, then I would reword the above as something like: For very flexible classifiers such as TCAM based ones, one could add arbitrary tuple rules which tend to be inefficient both from a space and lookup performance. One approach, taken by Mlxsw, is to assume a multi filter tuple arrangement which is inefficient from a space perspective when the user-specified rules dont make use of pre-provisioned tuple space. Typically users already know what tuples are of interest to them: for example for ipv4 route lookup purposes they may just want to lookup destination IP with a specified mask etc. This feature allows user to provide good hints to the classifier to optimize. > This patchset is providing the possibility to user to provide such > template to kernel and propagate it all the way down to device > drivers. > > See the examples below. > > Create dummy device with clsact first: > # ip link add type dummy > # tc qdisc add dev dummy0 clsact > > There is no template assigned by default: > # tc filter template show dev dummy0 ingress > > Add a template of type flower allowing to insert rules matching on last > 2 bytes of destination mac address: > # tc filter template add dev dummy0 ingress proto ip flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF > > The template is now showed in the list: > # tc filter template show dev dummy0 ingress > filter flower chain 0 > dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff > eth_type ipv4 > > Add another template, this time for chain number 22: > # tc filter template add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16 > # tc filter template show dev dummy0 ingress > filter flower chain 0 > dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff > eth_type ipv4 > filter flower chain 22 > eth_type ipv4 > dst_ip 0.0.0.0/16 > > Add a filter that fits the template: > # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:bb:cc:dd:ee:ff/00:00:00:00:00:0F action drop > > Addition of filters that does not fit the template would fail: > # tc filter add dev dummy0 ingress proto ip flower dst_mac aa:11:22:33:44:55/00:00:00:FF:00:00 action drop > Error: Mask does not fit the template. > We have an error talking to the kernel, -1 > # tc filter add dev dummy0 ingress proto ip flower dst_ip 10.0.0.1 action drop > Error: Mask does not fit the template. > We have an error talking to the kernel, -1 > > Additions of filters to chain 22: > # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/8 action drop > # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1 action drop > Error: Mask does not fit the template. > We have an error talking to the kernel, -1 > # tc filter add dev dummy0 ingress proto ip chain 22 flower dst_ip 10.0.0.1/24 action drop > Error: Mask does not fit the template. > We have an error talking to the kernel, -1 > > Removal of a template from non-empty chain would fail: > # tc filter template del dev dummy0 ingress > Error: The chain is not empty, unable to delete template. > We have an error talking to the kernel, -1 > > Once the chain is flushed, the template could be removed: > # tc filter del dev dummy0 ingress > # tc filter template del dev dummy0 ingress > BTW: unlike the other comments on this - I think the syntax above is fine ;-> Chain are already either explicitly or are implicitly (case of chain 0) specified. Assuming that one cant add a new template to a chain if it already has at least one filter (even if no template has been added). I like it - it may help making u32 more friendly to humans in some cases. cheers, jamal