Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On Sat, Jun 30, 2018 at 3:13 AM Jiri Pirko wrote: > Okay. So that would allow either create a chain or "chain with > template". Once that is done, there would be no means to manipulate the > template. One can only remove the chain. > > What about refounting? I think it would make sense that this implicit > chain addition would take one reference. That means if later on the last > filter is removed, the chain would stay there until user removes it by > hand. Yeah, it is very similar to tc actions. So you can take a look at how tc actions are refcnt'ed.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Sat, Jun 30, 2018 at 12:18:02AM CEST, xiyou.wangc...@gmail.com wrote: >On Fri, Jun 29, 2018 at 10:06 AM Samudrala, Sridhar > wrote: >> >> So instead of introducing 'chaintemplate' object in the kernel, can't we add >> 'chain' >> object in the kernel that takes the 'template' as an attribute? > >This is exactly what I mean above. Making the chain a standalone object >in kernel would benefit: > >1. Align with 'tc chain' in iproute2, add/del an object is natural > >2. Template is an attribute of this object when creating it: ># tc chain add template ># tc chain add ... # non-template chain Okay. So that would allow either create a chain or "chain with template". Once that is done, there would be no means to manipulate the template. One can only remove the chain. What about refounting? I think it would make sense that this implicit chain addition would take one reference. That means if later on the last filter is removed, the chain would stay there until user removes it by hand. Okay. Sounds good to me. Will do. Thanks! > >3. Easier for sharing by qdiscs: ># tc chain add X block Y ... ># tc filter add ... chain X block Y ... ># tc qdisc add dev eth0 block Y ... > >The current 'ingress_block 22 ingress' syntax is ugly.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On Fri, Jun 29, 2018 at 10:06 AM Samudrala, Sridhar wrote: > > So instead of introducing 'chaintemplate' object in the kernel, can't we add > 'chain' > object in the kernel that takes the 'template' as an attribute? This is exactly what I mean above. Making the chain a standalone object in kernel would benefit: 1. Align with 'tc chain' in iproute2, add/del an object is natural 2. Template is an attribute of this object when creating it: # tc chain add template # tc chain add ... # non-template chain 3. Easier for sharing by qdiscs: # tc chain add X block Y ... # tc filter add ... chain X block Y ... # tc qdisc add dev eth0 block Y ... The current 'ingress_block 22 ingress' syntax is ugly.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 6/29/2018 6:05 AM, Jiri Pirko wrote: Fri, Jun 29, 2018 at 02:54:36PM CEST, dsah...@gmail.com wrote: On 6/29/18 6:48 AM, Jiri Pirko wrote: Fri, Jun 29, 2018 at 02:12:21PM CEST, j...@mojatatu.com wrote: On 29/06/18 04:39 AM, Jiri Pirko wrote: Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangc...@gmail.com wrote: On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: Add a template of type flower allowing to insert rules matching on last 2 bytes of destination mac address: # tc chaintemplate 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 chaintemplate show dev dummy0 ingress chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16 # tc chaintemplate show dev dummy0 ingress chaintemplate flower chain 0 dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff eth_type ipv4 chaintemplate flower chain 22 eth_type ipv4 dst_ip 0.0.0.0/16 So, if I want to check the template of a chain, I have to use 'tc chaintemplate... chain X'. If I want to check the filters in a chain, I have to use 'tc filter show chain X'. If you introduce 'tc chain', it would just need one command: `tc chain show ... X` which could list its template first and followed by filters in this chain, something like: # tc chain show dev eth0 chain X template: # could be none filter1 ... filter2 ... Isn't it more elegant? Well, that is just another iproute2 command. It would use the same kernel uapi. Filters+templates. Sure, why not. Can be easily introduced. Let's do it in a follow-up iproute2 patch. Half a dozen or 6 - take your pick, really. I would call the template an attribute as opposed to a stand alone object i.e A chain of filters may have a template. If you have to introduce a new object then Sridhar's suggested syntax seems appealing. I think what I have makes sense. Maps nicely to what it really is inside kernel. What Cong proposes looks like nice extension of userspace app to do more things in one go. Will address that in followup iproute2 patch. The resolution of the syntax affect the uapi changes proposed. You are wanting to add new RTM commands which suggests new objects. If a template is an attribute of an existing object then the netlink API should indicate that. There is no existing "chain" object. So no, no uapi changes. So instead of introducing 'chaintemplate' object in the kernel, can't we add 'chain' object in the kernel that takes the 'template' as an attribute?
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: David Ahern Date: Fri, 29 Jun 2018 06:54:36 -0600 > The resolution of the syntax affect the uapi changes proposed. You are > wanting to add new RTM commands which suggests new objects. If a > template is an attribute of an existing object then the netlink API > should indicate that. +1
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Fri, Jun 29, 2018 at 02:54:36PM CEST, dsah...@gmail.com wrote: >On 6/29/18 6:48 AM, Jiri Pirko wrote: >> Fri, Jun 29, 2018 at 02:12:21PM CEST, j...@mojatatu.com wrote: >>> On 29/06/18 04:39 AM, Jiri Pirko wrote: Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangc...@gmail.com wrote: > On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: >> Add a template of type flower allowing to insert rules matching on last >> 2 bytes of destination mac address: >> # tc chaintemplate 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 chaintemplate show dev dummy0 ingress >> chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower >> dst_ip 0.0.0.0/16 >> # tc chaintemplate show dev dummy0 ingress >> chaintemplate flower chain 0 >>dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff >>eth_type ipv4 >> chaintemplate flower chain 22 >>eth_type ipv4 >>dst_ip 0.0.0.0/16 > > So, if I want to check the template of a chain, I have to use > 'tc chaintemplate... chain X'. > > If I want to check the filters in a chain, I have to use > 'tc filter show chain X'. > > If you introduce 'tc chain', it would just need one command: > `tc chain show ... X` which could list its template first and > followed by filters in this chain, something like: > > # tc chain show dev eth0 chain X > template: # could be none > > filter1 > ... > filter2 > ... > > Isn't it more elegant? Well, that is just another iproute2 command. It would use the same kernel uapi. Filters+templates. Sure, why not. Can be easily introduced. Let's do it in a follow-up iproute2 patch. >>> >>> Half a dozen or 6 - take your pick, really. >>> I would call the template an attribute as opposed to a stand alone >>> object i.e A chain of filters may have a template. If you have to >>> introduce a new object then Sridhar's suggested syntax seems appealing. >> >> I think what I have makes sense. Maps nicely to what it really is inside >> kernel. What Cong proposes looks like nice extension of userspace app to >> do more things in one go. Will address that in followup iproute2 patch. > >The resolution of the syntax affect the uapi changes proposed. You are >wanting to add new RTM commands which suggests new objects. If a >template is an attribute of an existing object then the netlink API >should indicate that. There is no existing "chain" object. So no, no uapi changes.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 6/29/18 6:48 AM, Jiri Pirko wrote: > Fri, Jun 29, 2018 at 02:12:21PM CEST, j...@mojatatu.com wrote: >> On 29/06/18 04:39 AM, Jiri Pirko wrote: >>> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangc...@gmail.com wrote: On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: > Add a template of type flower allowing to insert rules matching on last > 2 bytes of destination mac address: > # tc chaintemplate 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 chaintemplate show dev dummy0 ingress > chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip > 0.0.0.0/16 > # tc chaintemplate show dev dummy0 ingress > chaintemplate flower chain 0 >dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff >eth_type ipv4 > chaintemplate flower chain 22 >eth_type ipv4 >dst_ip 0.0.0.0/16 So, if I want to check the template of a chain, I have to use 'tc chaintemplate... chain X'. If I want to check the filters in a chain, I have to use 'tc filter show chain X'. If you introduce 'tc chain', it would just need one command: `tc chain show ... X` which could list its template first and followed by filters in this chain, something like: # tc chain show dev eth0 chain X template: # could be none filter1 ... filter2 ... Isn't it more elegant? >>> >>> Well, that is just another iproute2 command. It would use the same >>> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced. >>> Let's do it in a follow-up iproute2 patch. >>> >> >> Half a dozen or 6 - take your pick, really. >> I would call the template an attribute as opposed to a stand alone >> object i.e A chain of filters may have a template. If you have to >> introduce a new object then Sridhar's suggested syntax seems appealing. > > I think what I have makes sense. Maps nicely to what it really is inside > kernel. What Cong proposes looks like nice extension of userspace app to > do more things in one go. Will address that in followup iproute2 patch. The resolution of the syntax affect the uapi changes proposed. You are wanting to add new RTM commands which suggests new objects. If a template is an attribute of an existing object then the netlink API should indicate that.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Fri, Jun 29, 2018 at 02:12:21PM CEST, j...@mojatatu.com wrote: >On 29/06/18 04:39 AM, Jiri Pirko wrote: >> Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangc...@gmail.com wrote: >> > On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: >> > > Add a template of type flower allowing to insert rules matching on last >> > > 2 bytes of destination mac address: >> > > # tc chaintemplate 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 chaintemplate show dev dummy0 ingress >> > > chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower >> > > dst_ip 0.0.0.0/16 >> > > # tc chaintemplate show dev dummy0 ingress >> > > chaintemplate flower chain 0 >> > >dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff >> > >eth_type ipv4 >> > > chaintemplate flower chain 22 >> > >eth_type ipv4 >> > >dst_ip 0.0.0.0/16 >> > >> > So, if I want to check the template of a chain, I have to use >> > 'tc chaintemplate... chain X'. >> > >> > If I want to check the filters in a chain, I have to use >> > 'tc filter show chain X'. >> > >> > If you introduce 'tc chain', it would just need one command: >> > `tc chain show ... X` which could list its template first and >> > followed by filters in this chain, something like: >> > >> > # tc chain show dev eth0 chain X >> > template: # could be none >> > >> > filter1 >> > ... >> > filter2 >> > ... >> > >> > Isn't it more elegant? >> >> Well, that is just another iproute2 command. It would use the same >> kernel uapi. Filters+templates. Sure, why not. Can be easily introduced. >> Let's do it in a follow-up iproute2 patch. >> > >Half a dozen or 6 - take your pick, really. >I would call the template an attribute as opposed to a stand alone >object i.e A chain of filters may have a template. If you have to >introduce a new object then Sridhar's suggested syntax seems appealing. I think what I have makes sense. Maps nicely to what it really is inside kernel. What Cong proposes looks like nice extension of userspace app to do more things in one go. Will address that in followup iproute2 patch. > >cheers, >jamal
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 29/06/18 04:39 AM, Jiri Pirko wrote: Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangc...@gmail.com wrote: On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: Add a template of type flower allowing to insert rules matching on last 2 bytes of destination mac address: # tc chaintemplate 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 chaintemplate show dev dummy0 ingress chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16 # tc chaintemplate show dev dummy0 ingress chaintemplate flower chain 0 dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff eth_type ipv4 chaintemplate flower chain 22 eth_type ipv4 dst_ip 0.0.0.0/16 So, if I want to check the template of a chain, I have to use 'tc chaintemplate... chain X'. If I want to check the filters in a chain, I have to use 'tc filter show chain X'. If you introduce 'tc chain', it would just need one command: `tc chain show ... X` which could list its template first and followed by filters in this chain, something like: # tc chain show dev eth0 chain X template: # could be none filter1 ... filter2 ... Isn't it more elegant? Well, that is just another iproute2 command. It would use the same kernel uapi. Filters+templates. Sure, why not. Can be easily introduced. Let's do it in a follow-up iproute2 patch. Half a dozen or 6 - take your pick, really. I would call the template an attribute as opposed to a stand alone object i.e A chain of filters may have a template. If you have to introduce a new object then Sridhar's suggested syntax seems appealing. cheers, jamal
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Fri, Jun 29, 2018 at 12:25:53AM CEST, xiyou.wangc...@gmail.com wrote: >On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: >> Add a template of type flower allowing to insert rules matching on last >> 2 bytes of destination mac address: >> # tc chaintemplate 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 chaintemplate show dev dummy0 ingress >> chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip >> 0.0.0.0/16 >> # tc chaintemplate show dev dummy0 ingress >> chaintemplate flower chain 0 >> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff >> eth_type ipv4 >> chaintemplate flower chain 22 >> eth_type ipv4 >> dst_ip 0.0.0.0/16 > >So, if I want to check the template of a chain, I have to use >'tc chaintemplate... chain X'. > >If I want to check the filters in a chain, I have to use >'tc filter show chain X'. > >If you introduce 'tc chain', it would just need one command: >`tc chain show ... X` which could list its template first and >followed by filters in this chain, something like: > ># tc chain show dev eth0 chain X >template: # could be none > >filter1 >... >filter2 >... > >Isn't it more elegant? Well, that is just another iproute2 command. It would use the same kernel uapi. Filters+templates. Sure, why not. Can be easily introduced. Let's do it in a follow-up iproute2 patch.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On Thu, Jun 28, 2018 at 6:10 AM Jiri Pirko wrote: > Add a template of type flower allowing to insert rules matching on last > 2 bytes of destination mac address: > # tc chaintemplate 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 chaintemplate show dev dummy0 ingress > chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip > 0.0.0.0/16 > # tc chaintemplate show dev dummy0 ingress > chaintemplate flower chain 0 > dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff > eth_type ipv4 > chaintemplate flower chain 22 > eth_type ipv4 > dst_ip 0.0.0.0/16 So, if I want to check the template of a chain, I have to use 'tc chaintemplate... chain X'. If I want to check the filters in a chain, I have to use 'tc filter show chain X'. If you introduce 'tc chain', it would just need one command: `tc chain show ... X` which could list its template first and followed by filters in this chain, something like: # tc chain show dev eth0 chain X template: # could be none filter1 ... filter2 ... Isn't it more elegant?
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On Wed, Jun 27, 2018 at 9:48 PM David Miller wrote: > > This series doesn't apply cleanly to net-next, and also there seems to still > be some discussion about how the iproute2 command line should look. > I am sure you know this, so just to be clear: A redesign of "how iproute2 command line should look" usually means a redesign in the kernel code too. Apparently, 'tc chaintemplate' is a new subsystem under TC, while a 'tc filter template' is merely a new TC filter attribute.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Thu, Jun 28, 2018 at 05:50:08PM CEST, dsah...@gmail.com wrote: >On 6/28/18 9:37 AM, Jiri Pirko wrote: > > Why this restriction? It's a template, so why can't it be removed > regardless of whether there are filters? That means you could start to insert filters that does not match the original template. I wanted to avoid it. The chain is utilized in hw for the original template, the filter insertion would have to be sanitized in driver. With this restriction, drivers can depend on filters always be fitting. >>> >>> Then the hardware driver should have that restriction not the core tc code. >> >> But why? The same restriction would be in all drivers. I believe it is >> better to have in in tc in single place. Drivers can then depend on it. >> Do you have a usecase where you need to remove template for non-empty >> chain? >> > >If the hardware has the limitation then the driver should be rejecting a >change. The behaviour I defend is symmetrical with "template add". There is also possible to add the template only if the chain is empty.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 6/28/18 9:37 AM, Jiri Pirko wrote: Why this restriction? It's a template, so why can't it be removed regardless of whether there are filters? >>> >>> That means you could start to insert filters that does not match the >>> original template. I wanted to avoid it. The chain is utilized in hw for >>> the original template, the filter insertion would have to be sanitized >>> in driver. With this restriction, drivers can depend on filters always >>> be fitting. >>> >> >> Then the hardware driver should have that restriction not the core tc code. > > But why? The same restriction would be in all drivers. I believe it is > better to have in in tc in single place. Drivers can then depend on it. > Do you have a usecase where you need to remove template for non-empty > chain? > If the hardware has the limitation then the driver should be rejecting a change.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Thu, Jun 28, 2018 at 05:10:45PM CEST, dsah...@gmail.com wrote: >On 6/28/18 8:29 AM, Jiri Pirko wrote: >> Thu, Jun 28, 2018 at 04:18:47PM CEST, dsah...@gmail.com wrote: >>> On 6/28/18 7:08 AM, Jiri Pirko wrote: 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 chaintemplate show dev dummy0 ingress Add a template of type flower allowing to insert rules matching on last 2 bytes of destination mac address: # tc chaintemplate 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 chaintemplate show dev dummy0 ingress chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16 # tc chaintemplate show dev dummy0 ingress chaintemplate flower chain 0 dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff eth_type ipv4 chaintemplate 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 chaintemplate del dev dummy0 ingress Error: The chain is not empty, unable to delete template. We have an error talking to the kernel, -1 >>> >>> Why this restriction? It's a template, so why can't it be removed >>> regardless of whether there are filters? >> >> That means you could start to insert filters that does not match the >> original template. I wanted to avoid it. The chain is utilized in hw for >> the original template, the filter insertion would have to be sanitized >> in driver. With this restriction, drivers can depend on filters always >> be fitting. >> > >Then the hardware driver should have that restriction not the core tc code. But why? The same restriction would be in all drivers. I believe it is better to have in in tc in single place. Drivers can then depend on it. Do you have a usecase where you need to remove template for non-empty chain?
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 6/28/18 8:29 AM, Jiri Pirko wrote: > Thu, Jun 28, 2018 at 04:18:47PM CEST, dsah...@gmail.com wrote: >> On 6/28/18 7:08 AM, Jiri Pirko wrote: >>> 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 chaintemplate show dev dummy0 ingress >>> >>> Add a template of type flower allowing to insert rules matching on last >>> 2 bytes of destination mac address: >>> # tc chaintemplate 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 chaintemplate show dev dummy0 ingress >>> chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip >>> 0.0.0.0/16 >>> # tc chaintemplate show dev dummy0 ingress >>> chaintemplate flower chain 0 >>> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff >>> eth_type ipv4 >>> chaintemplate 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 chaintemplate del dev dummy0 ingress >>> Error: The chain is not empty, unable to delete template. >>> We have an error talking to the kernel, -1 >> >> Why this restriction? It's a template, so why can't it be removed >> regardless of whether there are filters? > > That means you could start to insert filters that does not match the > original template. I wanted to avoid it. The chain is utilized in hw for > the original template, the filter insertion would have to be sanitized > in driver. With this restriction, drivers can depend on filters always > be fitting. > Then the hardware driver should have that restriction not the core tc code.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Thu, Jun 28, 2018 at 04:18:47PM CEST, dsah...@gmail.com wrote: >On 6/28/18 7:08 AM, Jiri Pirko wrote: >> 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 chaintemplate show dev dummy0 ingress >> >> Add a template of type flower allowing to insert rules matching on last >> 2 bytes of destination mac address: >> # tc chaintemplate 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 chaintemplate show dev dummy0 ingress >> chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip >> 0.0.0.0/16 >> # tc chaintemplate show dev dummy0 ingress >> chaintemplate flower chain 0 >> dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff >> eth_type ipv4 >> chaintemplate 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 chaintemplate del dev dummy0 ingress >> Error: The chain is not empty, unable to delete template. >> We have an error talking to the kernel, -1 > >Why this restriction? It's a template, so why can't it be removed >regardless of whether there are filters? That means you could start to insert filters that does not match the original template. I wanted to avoid it. The chain is utilized in hw for the original template, the filter insertion would have to be sanitized in driver. With this restriction, drivers can depend on filters always be fitting. > >> >> Once the chain is flushed, the template could be removed: >> # tc filter del dev dummy0 ingress >> # tc chaintemplate del dev dummy0 ingress >> >
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 6/28/18 7:08 AM, Jiri Pirko wrote: > 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 chaintemplate show dev dummy0 ingress > > Add a template of type flower allowing to insert rules matching on last > 2 bytes of destination mac address: > # tc chaintemplate 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 chaintemplate show dev dummy0 ingress > chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip > 0.0.0.0/16 > # tc chaintemplate show dev dummy0 ingress > chaintemplate flower chain 0 > dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff > eth_type ipv4 > chaintemplate 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 chaintemplate del dev dummy0 ingress > Error: The chain is not empty, unable to delete template. > We have an error talking to the kernel, -1 Why this restriction? It's a template, so why can't it be removed regardless of whether there are filters? > > Once the chain is flushed, the template could be removed: > # tc filter del dev dummy0 ingress > # tc chaintemplate del dev dummy0 ingress >
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Thu, Jun 28, 2018 at 03:54:17PM CEST, j...@mojatatu.com wrote: >On 28/06/18 09:22 AM, Jiri Pirko wrote: >> Thu, Jun 28, 2018 at 03:13:30PM CEST, j...@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. > >Well - I was trying to understand what you said ;-> > >I think what you are getting at is two issues: >a) space in the tcams - if the user is just going to enter >rules which use one tuple (dst ip for example) the hardware >would be better off told that this is the case so it doesnt >allocate space in anticipation that someone is going to >specify src ip later on. Yes. >b) lookup speed in tcams - without the template hint a >selection of rules may end up looking like a linked list >which is not optimal for lookup Well. Not really, but wider keys have bigger overheads in general. So the motivation is to have the keys as small as possible for both performance and capacity reasons. > >> This is not >> restricted to hw offload. The templates apply to all filters, no matter >> if they are offloaded or not. >> > >Do you save anything with flower(in s/w) if you only added a template >with say dst ip/mask? I can see it will make sense for u32 which is more >flexible and protocol independent. No benefit for flower s/w path at this point. Perhaps the hashtables could be organized in more optimal way with the hint. I didn't look at it. > >cheers, >jamal
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 28/06/18 09:22 AM, Jiri Pirko wrote: Thu, Jun 28, 2018 at 03:13:30PM CEST, j...@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. Well - I was trying to understand what you said ;-> I think what you are getting at is two issues: a) space in the tcams - if the user is just going to enter rules which use one tuple (dst ip for example) the hardware would be better off told that this is the case so it doesnt allocate space in anticipation that someone is going to specify src ip later on. b) lookup speed in tcams - without the template hint a selection of rules may end up looking like a linked list which is not optimal for lookup This is not restricted to hw offload. The templates apply to all filters, no matter if they are offloaded or not. Do you save anything with flower(in s/w) if you only added a template with say dst ip/mask? I can see it will make sense for u32 which is more flexible and protocol independent. cheers, jamal
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Oh, this is v3 already. The changelog should be: --- v2->v3: - patch 5: - rebase on top of the reoffload pathset - patch 6: - rebase on top of the reoffload pathset - patch 9: - adjust to the userspace cmdline changes v1->v2: - patch 6: - remove leftover extack arg in fl_hw_create_tmplt()
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Thu, Jun 28, 2018 at 03:13:30PM CEST, j...@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
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
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
[patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
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. 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 chaintemplate show dev dummy0 ingress Add a template of type flower allowing to insert rules matching on last 2 bytes of destination mac address: # tc chaintemplate 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 chaintemplate show dev dummy0 ingress chaintemplate 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 chaintemplate add dev dummy0 ingress proto ip chain 22 flower dst_ip 0.0.0.0/16 # tc chaintemplate show dev dummy0 ingress chaintemplate flower chain 0 dst_mac 00:00:00:00:00:00/00:00:00:00:ff:ff eth_type ipv4 chaintemplate 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 chaintemplate 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 chaintemplate del dev dummy0 ingress --- v1->v2: - patch 5: - rebase on top of the reoffload pathset - patch 6: - remove leftover extack arg in fl_hw_create_tmplt() - rebase on top of the reoffload pathset - patch 9: - adjust to the userspace cmdline changes Jiri Pirko (9): net: sched: push ops lookup bits into tcf_proto_lookup_ops() net: sched: introduce chain templates net: sched: cls_flower: move key/mask dumping into a separate function net: sched: cls_flower: change fl_init_dissector to accept mask and dissector net: sched: cls_flower: implement chain templates net: sched: cls_flower: propagate chain teplate creation and destruction to drivers mlxsw: spectrum: Implement chain template hinting selftests: forwarding: move shblock tc support check to a separate helper selftests: forwarding: add tests for TC chain templates drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 + drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 12 +- drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 12 +- .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c| 25 +- .../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 44 ++- include/net/pkt_cls.h | 2 + include/net/sch_generic.h | 14 +- include/uapi/linux/rtnetlink.h | 7 + net/sched/cls_api.c| 424 +++-- net/sched/cls_basic.c | 2 +- net/sched/cls_bpf.c| 3 +- net/sched/cls_cgroup.c | 2 +- net/sched/cls_flow.c
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Thu, Jun 28, 2018 at 06:48:26AM CEST, da...@davemloft.net wrote: >From: Jiri Pirko >Date: Tue, 26 Jun 2018 09:59:51 +0200 > >> 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. >> >> This patchset is providing the possibility to user to provide such >> template to kernel and propagate it all the way down to device >> drivers. > >This series doesn't apply cleanly to net-next, and also there seems to still >be some discussion about how the iproute2 command line should look. Will re-spin. Thanks. > >Thanks.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
From: Jiri Pirko Date: Tue, 26 Jun 2018 09:59:51 +0200 > 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. > > This patchset is providing the possibility to user to provide such > template to kernel and propagate it all the way down to device > drivers. This series doesn't apply cleanly to net-next, and also there seems to still be some discussion about how the iproute2 command line should look. Thanks.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Wed, Jun 27, 2018 at 08:34:46AM CEST, sridhar.samudr...@intel.com wrote: > >On 6/26/2018 11:05 PM, Jiri Pirko wrote: >> Wed, Jun 27, 2018 at 02:04:31AM CEST, xiyou.wangc...@gmail.com wrote: >> > On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko wrote: >> > > 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 >> > Now you are extending 'tc filter' command with a new >> > subcommand 'template', which looks weird. >> > >> > Why not make it a new property of filter like you did for chain? >> > Like: >> > >> > tc filter add dev dummy0 ingress proto ip template flower >> But unlike chain, this is not a filter property. For chain, when you add >> filter, you add it to a specific chain. That makes sense. >> But for template, you need to add the template first. Then, later on, >> you add filters which either match or does not match the template. > >So can we say that template defines the types of rules(match fields/masks) that >can be added to a specific chain and there is 1-1 relationship between a >template >and a chain? yes > >Without attaching a template to a chain, i guess it is possible to add >different >types of rules to a chain? yes > > >> Does not make sense to have "template" the filter property as you >> suggest. > >template seems to a chain property. yes > >> > which is much better IMHO. >
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On 6/26/2018 11:05 PM, Jiri Pirko wrote: Wed, Jun 27, 2018 at 02:04:31AM CEST, xiyou.wangc...@gmail.com wrote: On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko wrote: 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 Now you are extending 'tc filter' command with a new subcommand 'template', which looks weird. Why not make it a new property of filter like you did for chain? Like: tc filter add dev dummy0 ingress proto ip template flower But unlike chain, this is not a filter property. For chain, when you add filter, you add it to a specific chain. That makes sense. But for template, you need to add the template first. Then, later on, you add filters which either match or does not match the template. So can we say that template defines the types of rules(match fields/masks) that can be added to a specific chain and there is 1-1 relationship between a template and a chain? Without attaching a template to a chain, i guess it is possible to add different types of rules to a chain? Does not make sense to have "template" the filter property as you suggest. template seems to a chain property. which is much better IMHO.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
Wed, Jun 27, 2018 at 02:04:31AM CEST, xiyou.wangc...@gmail.com wrote: >On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko wrote: >> 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 > >Now you are extending 'tc filter' command with a new >subcommand 'template', which looks weird. > >Why not make it a new property of filter like you did for chain? >Like: > >tc filter add dev dummy0 ingress proto ip template flower But unlike chain, this is not a filter property. For chain, when you add filter, you add it to a specific chain. That makes sense. But for template, you need to add the template first. Then, later on, you add filters which either match or does not match the template. Does not make sense to have "template" the filter property as you suggest. > >which is much better IMHO.
Re: [patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
On Tue, Jun 26, 2018 at 1:01 AM Jiri Pirko wrote: > 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 Now you are extending 'tc filter' command with a new subcommand 'template', which looks weird. Why not make it a new property of filter like you did for chain? Like: tc filter add dev dummy0 ingress proto ip template flower which is much better IMHO.
[patch net-next v2 0/9] net: sched: introduce chain templates support with offloading to mlxsw
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. 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 --- v1->v2: -patch 6: - remove leftover extack arg in fl_hw_create_tmplt() Jiri Pirko (9): net: sched: push ops lookup bits into tcf_proto_lookup_ops() net: sched: introduce chain templates net: sched: cls_flower: move key/mask dumping into a separate function net: sched: cls_flower: change fl_init_dissector to accept mask and dissector net: sched: cls_flower: implement chain templates net: sched: cls_flower: propagate chain teplate creation and destruction to drivers mlxsw: spectrum: Implement chain template hinting selftests: forwarding: move shblock tc support check to a separate helper selftests: forwarding: add tests for TC chain templates drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 5 + drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 12 +- drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 12 +- .../ethernet/mellanox/mlxsw/spectrum_acl_tcam.c| 25 +- .../net/ethernet/mellanox/mlxsw/spectrum_flower.c | 44 ++- include/net/pkt_cls.h | 2 + include/net/sch_generic.h | 14 +- include/uapi/linux/rtnetlink.h | 7 + net/sched/cls_api.c| 424 +++-- net/sched/cls_basic.c | 2 +- net/sched/cls_bpf.c| 3 +- net/sched/cls_cgroup.c | 2 +- net/sched/cls_flow.c | 3 +- net/sched/cls_flower.c | 250 +--- net/sched/cls_fw.c | 3 +-
