From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Olivier Matz" <olivier.matz@6wind.com>,
"Bruce Richardson" <bruce.richardson@intel.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC 0/8] mbuf: structure reorganization
Date: Tue, 21 Feb 2017 15:20:23 +0100 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC359EAD0D@smartserver.smartshare.dk> (raw)
In-Reply-To: <20170216171410.57bff4ed@platinum>
Comments at the end.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Thursday, February 16, 2017 5:14 PM
> To: Bruce Richardson
> Cc: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC 0/8] mbuf: structure reorganization
>
> On Thu, 16 Feb 2017 15:46:19 +0000, Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > On Thu, Feb 16, 2017 at 02:48:07PM +0100, Olivier Matz wrote:
> > > Hi Konstantin,
> > >
> > > Thanks for the feedback.
> > > Comments inline.
> > >
> > >
> > > On Mon, 6 Feb 2017 18:41:27 +0000, "Ananyev, Konstantin"
> > > <konstantin.ananyev@intel.com> wrote:
> > > > Hi Olivier,
> > > > Looks good in general, some comments from me below.
> > > > Thanks
> > > > Konstantin
> > > >
> > > > >
> > > > > The main changes are:
> > > > > - reorder structure to increase vector performance on some
> > > > > non-ia platforms.
> > > > > - add a 64bits timestamp field in the 1st cache line
> > > >
> > > > Wonder why it deserves to be in first cache line?
> > > > How it differs from seqn below (pure SW stuff right now).
> > >
> > > In case the timestamp is set from a NIC value, it is set in the Rx
> > > path. So that's why I think it deserve to be located in the 1st
> > > cache line.
> > >
> > > As you said, the seqn is a pure sw stuff right: it is set in a lib,
> > > not in a PMD rx path.
> > >
> > > > > - m->next, m->nb_segs, and m->refcnt are always initialized for
> > > > > mbufs in the pool, avoiding the need of setting m->next
> (located
> > > > > in the 2nd cache line) in the Rx path for mono-segment packets.
> > > > > - change port and nb_segs to 16 bits
> > > >
> > > > Not that I am completely against it, but changing nb_segs to 16
> > > > bits seems like an overkill to me.
> > > > I think we can keep and extra 8bits for something more useful in
> > > > future.
> > >
> > > In my case, I use the m->next field to chain more than 256 segments
> > > for L4 socket buffers. It also updates nb_seg that can overflow.
> > > It's not a big issue since at the end, nb_seg is decremented for
> > > each segment. On the other hand, if I enable some sanity checks on
> > > mbufs, it complains because the number of segments is not equal to
> > > nb_seg.
> > >
> > > There is also another use case with fragmentation as discussed
> > > recently: http://dpdk.org/dev/patchwork/patch/19819/
> > >
> > > Of course, dealing with a long mbuf list is not that efficient, but
> > > the application can maintain another structure to accelerate the
> > > access to the middle/end of the list.
> > >
> > > Finally, we have other ideas to get additional 8 bits if required
> in
> > > the future, so I don't think it's really a problem.
> > >
> > >
> > > >
> > > > > - move seqn in the 2nd cache line
> > > > >
> > > > > Things discussed but not done in the patchset:
> > > > > - move refcnt and nb_segs to the 2nd cache line: many drivers
> > > > > sets them in the Rx path, so it could introduce a performance
> > > > > regression, or
> > > >
> > > > I wonder can refcnt only be moved into the 2-nd cacheline?
> > > > As I understand thanks to other change (from above) m->refcnt
> will
> > > > already be initialized, so RX code don't need to touch it.
> > > > Though yes, it still would require changes in all PMDs.
> > >
> > > Yes, I agree, some fields could be moved in the 2nd cache line once
> > > all PMDs stop to write them in RX path. I propose to issue some
> > > guidelines to PMD maintainers at the same time the patchset is
> > > pushed. Then we can consider changing it in a future version, in
> > > case we need more room in the 1st mbuf cache line.
> > >
> >
> > If we are changing things, we should really do all that now, rather
> > than storing up future breaks to mbuf. Worst case, we should plan for
> > it immediately after the release where we make these changes. Have
> two
> > releases that break mbuf immediately after each other - and flagged
> as
> > such, but keep it stable thereafter. I don't like having technical
> > debt on mbuf just after we supposedly "fix" it.
>
> I think there is no need to do this change now. And I don't feel good
> with the idea of having a patchset that updates all the PMDs to remove
> the access to a field because it moved to the 2nd cache line
> (especially thinking about vector PMDs).
>
> That's why I think the plan could be:
> - push an updated version of this patchset quickly
> - advertise to PMD maintainers "you don't need to set the m->next,
> m->refcnt, and m->nb_segs in the RX path, please update your drivers"
> - later, if we need more room in the 1st cache line of the mbuf, we
> can move refcnt and nb_seg, probably without impacting the
> performance.
>
>
> Olivier
I suppose you mean that PMDs don't need to /initialize/ m->next, m->refcnt and m->nb_segs.
Forgive my ignorance, and this is wild speculation, but: Would a PMD not need to set m->next and m->nb_segs if it receives a jumbogram larger than an mbuf packet buffer? And if this is a realistic use case, these fields actually do belong in the 1st cache line. PMD developers please chime in.
And I tend to agree with Bruce about making all these mbuf changes in one go, rather than postponing some of them to later. Especially because the postponement also closes and reopens the whole discussion and decision process! (Not initializing a few fields in a PMD cannot require a lot of work by the PMD developers. Moving the fields to the 2nd cache line will in the worst case degrade the performance of the non-updated PMDs.)
A two step process makes good sense for the developers of DPDK, but both steps should be taken within the same release, so they are transparent to the users of DPDK.
Med venlig hilsen / kind regards
- Morten Brørup
next prev parent reply other threads:[~2017-02-21 14:20 UTC|newest]
Thread overview: 155+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-24 15:19 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 [this message]
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
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=98CBD80474FA8B44BF855DF32C47DC359EAD0D@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
--cc=olivier.matz@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).