lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190424090123.5089586c@bootlin.com> Date: Wed, 24 Apr 2019 09:01:23 +0200 From: Maxime Chevallier <maxime.chevallier@...tlin.com> To: Saeed Mahameed <saeedm@...lanox.com> Cc: "davem@...emloft.net" <davem@...emloft.net>, "miquel.raynal@...tlin.com" <miquel.raynal@...tlin.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux@...linux.org.uk" <linux@...linux.org.uk>, "thomas.petazzoni@...tlin.com" <thomas.petazzoni@...tlin.com>, "mw@...ihalf.com" <mw@...ihalf.com>, "gregory.clement@...tlin.com" <gregory.clement@...tlin.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "stefanc@...vell.com" <stefanc@...vell.com>, "nadavh@...vell.com" <nadavh@...vell.com>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "antoine.tenart@...tlin.com" <antoine.tenart@...tlin.com> Subject: Re: [PATCH net-next v2] net: mvpp2: cls: Add Classification offload support Hello Saeed, Thanks for the review, >> When inserting a rule in a given flow, the location given is relative >> to >> the flow : >> >> ethtool -N eth0 flow-type udp4 dst-port 1234 action 2 loc 0 >> >> ethtool -N eth0 flow-type tcp4 dst-port 1234 action 3 loc 0 >> >> However when removing a rule, the global location is to be used. This >> location can be retrieved by using ethtool -n <interface>. >> > >I am not sure what you mean by "the location given is relative to the >flow", it seems like the rule will end up in a different location than >the user intended, but looking at ethtool documentation it clearly says >that the location the user provides is an absolute rule id/location, >which will be used to delete this rule. > >from "man ethtool": >loc N: >Specify the location/ID to insert the rule. This will overwrite any >rule present in that location and will not go through any of the rule >ordering process. > >delete N >Deletes the RX classification rule with the given ID. I was unsure about this, so I'm glad you commented. One thing that made me think what I did could be okay is that the documentation for ETHTOOL_SRXCLSRLINS in ethtool.h says : "For %ETHTOOL_SRXCLSRLINS, @fs specifies the rule to add or update. @fs.@...ation either specifies the location to use or is a special location value with %RX_CLS_LOC_SPECIAL flag set. On return, @fs.@...ation is the actual rule location." I interpreted the "On return [...]" part as if we could rewrite the location if needed when inserting a rule (although it seems ethtool doesn't do anything with this return value) The point for doing so is that we have a clear separation in our classification tables between different traffic classes, so we have a range of entries for tcp4, one for udp4, one for tcp6, etc. Having a "global" location numbering scheme would, I think, also be confusing, since it would make the user use loc 0->7 for tcp4, loc 8->15 for udp4 and so on. Maybe in this case I should stick with insertions thay rely on RX_CLS_LOC_SPECIAL (such as "first", "last", "any") and have a scheme where priorisation is based strictly on the rule insertion order ? >So the above example should result in one flow rule in your hardware. >but according the code below the calculated index in >mvpp2_ethtool_cls_rule_ins might end up different than the requested >location, which will confuse the user. I'm also working on writing a proper documentation for this driver, including the behaviour of the classifier implementation, hopefully this would help. Thanks again for the review, Maxime
Powered by blists - more mailing lists