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: Thu, 2 Mar 2017 17:46:23 +0100 [thread overview]
Message-ID: <20170302174623.268592a7@platinum> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F11EE7A@irsmsx105.ger.corp.intel.com>
Hi Konstantin,
On Tue, 28 Feb 2017 22:53:55 +0000, "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> > > 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.
>
> My first thought also was about second flag to specify was timestamp
> already normalized or not.
> Though I still in doubt - is it all really worth it: extra ol_flag, new function in eth_dev API.
> My feeling that we trying to overcomplicate things.
I don't see what is so complicated. The idea is just to let the
application do the normalization if it is required.
If the time is normalized in nanosecond in the PMD, we would still
need to normalized the time reference (the 0). And for that we'd need
a call to a synchronization code as well.
> > 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.
>
> I understand that point, but from what I've seen with real example:
> http://dpdk.org/ml/archives/dev/2016-October/048810.html
> the amount of calculations at RX is pretty small.
> I don't think it would affect performance in a noticeable way
> (though I don't have any numbers here to prove it).
I think we can consider by default that adding code in the data path
impacts performance.
> From other side, if user doesn't want a timestamp he can always disable
> that feature anad save cycles, right?
>
> BTW, you and Jan both mention that not every packet would need a timestamp.
> Instead we need sort of a timestamp for the group of packets?
I think that for many applications the timestamp should be as precise
as possible for each packet.
> Is that really the only foreseen usage model?
No, but it could be one.
> If so, then why not to have a special function that would extract 'latest' timestamp
> from the dev?
> Or even have tx_burst_extra() that would return a latest timestamp (extra parameter or so).
> Then there is no need to put timestamp into mbuf at all.
Doing that will give a poor precision for the timestamp.
> > > > 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 worse if you
> > do a detach + attach.
>
> I am not talking about the mbuf->port value usage.
> Right now user can access/interpret all metadata fields set by PMD RX routines
> (vlan, rss hash, ol_flags, ptype, etc.) without need to accessing the device data or
> calling device functions.
> With that change it wouldn't be the case anymore.
That's the same for some other functions. If in my application I want
to call eth_rx_queue_count(m->port), I will have the same problem.
I think we also have something quite similar in examples/ptpclient:
rte_eth_rx_burst(portid, 0, &m, 1);
...
parse_ptp_frames(portid, m);
...
ptp_data.portid = portid;
...
rte_eth_timesync_read_tx_timestamp(ptp_data->portid, ...)
So, really, I think it's an application issue: when the app deletes
a port, it should ask itself if there are remaining references to
it (m->port).
> > So, I think it is already the responsibility of the application to do
> > the sync (flush retrieved packets before detaching a port).
>
> The packets are not in RX or TX queue of detaching device any more.
> I received a packet, after that I expect to have all its data and metadata inside mbuf.
> So I can store mbufs somewhere and process them much later.
> Or might be I would like to pass it to the secondary process for logging/analyzing, etc.
Yes, but that's still an app problem for me.
> > > >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).
>
> Yes, I suppose we do need an start timestamp and sort of factor() to convert
> HW value, something like:
>
> mbuf->timestamp = rxq->start_timestamp + factor(hw_timestamp);
>
> Right?
> Why passing start_timestamp at the configure() phase will be a problem?
>
> >
> > 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.
>
> But then that library would need to account all PMDs inside the system,
> and be aware about each HW clock skew, etc.
> Again, doesn't' sound like an simple task to me.
Exactly, that's also why I want to let the specialists take care of
it. Having non-normalized timestamps now allow to do the job later
when required, while allowing basic usages as required by metrics
libraries and mlx pmd.
Olivier
next prev parent reply other threads:[~2017-03-02 16:46 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
2017-02-28 22:53 ` Ananyev, Konstantin
2017-03-02 16:46 ` Olivier Matz [this message]
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=20170302174623.268592a7@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).