From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw Date: Thu, 28 Jun 2018 15:22:41 +0200 Message-ID: <20180628132241.GA2177@nanopsycho.orion> References: <20180626080000.12964-1-jiri@resnulli.us> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, 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: Jamal Hadi Salim Return-path: Received: from mail-wr0-f194.google.com ([209.85.128.194]:39886 "EHLO mail-wr0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752056AbeF1NYM (ORCPT ); Thu, 28 Jun 2018 09:24:12 -0400 Received: by mail-wr0-f194.google.com with SMTP id b8-v6so5515476wro.6 for ; Thu, 28 Jun 2018 06:24:11 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Thu, Jun 28, 2018 at 03:13:30PM CEST, jhs@mojatatu.com wrote: > >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. I'm afraid I don't understand clearly what you say. This is not restricted to hw offload. The templates apply to all filters, no matter if they are offloaded or not. > >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