From: Olivier Matz <olivier.matz@6wind.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: Jan Blunck <jblunck@infradead.org>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC 0/8] mbuf: structure reorganization
Date: Tue, 28 Feb 2017 13:28:24 +0100 [thread overview]
Message-ID: <20170228132825.37586902@platinum> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F11EA96@irsmsx105.ger.corp.intel.com>
On Tue, 28 Feb 2017 11:48:20 +0000, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com> wrote:
> >
> > On Tue, 28 Feb 2017 10:29:41 +0000, "Ananyev, Konstantin"
> > <konstantin.ananyev@intel.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Tue, 28 Feb 2017 09:05:07 +0000, "Ananyev, Konstantin"
> > > > <konstantin.ananyev@intel.com> wrote:
> > > > > Hi everyone,
> > > > >
> > > > > > >
> > > > > > > In my opinion, if we have the room in the first cache
> > > > > > > line, we should put it there. The only argument I see
> > > > > > > against is "we may find something more important in the
> > > > > > > future, and we won't have room for it in the first cache
> > > > > > > line". I don't feel we should penalize today's use cases
> > > > > > > for hypothetic future use cases.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >> 2. timestamp normalization point
> > > > > > >> inside PMD RX vs somewhere later as user needs it
> > > > > > >> (extra function in dev_ops?).
> > > > > > >
> > > > > > > This point could be changed. My initial proposition tries
> > > > > > > to provide a generic API for timestamp. Let me remind it
> > > > > > > here:
> > > > > > >
> > > > > > > a- the timestamp is in nanosecond
> > > > > > > b- the reference is always the same for a given path: if
> > > > > > > the timestamp is set in a PMD, all the packets for this
> > > > > > > PMD will have the same reference, but for 2 different
> > > > > > > PMDs (or a sw lib), the reference would not be the same.
> > > > > > >
> > > > > > > We may remove a-, and just have:
> > > > > > > - the reference and the unit are always the same for a
> > > > > > > given path: if the timestamp is set in a PMD, all the
> > > > > > > packets for this PMD will have the same reference and
> > > > > > > unit, but for 2 different PMDs (or a sw lib), they would
> > > > > > > not be the same.
> > > > > > >
> > > > > > > In both cases, we would need a conversion code (maybe in a
> > > > > > > library) if the application wants to work with timestamps
> > > > > > > from several sources. The second solution removes the
> > > > > > > normalization code in the PMD when not needed, it is
> > > > > > > probably better.
> > > > > >
> > > > > > I agree.
> > > > >
> > > > > One question - does that mean that application would need to
> > > > > keep a track from what PMD each particular packet came to do
> > > > > the normalization? Konstantin
> > > >
> > > > I'd say yes. It does not look very difficult to do, since the
> > > > mbuf contains the input port id.
> > > >
> > >
> > > I understand that we can use mbuf->port here, but it means that
> > > we'll introduce new implicit dependency between timestamp and
> > > port values. From my point that introduces new implications:
> > > 1. all PMDs that do set a timestamp would also have to set port
> > > value too. Probably not a big deal as most of PMDs do set port
> > > value anyway right now, but it means it would be hard to get
> > > rid/change mbuf->port in future.
> >
> > Currently, all PMDs must set m->port.
> > If in the future we remove m->port, the applications that use it
> > will need to store the value in a mbuf metadata, or pass it as
> > arguments through function calls.
> >
> >
> > > 2. Applications would not allowed to change mbuf->port value
> > > before normalization is done (from what I heard some apps do
> > > update mbuf->port to store routing decisions). BTW, how the app
> > > would keep track which mbufs were already normalized, and which
> > > were not?
> >
> > I don't think it should be allowed to change m->port value.
>
> As far as I know it is allowed right now.
> PMD RX routine sets mbuf->port, after that application is free to use
> it in a way it likes.
The descriptor or m->port is "Input port". If the applications stores
something else than the input port, it is its responsibility if it
breaks something else. Like changing any other field to put something
that does not match the description.
> What we are introducing here is basically a new dependency between 2
> mbuf fields and new restriction.
On the other hand, there is no strong dependency: the API to do the
normalization can take the port as a parameter.
>
> Another thing that doesn't look very convenient to me here -
> We can have 2 different values of timestamp (both normalized and not)
> and there is no clear way for the application to know which one is in
> use right now. So each app writer would have to come-up with his own
> solution.
It depends:
- the solution you describe is to have the application storing the
normalized value in its private metadata.
- another solution would be to store the normalized value in
m->timestamp. In this case, we would need a flag to tell if the
timestamp value is normalized.
The problem pointed out by Jan is that doing the timestamp
normalization may take some CPU cycles, even if a small part of packets
requires it.
>
> > Applications that
> > are doing this are responsible of what they change.
> >
> >
> > > 3. In theory with eth_dev_detach() - mbuf->port value might be
> > > not valid at the point when application would decide to do
> > > normalization.
> > >
> > > So to me all that approach with delayed normalization seems
> > > unnecessary overcomplicated. Original one suggested by Olivier,
> > > when normalization is done in PMD at RX look much cleaner and
> > > more manageable.
> >
> > Detaching a device requires a synchronization between control and
> > data plane, and not only for this use case.
>
> Of course it does.
> But right now it is possible to do:
>
> eth_rx_burst(port=0, ..., &mbuf, 1);
> eth_dev_detach(port=0, ...);
> ...
> /*process previously received mbuf */
>
> With what you are proposing it would be not always possible any more.
With your example, it does not work even without the timestamp feature,
since the mbuf input port would reference an invalid port. This port
is usually used in the application to do a lookup for an port structure,
so it is expected that the entry is valid. It would be even worst if you
do a detach + attach.
So, I think it is already the responsibility of the application to do
the sync (flush retrieved packets before detaching a port).
>
> >In the first solution, the normalization is
> > partial: unit is nanosecond, but the time reference is different.
>
> Not sure I get you here...
In the first solution I described, each PMD had to convert its unit
into nanosecond. This is easy because we assume the PMD knows the
value of its clock. But to get a fully normalized value, it also has to
use the same time reference, so we would also need to manage an offset
(we need a new API to give this value to the PMD).
I have another fear related to hardware clocks: if clocks are not
synchronized between PMDs, the simple operation "t * ratio - offset"
won't work. That's why I think we could delegate this job in a specific
library that would manage this.
Having a non-normalized timestamp as of today would allow applications
to take advantage of it for many use cases, even without the
normalization library that could come later (and that may probably
be more complex than expected).
Olivier
next prev parent reply other threads:[~2017-02-28 12:28 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
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 [this message]
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=20170228132825.37586902@platinum \
--to=olivier.matz@6wind.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=jblunck@infradead.org \
--cc=konstantin.ananyev@intel.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).