From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AE371C10F11 for ; Wed, 24 Apr 2019 07:01:48 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7EC0D20878 for ; Wed, 24 Apr 2019 07:01:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="W13Pnqid" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7EC0D20878 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=bootlin.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sjlXjhMCyxx+pDMWIhstgKZsWI1EzXDqK6fbSH3Glxc=; b=W13PnqidEvpClf wamu8yL2U+RPhaJoo3jbN7hJwQg4SnBMVzpMxzREXf46j+scMVDIXJEgs28rO8s+ELRvgWp9MHVRR 2hU7eaQV9Sik8et9pcFvxYNp7MlQkqC1CplpG/LmbzBlc8dSYgp52rzpzyuexU0t+lUHRtqVpWZVP RzIy/S9EfRwsoLFxgkH2UPlRPIUmRT1X3nvHpu+Rvht73fDKNdkJaSKxcmYmzbfHx8MXVZ4t+NENV cA8zTn8PmhB+cw6WQo4FhvSY6snAR+nTSxFx9tk9UuXTPEUj8yZVBG0v7Kz8RrIinlE+ZxcBytyqp 4HphAPfPN6CgisHRRTgw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJBuW-0000uc-8M; Wed, 24 Apr 2019 07:01:40 +0000 Received: from relay7-d.mail.gandi.net ([217.70.183.200]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJBuS-0000u5-JM for linux-arm-kernel@lists.infradead.org; Wed, 24 Apr 2019 07:01:38 +0000 X-Originating-IP: 90.88.147.33 Received: from bootlin.com (aaubervilliers-681-1-27-33.w90-88.abo.wanadoo.fr [90.88.147.33]) (Authenticated sender: maxime.chevallier@bootlin.com) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 09DB520019; Wed, 24 Apr 2019 07:01:23 +0000 (UTC) Date: Wed, 24 Apr 2019 09:01:23 +0200 From: Maxime Chevallier To: Saeed Mahameed Subject: Re: [PATCH net-next v2] net: mvpp2: cls: Add Classification offload support Message-ID: <20190424090123.5089586c@bootlin.com> In-Reply-To: References: <20190423075031.26074-1-maxime.chevallier@bootlin.com> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190424_000136_935051_DC596200 X-CRM114-Status: GOOD ( 13.82 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "antoine.tenart@bootlin.com" , "netdev@vger.kernel.org" , "gregory.clement@bootlin.com" , "linux@armlinux.org.uk" , "linux-kernel@vger.kernel.org" , "nadavh@marvell.com" , "thomas.petazzoni@bootlin.com" , "miquel.raynal@bootlin.com" , "stefanc@marvell.com" , "mw@semihalf.com" , "davem@davemloft.net" , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 . >> > >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.@location either specifies the location to use or is a special location value with %RX_CLS_LOC_SPECIAL flag set. On return, @fs.@location 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel