From: Olivier Matz <olivier.matz@6wind.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: "Richardson, Bruce" <bruce.richardson@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"mb@smartsharesystems.com" <mb@smartsharesystems.com>,
"Chilikin, Andrey" <andrey.chilikin@intel.com>,
"jblunck@infradead.org" <jblunck@infradead.org>,
"nelio.laranjeiro@6wind.com" <nelio.laranjeiro@6wind.com>,
"arybchenko@solarflare.com" <arybchenko@solarflare.com>
Subject: Re: [dpdk-dev] [PATCH 0/9] mbuf: structure reorganization
Date: Fri, 31 Mar 2017 10:41:07 +0200 [thread overview]
Message-ID: <20170331104107.5b29cc0e@platinum> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583FAE2B2F@IRSMSX109.ger.corp.intel.com>
On Thu, 30 Mar 2017 18:06:35 +0000, "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Thursday, March 30, 2017 5:48 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; Olivier Matz
> > <olivier.matz@6wind.com>
> > Cc: dev@dpdk.org; mb@smartsharesystems.com; Chilikin, Andrey <andrey.chilikin@intel.com>; 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 <bruce.richardson@intel.com>; Olivier Matz <olivier.matz@6wind.com>
> > > Cc: dev@dpdk.org; mb@smartsharesystems.com; Chilikin, Andrey <andrey.chilikin@intel.com>; 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 <olivier.matz@6wind.com>
> > > > Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>; mb@smartsharesystems.com; Chilikin, Andrey
> > > > <andrey.chilikin@intel.com>; 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 <bruce.richardson@intel.com> 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
next prev parent reply other threads:[~2017-03-31 8:41 UTC|newest]
Thread overview: 155+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-24 15:19 [dpdk-dev] [RFC 0/8] " Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 1/8] mbuf: make segment prefree function public Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 2/8] mbuf: make raw free " Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 3/8] mbuf: set mbuf fields while in pool Olivier Matz
2017-01-24 15:50 ` Bruce Richardson
2017-02-28 14:51 ` Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 4/8] net: don't touch mbuf next or nb segs on Rx Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 5/8] mbuf: make rearm data address naturally aligned Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 6/8] mbuf: use 2 bytes for port and nb segments Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 7/8] mbuf: move sequence number in second cache line Olivier Matz
2017-01-24 15:19 ` [dpdk-dev] [RFC 8/8] mbuf: add a timestamp field Olivier Matz
2017-01-24 15:59 ` [dpdk-dev] [RFC 0/8] mbuf: structure reorganization Bruce Richardson
2017-01-24 16:16 ` Olivier MATZ
2017-02-06 18:41 ` Ananyev, Konstantin
2017-02-09 16:20 ` Morten Brørup
2017-02-09 16:56 ` Ananyev, Konstantin
2017-02-16 13:48 ` Olivier Matz
2017-02-16 15:46 ` Bruce Richardson
2017-02-16 16:14 ` Olivier Matz
2017-02-21 14:20 ` Morten Brørup
2017-02-21 14:28 ` Bruce Richardson
2017-02-21 15:04 ` Olivier MATZ
2017-02-21 15:18 ` Bruce Richardson
2017-02-21 15:18 ` Morten Brørup
2017-02-19 19:04 ` Chilikin, Andrey
2017-02-21 9:53 ` Olivier MATZ
2017-02-16 17:26 ` Jan Blunck
2017-02-17 10:51 ` Olivier Matz
2017-02-17 12:49 ` Nélio Laranjeiro
2017-02-17 13:51 ` Jan Blunck
2017-02-18 5:48 ` Andrew Rybchenko
2017-02-17 13:38 ` Jan Blunck
2017-02-17 14:17 ` Olivier Matz
2017-02-17 18:42 ` Ananyev, Konstantin
2017-02-21 9:53 ` Olivier MATZ
2017-02-21 10:28 ` Ananyev, Konstantin
2017-02-20 9:27 ` Jan Blunck
2017-02-21 9:54 ` Olivier MATZ
2017-02-21 16:12 ` Jan Blunck
2017-02-21 16:38 ` Bruce Richardson
2017-02-21 17:04 ` Jan Blunck
2017-02-21 17:26 ` Ananyev, Konstantin
2017-02-21 19:17 ` Jan Blunck
2017-02-21 20:30 ` Ananyev, Konstantin
2017-02-21 21:51 ` Morten Brørup
2017-02-24 14:11 ` Olivier Matz
2017-02-24 14:00 ` Olivier Matz
2017-02-24 14:21 ` Bruce Richardson
2017-02-28 8:55 ` Jan Blunck
2017-02-28 9:05 ` Ananyev, Konstantin
2017-02-28 9:23 ` Olivier Matz
2017-02-28 9:33 ` Jan Blunck
2017-02-28 10:29 ` Ananyev, Konstantin
2017-02-28 10:50 ` Olivier Matz
2017-02-28 11:48 ` Ananyev, Konstantin
2017-02-28 12:28 ` Olivier Matz
2017-02-28 22:53 ` Ananyev, Konstantin
2017-03-02 16:46 ` Olivier Matz
2017-03-08 11:11 ` Ananyev, Konstantin
2017-03-20 9:00 ` Olivier Matz
2017-03-22 17:42 ` Ananyev, Konstantin
2017-03-24 8:35 ` Jerin Jacob
2017-03-24 13:35 ` Olivier Matz
2017-02-28 9:25 ` Jan Blunck
2017-02-19 23:45 ` Ananyev, Konstantin
2017-02-21 9:22 ` Morten Brørup
2017-02-21 9:54 ` Olivier MATZ
2017-03-08 9:41 ` [dpdk-dev] [PATCH 0/9] " Olivier Matz
2017-03-08 9:41 ` [dpdk-dev] [PATCH 1/9] mbuf: make segment prefree function public Olivier Matz
2017-03-08 9:41 ` [dpdk-dev] [PATCH 2/9] mbuf: make raw free " Olivier Matz
2017-03-08 9:41 ` [dpdk-dev] [PATCH 3/9] mbuf: set mbuf fields while in pool Olivier Matz
2017-03-31 11:21 ` Bruce Richardson
2017-03-31 11:51 ` Ananyev, Konstantin
2017-03-08 9:41 ` [dpdk-dev] [PATCH 4/9] drivers/net: don't touch mbuf next or nb segs on Rx Olivier Matz
2017-03-08 9:41 ` [dpdk-dev] [PATCH 5/9] mbuf: make rearm data address naturally aligned Olivier Matz
2017-03-08 9:41 ` [dpdk-dev] [PATCH 6/9] mbuf: use 2 bytes for port and nb segments Olivier Matz
2017-03-08 9:41 ` [dpdk-dev] [PATCH 7/9] mbuf: move sequence number in second cache line Olivier Matz
2017-03-08 9:42 ` [dpdk-dev] [PATCH 8/9] mbuf: add a timestamp field Olivier Matz
2017-04-04 10:29 ` [dpdk-dev] [PATCH 0/2] reduce writes to mbuf in ixgbe vRX Konstantin Ananyev
2017-04-07 15:13 ` Ferruh Yigit
2017-04-07 15:44 ` Ferruh Yigit
2017-04-09 22:56 ` Ananyev, Konstantin
2017-04-04 10:29 ` [dpdk-dev] [PATCH 1/2] net/ixgbe: eliminate mbuf write on rearm Konstantin Ananyev
2017-04-10 15:59 ` [dpdk-dev] [PATCH v2 0/2] reduce writes to mbuf in ixgbe vRX Konstantin Ananyev
2017-04-10 16:17 ` Ferruh Yigit
2017-04-10 15:59 ` [dpdk-dev] [PATCH v2 1/2] net/ixgbe: eliminate mbuf write on rearm Konstantin Ananyev
2017-04-10 15:59 ` [dpdk-dev] [PATCH v2 2/2] net/ixgbe: remove option to disable offload flags Konstantin Ananyev
2017-04-04 10:29 ` [dpdk-dev] [PATCH " Konstantin Ananyev
2017-03-08 9:42 ` [dpdk-dev] [PATCH 9/9] mbuf: reorder VLAN tci and buffer len fields Olivier Matz
2017-03-29 15:56 ` [dpdk-dev] [PATCH 0/9] mbuf: structure reorganization Olivier Matz
2017-03-29 16:03 ` Morten Brørup
2017-03-29 20:09 ` Bruce Richardson
2017-03-30 9:31 ` Bruce Richardson
2017-03-30 12:02 ` Olivier Matz
2017-03-30 12:23 ` Bruce Richardson
2017-03-30 16:45 ` Ananyev, Konstantin
2017-03-30 16:47 ` Ananyev, Konstantin
2017-03-30 18:06 ` Ananyev, Konstantin
2017-03-31 8:41 ` Olivier Matz [this message]
2017-03-31 9:58 ` Ananyev, Konstantin
2017-03-31 1:00 ` Ananyev, Konstantin
2017-03-31 7:21 ` Morten Brørup
2017-03-31 8:26 ` Olivier Matz
2017-03-31 8:41 ` Bruce Richardson
2017-03-31 8:59 ` Olivier Matz
2017-03-31 9:18 ` Ananyev, Konstantin
2017-03-31 9:36 ` Olivier Matz
2017-04-03 16:15 ` Thomas Monjalon
2017-04-04 7:58 ` Olivier MATZ
2017-04-04 8:53 ` Bruce Richardson
2017-03-31 9:23 ` Bruce Richardson
2017-03-31 11:18 ` Nélio Laranjeiro
2017-03-30 14:54 ` Andrew Rybchenko
2017-03-30 15:12 ` Jerin Jacob
2017-04-04 16:27 ` [dpdk-dev] [PATCH v2 0/8] " Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 1/8] mbuf: make segment prefree function public Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 2/8] mbuf: make raw free " Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 3/8] mbuf: set mbuf fields while in pool Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 4/8] drivers/net: don't touch mbuf next or nb segs on Rx Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 5/8] mbuf: make rearm data address naturally aligned Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and nb segments Olivier Matz
2017-04-06 5:45 ` Yuanhan Liu
2017-04-18 13:03 ` Olivier MATZ
2017-07-04 7:54 ` Wang, Zhihong
2017-07-10 8:00 ` Olivier Matz
2017-07-10 8:15 ` Morten Brørup
2017-07-11 13:25 ` Wiles, Keith
2017-07-11 13:30 ` Morten Brørup
2017-07-11 15:05 ` Thomas Monjalon
2017-07-11 15:23 ` [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and nbsegments Morten Brørup
2017-07-11 16:48 ` Wiles, Keith
2017-07-12 7:25 ` Morten Brørup
2017-07-12 9:02 ` Yang, Zhiyong
2017-07-12 9:50 ` [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port andnbsegments Morten Brørup
2017-07-12 15:35 ` Stephen Hemminger
2017-07-12 15:57 ` Morten Brørup
2017-07-12 16:23 ` Thomas Monjalon
2017-07-12 18:20 ` Wiles, Keith
2017-07-21 15:03 ` Bruce Richardson
2017-07-12 15:34 ` [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and nbsegments Wiles, Keith
2017-07-11 13:34 ` [dpdk-dev] [PATCH v2 6/8] mbuf: use 2 bytes for port and nb segments Wiles, Keith
2017-07-11 13:46 ` Olivier MATZ
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 7/8] mbuf: move sequence number in second cache line Olivier Matz
2017-04-04 16:28 ` [dpdk-dev] [PATCH v2 8/8] mbuf: add a timestamp field Olivier Matz
2017-04-05 9:37 ` [dpdk-dev] [PATCH v2 0/8] mbuf: structure reorganization Thomas Monjalon
2017-04-05 9:46 ` Olivier MATZ
2017-04-05 9:48 ` Richardson, Bruce
2017-04-05 12:06 ` Ferruh Yigit
2017-04-14 13:10 ` Ferruh Yigit
2017-04-18 13:04 ` Olivier MATZ
2017-04-19 9:39 ` Thomas Monjalon
2017-04-19 12:28 ` Olivier MATZ
2017-04-19 12:56 ` Thomas Monjalon
2017-04-19 13:03 ` Ferruh Yigit
2017-04-19 13:12 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170331104107.5b29cc0e@platinum \
--to=olivier.matz@6wind.com \
--cc=andrey.chilikin@intel.com \
--cc=arybchenko@solarflare.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=jblunck@infradead.org \
--cc=konstantin.ananyev@intel.com \
--cc=mb@smartsharesystems.com \
--cc=nelio.laranjeiro@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).