From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f178.google.com (mail-wr0-f178.google.com [209.85.128.178]) by dpdk.org (Postfix) with ESMTP id 7385F2B92 for ; Fri, 31 Mar 2017 10:41:11 +0200 (CEST) Received: by mail-wr0-f178.google.com with SMTP id w11so94266513wrc.3 for ; Fri, 31 Mar 2017 01:41:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=nIqvHGr41McbrK27cG6wdMVDA8mf4dYOeQVaE0SO3DQ=; b=D5pPOdlsalf2Que38+TXBRGilShJ/A4CTM2uLoa+AGmoyCRg7Jj5JZONn+n4LXjSd5 4suRbAu+hP6cni4EJrofsp5bxPUqmp/t93zeN+TCY9wcVXA+QOBVOnXhkiDlyxdhFo76 bBWN+W/OsL9zA0PqCjHmnJjCltUZ7IdNIL4zzKGgTmzEIQ5ULZmvqFjQQUyNEqPNhrlM 7Lg4KP+mQ0DF+tWgfNnJucpnU/pOBZ1c+jM1X6Eo1ho2JD9m3g8+w1S9F+N+zCwFeLC/ ogIFtvdNzKWGO108kuZQiytdSRvZAySisZLhsD5gwc2dF+rGqjP7GAI/KMOWG7pDXlc+ talw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=nIqvHGr41McbrK27cG6wdMVDA8mf4dYOeQVaE0SO3DQ=; b=Hc5uMC7l9icRRaoVRbnVi1iWxU6a9lh819yNQCRMobSjvZzBB7BdagkS9iGQVLDZG6 x98sqkK/VbLQcJHZNfSJanIoFHBFfwv/kVvT+N0dtsdHpD4z/94JR9agDsaNJAufcXXa fj/OHrA0yfHCgMPyou1wpcnsWoONG4t1+RoNtQ8Sc/i3WDywUtfW1uC7aDqVfmM43zXP zjUAvTqAbgX3DBNPzziKJPCnfqaYfhPRUUiuQ8dfyE35xXr2LAUHDEwWfPvhtke9bexO n375iP++pdR38AiQRPLyPnDI8d9b9FTYQ8ljbUKgyCxN7O4+FPq61VpO797+qjNGvSqR Eigw== X-Gm-Message-State: AFeK/H3ey4PzFsk3ppRIP27ZHdzNCxzmM/8ivuePYpdyMooZiyw8Z+6V2CsymLzueU1Ojqr0 X-Received: by 10.28.28.69 with SMTP id c66mr2173368wmc.28.1490949670956; Fri, 31 Mar 2017 01:41:10 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id o15sm5813521wra.61.2017.03.31.01.41.10 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 31 Mar 2017 01:41:10 -0700 (PDT) Date: Fri, 31 Mar 2017 10:41:07 +0200 From: Olivier Matz To: "Ananyev, Konstantin" Cc: "Richardson, Bruce" , "dev@dpdk.org" , "mb@smartsharesystems.com" , "Chilikin, Andrey" , "jblunck@infradead.org" , "nelio.laranjeiro@6wind.com" , "arybchenko@solarflare.com" Message-ID: <20170331104107.5b29cc0e@platinum> In-Reply-To: <2601191342CEEE43887BDE71AB9772583FAE2B2F@IRSMSX109.ger.corp.intel.com> References: <1485271173-13408-1-git-send-email-olivier.matz@6wind.com> <1488966121-22853-1-git-send-email-olivier.matz@6wind.com> <20170329175629.68810924@platinum> <20170329200923.GA11516@bricha3-MOBL3.ger.corp.intel.com> <20170330093108.GA10652@bricha3-MOBL3.ger.corp.intel.com> <20170330140236.0d2ebac8@platinum> <20170330122305.GA14272@bricha3-MOBL3.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583FAE2A51@IRSMSX109.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583FAE2A6E@IRSMSX109.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772583FAE2B2F@IRSMSX109.ger.corp.intel.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 0/9] mbuf: structure reorganization 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: , X-List-Received-Date: Fri, 31 Mar 2017 08:41:11 -0000 On Thu, 30 Mar 2017 18:06:35 +0000, "Ananyev, Konstantin" wrote: > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Thursday, March 30, 2017 5:48 PM > > To: Ananyev, Konstantin ; Richardson, Bruce ; Olivier Matz > > > > Cc: dev@dpdk.org; mb@smartsharesystems.com; Chilikin, Andrey ; jblunck@infradead.org; > > nelio.laranjeiro@6wind.com; arybchenko@solarflare.com > > Subject: RE: [dpdk-dev] [PATCH 0/9] mbuf: structure reorganization > > > > > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin > > > Sent: Thursday, March 30, 2017 5:45 PM > > > To: Richardson, Bruce ; Olivier Matz > > > Cc: dev@dpdk.org; mb@smartsharesystems.com; Chilikin, Andrey ; jblunck@infradead.org; > > > nelio.laranjeiro@6wind.com; arybchenko@solarflare.com > > > Subject: Re: [dpdk-dev] [PATCH 0/9] mbuf: structure reorganization > > > > > > > > > > > > > -----Original Message----- > > > > From: Richardson, Bruce > > > > Sent: Thursday, March 30, 2017 1:23 PM > > > > To: Olivier Matz > > > > Cc: dev@dpdk.org; Ananyev, Konstantin ; mb@smartsharesystems.com; Chilikin, Andrey > > > > ; jblunck@infradead.org; nelio.laranjeiro@6wind.com; arybchenko@solarflare.com > > > > Subject: Re: [dpdk-dev] [PATCH 0/9] mbuf: structure reorganization > > > > > > > > On Thu, Mar 30, 2017 at 02:02:36PM +0200, Olivier Matz wrote: > > > > > On Thu, 30 Mar 2017 10:31:08 +0100, Bruce Richardson wrote: > > > > > > On Wed, Mar 29, 2017 at 09:09:23PM +0100, Bruce Richardson wrote: > > > > > > > On Wed, Mar 29, 2017 at 05:56:29PM +0200, Olivier Matz wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > Does anyone have any other comment on this series? > > > > > > > > Can it be applied? > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Olivier > > > > > > > > > > > > > > > > > > > > > > I assume all driver maintainers have done performance analysis to check > > > > > > > for regressions. Perhaps they can confirm this is the case. > > > > > > > > > > > > > > /Bruce > > > > > > > > > > > > > > In the absence, of anyone else reporting performance numbers with this > > > > > > patchset, I ran a single-thread testpmd test using 2 x 40G ports (i40e) > > > > > > driver. With RX & TX descriptor ring sizes of 512 or above, I'm seeing a > > > > > > fairly noticable performance drop. I still need to dig in more, e.g. do > > > > > > an RFC2544 zero-loss test, and also bisect the patchset to see what > > > > > > parts may be causing the problem. > > > > > > > > > > > > Has anyone else tried any other drivers or systems to see what the perf > > > > > > impact of this set may be? > > > > > > > > > > I did, of course. I didn't see any noticeable performance drop on > > > > > ixgbe (4 NICs, one port per NIC, 1 core). I can replay the test with > > > > > current version. > > > > > > > > > I had no doubt you did some perf testing! :-) > > > > > > > > Perhaps the regression I see is limited to i40e driver. I've confirmed I > > > > still see it with that driver in zero-loss tests, so next step is to try > > > > and localise what change in the patchset is causing it. > > > > > > > > Ideally, though, I think we should see acks or other comments from > > > > driver maintainers at least confirming that they have tested. You cannot > > > > be held responsible for testing every DPDK driver before you submit work > > > > like this. > > > > > > > > > > Unfortunately I also see a regression. > > > Did a quick flood test on 2.8 GHZ IVB with 4x10Gb. > > > > Sorry, forgot to mention - it is on ixgbe. > > So it doesn't look like i40e specific. > > > > > Observed a drop even with default testpmd RXD/TXD numbers (128/512): > > > from 50.8 Mpps down to 47.8 Mpps. > > > From what I am seeing the particular patch that causing it: > > > [dpdk-dev,3/9] mbuf: set mbuf fields while in pool > > > > > > cc version 5.3.1 20160406 (Red Hat 5.3.1-6) (GCC) > > > cmdline: > > > ./dpdk.org-1705-mbuf1/x86_64-native-linuxapp-gcc/app/testpmd --lcores='7,8' -n 4 --socket-mem='1024,0' -w 04:00.1 -w 07:00.1 -w > > > 0b:00.1 -w 0e:00.1 -- -i > > > > > Actually one more question regarding: > [dpdk-dev,9/9] mbuf: reorder VLAN tci and buffer len fields > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index fd97bd3..ada98d5 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -449,8 +449,7 @@ struct rte_mbuf { > > uint32_t pkt_len; /**< Total pkt len: sum of all segments. */ > uint16_t data_len; /**< Amount of data in segment buffer. */ > - /** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */ > - uint16_t vlan_tci; > + uint16_t buf_len; /**< Size of segment buffer. */ > > union { > uint32_t rss; /**< RSS hash result if RSS enabled */ > @@ -475,11 +474,11 @@ struct rte_mbuf { > uint32_t usr; /**< User defined tags. See rte_distributor_process() */ > } hash; /**< hash information */ > > + /** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */ > + uint16_t vlan_tci; > /** Outer VLAN TCI (CPU order), valid if PKT_RX_QINQ_STRIPPED is set. */ > uint16_t vlan_tci_outer; > > - uint16_t buf_len; /**< Length of segment buffer. */ > - > /** Valid if PKT_RX_TIMESTAMP is set. The unit and time reference > * are not normalized but are always the same for a given port. > */ > > How ixgbe and i40e SSE version supposed to work correctly after that change? > As I remember both of them sets vlan_tci as part of 16B shuffle operation. > Something like that: > pkt_mb4 = _mm_shuffle_epi8(descs[3], shuf_msk); > ... > mm_storeu_si128((void *)&rx_pkts[pos+3]->rx_descriptor_fields1, > pkt_mb4); > > But now vlan_tci is swapped with buf_len. > Which means 2 things to me: > It is more than 16B away from rx_descriptor_fields1 and can't be updated in one go anymore, > and instead of vlan_tci we are updating buf_len. Sorry, I missed it. But this shows something problematic: changing the order of fields in a structure breaks code without notification. I think that drivers expecting a field at a specific position should have some BUG_ON() to check that the condition is still valid. We can't expect anyone to know all the constraints of all vectors PMDs in DPDK. The original idea of this patch was to group vlan_tci and vlan_outer_tci, which looked to be a good idea at first glance. If it requires to change all vector code, let's drop it. Just for the exercice, let's imagine we need that patch. What would be the procedure to have it integrated? How can we detect there is an issue? Who would be in charge of modifying all the vector code in PMDs? Regards, Olivier