From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 442EEA04A4; Wed, 3 Jun 2020 16:56:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 68A1F1D547; Wed, 3 Jun 2020 16:56:15 +0200 (CEST) Received: from new4-smtp.messagingengine.com (new4-smtp.messagingengine.com [66.111.4.230]) by dpdk.org (Postfix) with ESMTP id 83D511C1B7; Wed, 3 Jun 2020 16:56:13 +0200 (CEST) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailnew.nyi.internal (Postfix) with ESMTP id AF6FE580168; Wed, 3 Jun 2020 10:56:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute7.internal (MEProxy); Wed, 03 Jun 2020 10:56:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm1; bh= t7ktq2GVrlo1fugyudRy6h9Gu2BEot07W3JBqCIL1Fw=; b=niGzaHZCnzxvr5Le bqPuDXqcG1fQ5oWvltKG4twxlFkCUuUaQpySAJVeuD17KLzBLrmySAHhvYyhGYvA 8NWhaoFb9YqOmKpCdJfo0pALZ6bmkc5WT3QwkmSfJZELwaMSJMOWi9jN6fshAlx3 OUcGWbrq6RzLDrHOWphiurmNOnk0JreUqhhNmTromT8LcZRgR+jCk/nSBRLTFHiV RvZBNiqExLRaAhZgzkK03wj3WgksPh6jScQnN4MfVDQ0ZiCxpTu/e/qsca21c4Zo t4Xc85xKFztFg9y2RzhhyKzWrmElJa5yzm7l2oEV+EsFTne4hvAzW5vpltrVaJfU KQKmWg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=t7ktq2GVrlo1fugyudRy6h9Gu2BEot07W3JBqCIL1 Fw=; b=Jo+5YjsfYuCVH1hOSAndlOBATHTY/DYGIhkoz6ed2wuIFApIdFQz95PFa C6+iDXt8QPAYOCwzIJ/7J8eaRxHDoTpF409meqrDqwXQWOcM8VTQv3HQ4Ak3lkhA xSJ3+9FgQOmdhlvZYSO1yY9mczEZ3VGRwRn0eQMiCRbJigg++rzXLOX8QHno7rV+ zXzb1gxeBsZE3QB28/otpZUprd3Bprr1uwcJaMk4xtb1ejmEwdyVf9Z2ZC0wo3ab HApkg5vn1f9supP45m5QhReU5K24VqI5AqVyupX0E7d9Qno3KV/AE2JfzTJnmN66 19VatLpapyM6PUO8hV/oyPsFA32FQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrudefledgheelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepffdvffejueetleefieeludduuefgteejleevfeekjeefieegheet ffdvkeefgedunecuffhomhgrihhnpeguphgukhdrohhrghenucfkphepjeejrddufeegrd dvtdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhf rhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 2ECA23062499; Wed, 3 Jun 2020 10:56:10 -0400 (EDT) From: Thomas Monjalon To: Nithin Dabilpuram Cc: dev@dpdk.org, "Ananyev, Konstantin" , Jerin Jacob , "Dumitrescu, Cristian" , Nithin Dabilpuram , "Yigit, Ferruh" , Andrew Rybchenko , Ori Kam , "Burakov, Anatoly" , "Mcnamara, John" , "Kovacevic, Marko" , dpdk-dev , Jerin Jacob , Krzysztof Kanas , "techboard@dpdk.org" , Olivier Matz Date: Wed, 03 Jun 2020 16:56:08 +0200 Message-ID: <1634872.Uel37ra6X5@thomas> In-Reply-To: <20200603113822.GI12564@platinum> References: <20200514202931.GF1739@platinum> <20200603104414.GA28936@outlook.office365.com> <20200603113822.GI12564@platinum> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [EXT] Re: [PATCH 1/3] mbuf: add Tx offloads for packet marking X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 03/06/2020 13:38, Olivier Matz: > On Wed, Jun 03, 2020 at 04:14:14PM +0530, Nithin Dabilpuram wrote: > > On Wed, Jun 03, 2020 at 10:28:44AM +0200, Olivier Matz wrote: > > > On Tue, Jun 02, 2020 at 07:55:37PM +0530, Nithin Dabilpuram wrote: > > > > On Tue, Jun 02, 2020 at 10:53:08AM +0000, Ananyev, Konstantin wrote: > > > > > > > > > I also share Olivier's concern about consuming 3 bits in ol_flags for that feature. > > > > > > > > > Can it probably be squeezed somehow? > > > > > > > > > Let say we reserve one flag that this information is present or not, and > > > > > > > > > re-use one of rx-only fields for store additional information (packet_type, or so). > > > > > > > > > Or might be some other approach. > > > > > > > > > > > > > > > > We are fine with this approach where we define one bit in Tx offloads for pkt > > > > > > > > marking and and 3 bits reused from Rx offload flags area. [...] > > > I'm not a big fan of reusing Rx fields or flags for Tx. > > > It's not obvious for an application than adding a tx_mark will overwrite > > > the packet_type. I understand that the risk is limited because packet_type > > > is Rx and the marks are Tx, but there is still one. Mixing Rx and Tx info in the same field is a bad design pattern which will create a lot of difficult bugs. > > I'm also not a big fan but just wanted to take this approach so that, > > it can both conserve space and also help fast path. > > Reusing Rx area is however not a new thing as is already followed for > > mbuf->txadapter field. Yes there is a txadapter field union'ed with flow director and QoS. This is a bad pattern that I highlighted in this presentation (slide 8): https://www.dpdk.org/wp-content/uploads/sites/35/2019/10/DynamicMbuf.pdf > Yes, and in my opinion this is something we should avoid when possible, > because it makes some features exclusive (ex: the big union with > sched/rss/adapter/usr/...). Yes, the "RSS union" must be cleaned-up, as some other mbuf parts. > > Apart from documentation issue, Is there any other issue or future > > ramification with using Rx field's for Tx ? > > No, I don't see any other issue except the ones we already mentioned > (doc, code clarity, ). "doc clarity" should be understood as the opposite of "design leading inevitably to bugs". > > If it is only about documentation, then we can add more documentation to make things clear. More documentation won't make a bad design better, unfortunately. > > > To summarize the different proposed approaches (please correct me if I'm wrong): > > > > > > a- add 3 Tx mbuf flags > > > (-) consumes limited resource > > > > > > b- add 3 dynamic flags > > > (-) slower > > > > - Tx burst Vector implementation can't be done for this tx offload as > > offset keeps changing. > > A vector implementation can be done. But yes, it would be slower than > with a static flag. > > > > c- add 1 Tx flag and union with Rx field > > > (-) exclusive with Rx field > > > (-) still consumes one flag > > > > > > My preference is still b-, for these reasons: Me too, my preference is (b).