* Re: mbuf changes
From: Richardson, Bruce @ 2016-10-28 17:00 UTC (permalink / raw)
To: Adrien Mazarguil, Morten Brørup; +Cc: dev@dpdk.org
In-Reply-To: <20161028165005.GY5733@6wind.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Friday, October 28, 2016 5:50 PM
> To: Morten Brørup <mb@smartsharesystems.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] mbuf changes
>
> On Fri, Oct 28, 2016 at 04:11:45PM +0200, Morten Brørup wrote:
> > Comments at the end.
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pattan, Reshma
> > > Sent: Friday, October 28, 2016 3:35 PM
> > > To: Olivier Matz
> > > Cc: dev@dpdk.org; Morten Brørup
> > > Subject: Re: [dpdk-dev] mbuf changes
> > >
> > > Hi Olivier,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > > Sent: Tuesday, October 25, 2016 1:49 PM
> > > > To: Richardson, Bruce <bruce.richardson@intel.com>; Morten Brørup
> > > > <mb@smartsharesystems.com>
> > > > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Wiles, Keith
> > > > <keith.wiles@intel.com>; dev@dpdk.org; Oleg Kuporosov
> > > > <olegk@mellanox.com>
> > > > Subject: Re: [dpdk-dev] mbuf changes
> > > >
> > > >
> > > >
> > > > On 10/25/2016 02:45 PM, Bruce Richardson wrote:
> > > > > On Tue, Oct 25, 2016 at 02:33:55PM +0200, Morten Brørup wrote:
> > > > >> Comments at the end.
> > > > >>
> > > > >> Med venlig hilsen / kind regards
> > > > >> - Morten Brørup
> > > > >>
> > > > >>> -----Original Message-----
> > > > >>> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > > >>> Sent: Tuesday, October 25, 2016 2:20 PM
> > > > >>> To: Morten Brørup
> > > > >>> Cc: Adrien Mazarguil; Wiles, Keith; dev@dpdk.org; Olivier
> > > > >>> Matz; Oleg Kuporosov
> > > > >>> Subject: Re: [dpdk-dev] mbuf changes
> > > > >>>
> > > > >>> On Tue, Oct 25, 2016 at 02:16:29PM +0200, Morten Brørup wrote:
> > > > >>>> Comments inline.
> > > > >>>>
> > > > >>>>> -----Original Message-----
> > > > >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> > > > >>>>> Richardson
> > > > >>>>> Sent: Tuesday, October 25, 2016 1:14 PM
> > > > >>>>> To: Adrien Mazarguil
> > > > >>>>> Cc: Morten Brørup; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > > > >>>>> Oleg Kuporosov
> > > > >>>>> Subject: Re: [dpdk-dev] mbuf changes
> > > > >>>>>
> > > > >>>>> On Tue, Oct 25, 2016 at 01:04:44PM +0200, Adrien Mazarguil
> > > wrote:
> > > > >>>>>> On Tue, Oct 25, 2016 at 12:11:04PM +0200, Morten Brørup
> wrote:
> > > > >>>>>>> Comments inline.
> > > > >>>>>>>
> > > > >>>>>>> Med venlig hilsen / kind regards
> > > > >>>>>>> - Morten Brørup
> > > > >>>>>>>
> > > > >>>>>>>
> > > > >>>>>>>> -----Original Message-----
> > > > >>>>>>>> From: Adrien Mazarguil
> > > > >>>>>>>> [mailto:adrien.mazarguil@6wind.com]
> > > > >>>>>>>> Sent: Tuesday, October 25, 2016 11:39 AM
> > > > >>>>>>>> To: Bruce Richardson
> > > > >>>>>>>> Cc: Wiles, Keith; Morten Brørup; dev@dpdk.org; Olivier
> > > > >>>>>>>> Matz; Oleg Kuporosov
> > > > >>>>>>>> Subject: Re: [dpdk-dev] mbuf changes
> > > > >>>>>>>>
> > > > >>>>>>>> On Mon, Oct 24, 2016 at 05:25:38PM +0100, Bruce
> > > > >>>>>>>> Richardson
> > > > >>> wrote:
> > > > >>>>>>>>> On Mon, Oct 24, 2016 at 04:11:33PM +0000, Wiles, Keith
> > > > >>> wrote:
> > > > >>>>>>>> [...]
> > > > >>>>>>>>>>> On Oct 24, 2016, at 10:49 AM, Morten Brørup
> > > > >>>>>>>> <mb@smartsharesystems.com> wrote:
> > > > >>>>>>>> [...]
> > > > >>>>
> > > > >>>>>>>>> One other point I'll mention is that we need to have a
> > > > >>>>>>>>> discussion on how/where to add in a timestamp value into
> > > > >>> the
> > > > >>>>>>>>> mbuf. Personally, I think it can be in a union with the
> > > > >>>>> sequence
> > > > >>>>>>>>> number value, but I also suspect that 32-bits of a
> > > > >>> timestamp
> > > > >>>>>>>>> is not going to be enough for
> > > > >>>>>>>> many.
> > > > >>>>>>>>>
> > > > >>>>>>>>> Thoughts?
> > > > >>>>>>>>
> > > > >>>>>>>> If we consider that timestamp representation should use
> > > > >>>>> nanosecond
> > > > >>>>>>>> granularity, a 32-bit value may likely wrap around too
> > > > >>> quickly
> > > > >>>>>>>> to be useful. We can also assume that applications
> > > requesting
> > > > >>>>>>>> timestamps may care more about latency than throughput,
> > > > >>>>>>>> Oleg
> > > > >>>>> found
> > > > >>>>>>>> that using the second cache line for this purpose had a
> > > > >>>>> noticeable impact [1].
> > > > >>>>>>>>
> > > > >>>>>>>> [1] http://dpdk.org/ml/archives/dev/2016-
> > > October/049237.html
> > > > >>>>>>>
> > > > >>>>>>> I agree with Oleg about the latency vs. throughput
> > > > >>>>>>> importance for
> > > > >>>>> such applications.
> > > > >>>>>>>
> > > > >>>>>>> If you need high resolution timestamps, consider them to
> > > > >>>>>>> be
> > > > >>>>> generated by the NIC RX driver, possibly by the hardware
> > > > >>>>> itself
> > > > >>>>> (http://w3new.napatech.com/features/time-precision/hardware-
> > > time
> > > > >>>>> - stamp), so the timestamp belongs in the first cache line.
> > > > >>>>> And I am proposing that it should have the highest possible
> > > > >>>>> accuracy, which makes the value hardware dependent.
> > > > >>>>>>>
> > > > >>>>>>> Furthermore, I am arguing that we leave it up to the
> > > > >>> application
> > > > >>>>>>> to
> > > > >>>>> keep track of the slowly moving bits (i.e. counting whole
> > > > >>>>> seconds, hours and calendar date) out of band, so we don't
> > > > >>>>> use precious
> > > > >>> space
> > > > >>>>> in the mbuf. The application doesn't need the NIC RX
> > > > >>>>> driver's fast path to capture which date (or even which
> > > > >>>>> second) a packet was received. Yes, it adds complexity to
> > > > >>>>> the application, but
> > > we
> > > > >>>>> can't set aside 64 bit for a generic timestamp. Or as a
> > > > >>>>> weird
> > > tradeoff:
> > > > >>>>> Put the fast moving 32 bit in the first cache line and the
> > > > >>>>> slow moving 32 bit in the second cache line, as a
> > > > >>>>> placeholder for
> > > the
> > > > >>> application to fill out if needed.
> > > > >>>>> Yes, it means that the application needs to check the time
> > > > >>>>> and update its variable holding the slow moving time once
> > > > >>>>> every second or so; but that should be doable without
> > > > >>>>> significant
> > > effort.
> > > > >>>>>>
> > > > >>>>>> That's a good point, however without a 64 bit value,
> > > > >>>>>> elapsed time between two arbitrary mbufs cannot be measured
> > > > >>>>>> reliably due to
> > > > >>> not
> > > > >>>>>> enough context, one way or another the low resolution value
> > > > >>>>>> is also
> > > > >>>>> needed.
> > > > >>>>>>
> > > > >>>>>> Obviously latency-sensitive applications are unlikely to
> > > > >>>>>> perform lengthy buffering and require this but I'm not sure
> > > > >>>>>> about all the possible use-cases. Considering many NICs
> > > > >>>>>> expose
> > > > >>>>>> 64 bit
> > > > >>> timestaps,
> > > > >>>>>> I suggest we do not truncate them.
> > > > >>>>>>
> > > > >>>>>> I'm not a fan of the weird tradeoff either, PMDs will be
> > > > >>>>>> tempted to fill the extra 32 bits whenever they can and
> > > > >>>>>> negate the performance improvement of the first cache line.
> > > > >>>>>
> > > > >>>>> I would tend to agree, and I don't really see any convenient
> > > way
> > > > >>>>> to avoid putting in a 64-bit field for the timestamp in
> > > > >>>>> cache-
> > > line 0.
> > > > >>>>> If we are ok with having this overlap/partially overlap with
> > > > >>>>> sequence number, it will use up an extra 4B of storage in
> > > > >>>>> that
> > > > >>> cacheline.
> > > > >>>>
> > > > >>>> I agree about the lack of convenience! And Adrien certainly
> > > > >>>> has
> > > a
> > > > >>> point about PMD temptations.
> > > > >>>>
> > > > >>>> However, I still don't think that a NICs ability to
> > > > >>>> date-stamp a
> > > > >>> packet is sufficient reason to put a date-stamp in cache line
> > > > >>> 0
> > > of
> > > > >>> the mbuf. Storing only the fast moving 32 bit in cache line 0
> > > > >>> seems like a good compromise to me.
> > > > >>>>
> > > > >>>> Maybe you can find just one more byte, so it can hold 17
> > > > >>>> minutes with nanosecond resolution. (I'm joking!)
> > > > >>>>
> > > > >>>> Please don't sacrifice the sequence number for the
> > > > >>>> seconds/hours/days
> > > > >>> part a timestamp. Maybe it could be configurable to use a 32
> > > > >>> bit or
> > > > >>> 64 bit timestamp.
> > > > >>>>
> > > > >>> Do you see both timestamp and sequence numbers being used
> > > together?
> > > > >>> I would have thought that apps would either use one or the
> other?
> > > > >>> However, your suggestion is workable in any case, to allow the
> > > > >>> sequence number to overlap just the high 32 bits of the
> > > timestamp,
> > > > >>> rather than the low.
> > > > >>
> > > > >> In our case, I can foresee sequence numbers used for packet
> > > > >> processing and
> > > > timestamps for timing analysis (and possibly for packet capturing,
> > > > when being used). For timing analysis, we don’t need long
> > > > durations, e.g. 4 seconds with 32 bit nanosecond resolution
> > > > suffices. And for packet capturing we are perfectly capable of
> > > > adding the slowly moving
> > > > 32 bit of the timestamp to our output data stream without fetching
> > > > it
> > > from the mbuf.
> > > > >>
> > > >
> > > > We should keep in mind that today we have the seqn field but it is
> > > not
> > > > used by any PMD. In case it is implemented, would it be a
> > > > per-queue
> > > sequence number?
> > > > Is it useful from an application view?
> > > >
> > > > This field is only used by the librte_reorder library, and in my
> > > > opinion, we should consider moving it in the second cache line
> > > > since
> > > it is not filled by the PMD.
> > > >
> > > >
> > > > > For the 32-bit timestamp case, it might be useful to have a
> > > > > right-shift value passed in to the ethdev driver. If we assume a
> > > NIC
> > > > > with nanosecond resolution, (or TSC value with resolution of
> > > > > that order of magnitude), then the app can choose to have 1 ns
> > > resolution
> > > > > with 4 second wraparound, or alternatively 4ns resolution with
> > > > > 16 second wraparound, or even microsecond resolution with wrap
> > > > > around of over
> > > > an hour.
> > > > > The cost is obviously just a shift op in the driver code per
> > > > > packet
> > > > > - hopefully with multiple packets done at a time using vector
> > > operations.
> > > >
> > > >
> > > > About the timestamp, we can manage to find 64 bits in the first
> > > > cache line, without sacrifying any field we have today. The
> > > > question is
> > > more
> > > > for the fields we may want to add later.
> > > >
> > > > To answer to the question of the size of the timestamp, the first
> > > > question is to know what is the precision required for the
> > > applications using it?
> > >
> > > As part of the 17.02 latency calculation feature, the requirement is
> > > to report latency in nanoseconds. So, +1 on keeping timestamp as
> 64bits.
> > > Since the feature is planned for 17.02, can we finalize on the
> > > timestamp position in the mbuf struct?
> > > Based on the decision, I am planning to make the change and send ABI
> > > notice if needed.
> > >
> > > Reshma
> > >
> > > >
> > > > I don't quite like the idea of splitting the timestamp in the 2
> > > > cache lines, I think it would not be easy to use.
> > > >
> > > >
> > > > Olivier
> >
> > Nanosecond precision in latency calculations does not require more than
> 32 bit, unless we must also be able to measure more than 4 seconds of
> latency. And please consider how many packets we would need to hold in
> memory to keep track of 4 seconds of traffic on a 100 Gbit/s link (if we
> are measuring network latency): With an average packet size of 300 B that
> would be 40 million packets.
>
> Consider another, perhaps more realistic use-case: keeping a few relevant
> packets from a stream for later analysis, which could occur outside of the
> normal TX processing path for performance reasons. In this case 4 seconds
> worth of context may not be enough to avoid losing information.
>
> > If I recall correctly, the 17.02 feature is about measuring application
> latency, not network latency (as my calculation above is about). I don't
> consider application latency measuring a common feature for most
> applications, except for application debugging/profiling purposes (not
> network debugging purposes). So do we really need both nanosecond
> precision and the ability to measure 4+ seconds of latency?
>
> Existing HW can already provide 64-bit precision, why truncate part of it?
> The only valid reason would be that there is not enough room in the mbuf
> header, which is not the case. Looks like today there is even enough room
> in the first 64 bytes.
>
> As suggested by Bruce, we can even union (part of) this field with another
> if necessary, as long as we make sure both cannot be requested at the same
> time (determining which field should be sacrificed is going to be tricky
> though).
>
> > Furthermore, if the timestamp isn't set by hardware, why not just take
> the entire array of packets pulled out from the NIC and set their
> timestamp to the same value, instead of setting them one by one in the PMD
> - that would also consolidate the expense of obtaining the time from some
> clock source.
>
> Like any offload, some PMD processing is expected to convert the possibly
> HW-specific value to a common mbuf format, however if hardware does not
> report any kind of information relevant to timestamp individual packets, I
> do not think the PMD should expose that feature and implement it in
> software. It would be much worse, certainly not what the application
> wanted as it could have done the same on its own.
>
> What matters is to know when NIC receives a packet, not when the
> application fetches it. You should assume there is a significant delay
> between these two events, hence the need for it to be offloaded.
>
I would actually see both use cases being used. In some cases, NICs will
have hardware offload capability for timestamping which can be used, while
in many cases, timestamping on retrieval by the core will suffice for
latency measurements within an app. I don't think we can discount either
possibility. Thankfully, AFAIK which model is used doesn't really affect
whether we need 32 or 64 bits for the data.
/Bruce
^ permalink raw reply
* Re: mbuf changes
From: Adrien Mazarguil @ 2016-10-28 16:50 UTC (permalink / raw)
To: Morten Brørup; +Cc: dev
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC359EA8DB@smartserver.smartshare.dk>
On Fri, Oct 28, 2016 at 04:11:45PM +0200, Morten Brørup wrote:
> Comments at the end.
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pattan, Reshma
> > Sent: Friday, October 28, 2016 3:35 PM
> > To: Olivier Matz
> > Cc: dev@dpdk.org; Morten Brørup
> > Subject: Re: [dpdk-dev] mbuf changes
> >
> > Hi Olivier,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > Sent: Tuesday, October 25, 2016 1:49 PM
> > > To: Richardson, Bruce <bruce.richardson@intel.com>; Morten Brørup
> > > <mb@smartsharesystems.com>
> > > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Wiles, Keith
> > > <keith.wiles@intel.com>; dev@dpdk.org; Oleg Kuporosov
> > > <olegk@mellanox.com>
> > > Subject: Re: [dpdk-dev] mbuf changes
> > >
> > >
> > >
> > > On 10/25/2016 02:45 PM, Bruce Richardson wrote:
> > > > On Tue, Oct 25, 2016 at 02:33:55PM +0200, Morten Brørup wrote:
> > > >> Comments at the end.
> > > >>
> > > >> Med venlig hilsen / kind regards
> > > >> - Morten Brørup
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > >>> Sent: Tuesday, October 25, 2016 2:20 PM
> > > >>> To: Morten Brørup
> > > >>> Cc: Adrien Mazarguil; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > > >>> Oleg Kuporosov
> > > >>> Subject: Re: [dpdk-dev] mbuf changes
> > > >>>
> > > >>> On Tue, Oct 25, 2016 at 02:16:29PM +0200, Morten Brørup wrote:
> > > >>>> Comments inline.
> > > >>>>
> > > >>>>> -----Original Message-----
> > > >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> > > >>>>> Richardson
> > > >>>>> Sent: Tuesday, October 25, 2016 1:14 PM
> > > >>>>> To: Adrien Mazarguil
> > > >>>>> Cc: Morten Brørup; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > > >>>>> Oleg Kuporosov
> > > >>>>> Subject: Re: [dpdk-dev] mbuf changes
> > > >>>>>
> > > >>>>> On Tue, Oct 25, 2016 at 01:04:44PM +0200, Adrien Mazarguil
> > wrote:
> > > >>>>>> On Tue, Oct 25, 2016 at 12:11:04PM +0200, Morten Brørup wrote:
> > > >>>>>>> Comments inline.
> > > >>>>>>>
> > > >>>>>>> Med venlig hilsen / kind regards
> > > >>>>>>> - Morten Brørup
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>>> -----Original Message-----
> > > >>>>>>>> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > >>>>>>>> Sent: Tuesday, October 25, 2016 11:39 AM
> > > >>>>>>>> To: Bruce Richardson
> > > >>>>>>>> Cc: Wiles, Keith; Morten Brørup; dev@dpdk.org; Olivier Matz;
> > > >>>>>>>> Oleg Kuporosov
> > > >>>>>>>> Subject: Re: [dpdk-dev] mbuf changes
> > > >>>>>>>>
> > > >>>>>>>> On Mon, Oct 24, 2016 at 05:25:38PM +0100, Bruce Richardson
> > > >>> wrote:
> > > >>>>>>>>> On Mon, Oct 24, 2016 at 04:11:33PM +0000, Wiles, Keith
> > > >>> wrote:
> > > >>>>>>>> [...]
> > > >>>>>>>>>>> On Oct 24, 2016, at 10:49 AM, Morten Brørup
> > > >>>>>>>> <mb@smartsharesystems.com> wrote:
> > > >>>>>>>> [...]
> > > >>>>
> > > >>>>>>>>> One other point I'll mention is that we need to have a
> > > >>>>>>>>> discussion on how/where to add in a timestamp value into
> > > >>> the
> > > >>>>>>>>> mbuf. Personally, I think it can be in a union with the
> > > >>>>> sequence
> > > >>>>>>>>> number value, but I also suspect that 32-bits of a
> > > >>> timestamp
> > > >>>>>>>>> is not going to be enough for
> > > >>>>>>>> many.
> > > >>>>>>>>>
> > > >>>>>>>>> Thoughts?
> > > >>>>>>>>
> > > >>>>>>>> If we consider that timestamp representation should use
> > > >>>>> nanosecond
> > > >>>>>>>> granularity, a 32-bit value may likely wrap around too
> > > >>> quickly
> > > >>>>>>>> to be useful. We can also assume that applications
> > requesting
> > > >>>>>>>> timestamps may care more about latency than throughput, Oleg
> > > >>>>> found
> > > >>>>>>>> that using the second cache line for this purpose had a
> > > >>>>> noticeable impact [1].
> > > >>>>>>>>
> > > >>>>>>>> [1] http://dpdk.org/ml/archives/dev/2016-
> > October/049237.html
> > > >>>>>>>
> > > >>>>>>> I agree with Oleg about the latency vs. throughput importance
> > > >>>>>>> for
> > > >>>>> such applications.
> > > >>>>>>>
> > > >>>>>>> If you need high resolution timestamps, consider them to be
> > > >>>>> generated by the NIC RX driver, possibly by the hardware itself
> > > >>>>> (http://w3new.napatech.com/features/time-precision/hardware-
> > time
> > > >>>>> - stamp), so the timestamp belongs in the first cache line. And
> > > >>>>> I am proposing that it should have the highest possible
> > > >>>>> accuracy, which makes the value hardware dependent.
> > > >>>>>>>
> > > >>>>>>> Furthermore, I am arguing that we leave it up to the
> > > >>> application
> > > >>>>>>> to
> > > >>>>> keep track of the slowly moving bits (i.e. counting whole
> > > >>>>> seconds, hours and calendar date) out of band, so we don't use
> > > >>>>> precious
> > > >>> space
> > > >>>>> in the mbuf. The application doesn't need the NIC RX driver's
> > > >>>>> fast path to capture which date (or even which second) a packet
> > > >>>>> was received. Yes, it adds complexity to the application, but
> > we
> > > >>>>> can't set aside 64 bit for a generic timestamp. Or as a weird
> > tradeoff:
> > > >>>>> Put the fast moving 32 bit in the first cache line and the slow
> > > >>>>> moving 32 bit in the second cache line, as a placeholder for
> > the
> > > >>> application to fill out if needed.
> > > >>>>> Yes, it means that the application needs to check the time and
> > > >>>>> update its variable holding the slow moving time once every
> > > >>>>> second or so; but that should be doable without significant
> > effort.
> > > >>>>>>
> > > >>>>>> That's a good point, however without a 64 bit value, elapsed
> > > >>>>>> time between two arbitrary mbufs cannot be measured reliably
> > > >>>>>> due to
> > > >>> not
> > > >>>>>> enough context, one way or another the low resolution value is
> > > >>>>>> also
> > > >>>>> needed.
> > > >>>>>>
> > > >>>>>> Obviously latency-sensitive applications are unlikely to
> > > >>>>>> perform lengthy buffering and require this but I'm not sure
> > > >>>>>> about all the possible use-cases. Considering many NICs expose
> > > >>>>>> 64 bit
> > > >>> timestaps,
> > > >>>>>> I suggest we do not truncate them.
> > > >>>>>>
> > > >>>>>> I'm not a fan of the weird tradeoff either, PMDs will be
> > > >>>>>> tempted to fill the extra 32 bits whenever they can and negate
> > > >>>>>> the performance improvement of the first cache line.
> > > >>>>>
> > > >>>>> I would tend to agree, and I don't really see any convenient
> > way
> > > >>>>> to avoid putting in a 64-bit field for the timestamp in cache-
> > line 0.
> > > >>>>> If we are ok with having this overlap/partially overlap with
> > > >>>>> sequence number, it will use up an extra 4B of storage in that
> > > >>> cacheline.
> > > >>>>
> > > >>>> I agree about the lack of convenience! And Adrien certainly has
> > a
> > > >>> point about PMD temptations.
> > > >>>>
> > > >>>> However, I still don't think that a NICs ability to date-stamp a
> > > >>> packet is sufficient reason to put a date-stamp in cache line 0
> > of
> > > >>> the mbuf. Storing only the fast moving 32 bit in cache line 0
> > > >>> seems like a good compromise to me.
> > > >>>>
> > > >>>> Maybe you can find just one more byte, so it can hold 17 minutes
> > > >>>> with nanosecond resolution. (I'm joking!)
> > > >>>>
> > > >>>> Please don't sacrifice the sequence number for the
> > > >>>> seconds/hours/days
> > > >>> part a timestamp. Maybe it could be configurable to use a 32 bit
> > > >>> or
> > > >>> 64 bit timestamp.
> > > >>>>
> > > >>> Do you see both timestamp and sequence numbers being used
> > together?
> > > >>> I would have thought that apps would either use one or the other?
> > > >>> However, your suggestion is workable in any case, to allow the
> > > >>> sequence number to overlap just the high 32 bits of the
> > timestamp,
> > > >>> rather than the low.
> > > >>
> > > >> In our case, I can foresee sequence numbers used for packet
> > > >> processing and
> > > timestamps for timing analysis (and possibly for packet capturing,
> > > when being used). For timing analysis, we don’t need long durations,
> > > e.g. 4 seconds with 32 bit nanosecond resolution suffices. And for
> > > packet capturing we are perfectly capable of adding the slowly moving
> > > 32 bit of the timestamp to our output data stream without fetching it
> > from the mbuf.
> > > >>
> > >
> > > We should keep in mind that today we have the seqn field but it is
> > not
> > > used by any PMD. In case it is implemented, would it be a per-queue
> > sequence number?
> > > Is it useful from an application view?
> > >
> > > This field is only used by the librte_reorder library, and in my
> > > opinion, we should consider moving it in the second cache line since
> > it is not filled by the PMD.
> > >
> > >
> > > > For the 32-bit timestamp case, it might be useful to have a
> > > > right-shift value passed in to the ethdev driver. If we assume a
> > NIC
> > > > with nanosecond resolution, (or TSC value with resolution of that
> > > > order of magnitude), then the app can choose to have 1 ns
> > resolution
> > > > with 4 second wraparound, or alternatively 4ns resolution with 16
> > > > second wraparound, or even microsecond resolution with wrap around
> > > > of over
> > > an hour.
> > > > The cost is obviously just a shift op in the driver code per packet
> > > > - hopefully with multiple packets done at a time using vector
> > operations.
> > >
> > >
> > > About the timestamp, we can manage to find 64 bits in the first cache
> > > line, without sacrifying any field we have today. The question is
> > more
> > > for the fields we may want to add later.
> > >
> > > To answer to the question of the size of the timestamp, the first
> > > question is to know what is the precision required for the
> > applications using it?
> >
> > As part of the 17.02 latency calculation feature, the requirement is to
> > report latency in nanoseconds. So, +1 on keeping timestamp as 64bits.
> > Since the feature is planned for 17.02, can we finalize on the
> > timestamp position in the mbuf struct?
> > Based on the decision, I am planning to make the change and send ABI
> > notice if needed.
> >
> > Reshma
> >
> > >
> > > I don't quite like the idea of splitting the timestamp in the 2 cache
> > > lines, I think it would not be easy to use.
> > >
> > >
> > > Olivier
>
> Nanosecond precision in latency calculations does not require more than 32 bit, unless we must also be able to measure more than 4 seconds of latency. And please consider how many packets we would need to hold in memory to keep track of 4 seconds of traffic on a 100 Gbit/s link (if we are measuring network latency): With an average packet size of 300 B that would be 40 million packets.
Consider another, perhaps more realistic use-case: keeping a few relevant
packets from a stream for later analysis, which could occur outside of the
normal TX processing path for performance reasons. In this case 4 seconds
worth of context may not be enough to avoid losing information.
> If I recall correctly, the 17.02 feature is about measuring application latency, not network latency (as my calculation above is about). I don't consider application latency measuring a common feature for most applications, except for application debugging/profiling purposes (not network debugging purposes). So do we really need both nanosecond precision and the ability to measure 4+ seconds of latency?
Existing HW can already provide 64-bit precision, why truncate part of it?
The only valid reason would be that there is not enough room in the mbuf
header, which is not the case. Looks like today there is even enough room in
the first 64 bytes.
As suggested by Bruce, we can even union (part of) this field with another
if necessary, as long as we make sure both cannot be requested at the same
time (determining which field should be sacrificed is going to be tricky
though).
> Furthermore, if the timestamp isn't set by hardware, why not just take the entire array of packets pulled out from the NIC and set their timestamp to the same value, instead of setting them one by one in the PMD - that would also consolidate the expense of obtaining the time from some clock source.
Like any offload, some PMD processing is expected to convert the possibly
HW-specific value to a common mbuf format, however if hardware does not
report any kind of information relevant to timestamp individual packets, I
do not think the PMD should expose that feature and implement it in
software. It would be much worse, certainly not what the application
wanted as it could have done the same on its own.
What matters is to know when NIC receives a packet, not when the application
fetches it. You should assume there is a significant delay between these two
events, hence the need for it to be offloaded.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Stephen Hemminger @ 2016-10-28 16:25 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: DPDK
In-Reply-To: <8c7f9d25-b042-6b7e-b197-7873ea7425ef@intel.com>
On Fri, 28 Oct 2016 15:31:50 +0100
Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> Discussed alternatives were:
> * Tun/Tap
> This won't be as fast as KNI and performance is an issue.
That is a myth. Both require the some number of copies.
TUN/TAP copies is a syscall and KNI copies is a kthread.
Actually, the KNI method is worse because it has kernel thread
always running chewing a CPU. I.e it is pure poll mode.
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Thomas Monjalon @ 2016-10-28 16:13 UTC (permalink / raw)
To: Richardson, Bruce; +Cc: dev, Yigit, Ferruh
In-Reply-To: <59AF69C657FD0841A61C55336867B5B035B31F7F@IRSMSX103.ger.corp.intel.com>
2016-10-28 15:51, Richardson, Bruce:
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > 2016-10-28 15:31, Ferruh Yigit:
> > > * virtio-user + vhost-net
> > > This can be valid alternative, removes the out of tree kernel module
> > > need. But missing control path. Proof of concept work will be done.
> >
> > That's probably a smart alternative for packet injection.
> > What do you mean exactly by "missing control path"?
>
> We'll have to see how it performs - which is the key gap for data path that KNI fills. Until we get an alternative with (nearly) equivalent performance, there will be demand for KNI to stick around.
> The "control path" is the ethtool part, to get stats and do operations on the NIC using command-line tools.
>
> >
> > > * Remove ethtool support ?
> >
> > That's the other part of KNI.
> > It works only for e1000/ixgbe. That's a niche.
>
> Yes, it's something we need to remove, but again, we need an alternative first.
>
> >
> > > Still there is some interest, will keep it. But not able to extend it
> > > to other drivers with current design.
> >
> > It should be removed one day.
> > We must seriously think about a generic alternative.
> > Either we add DPDK support in ethtool or we create a dpdk-ethtool.
> > (or at least a library as the one in examples/).
>
> I don't view that as a great path forward. Sure, we can do our own ethtool, but then people will look for ifconfig to work, and "ip" to work, etc. I view having a kernel proxy module as the best path here as it is tool agnostic on the userspace side, rather than trying to make every tool for working with kernel netdevs also have support for dpdk ports.
Yes that's the ultimately best solution.
But:
- we need some cooperation of the kernel team
- ethtool manages a device (what DPDK provides) whereas iproute and others
manage a TCP/IP stack so is out of control of DPDK.
> > Or we do nothing and wait to have more hardware like Mellanox supporting a
> > kernel bifurcated driver approach.
>
> Given the lack of other NICs supporting that, I think it could be quite a wait! Also, it doesn't work for virtio ports, for pcap ports, or any other ports which don't have physical hardware backing them. No reason you shouldn't be able to pull stats from all your dpdk ethdevs, not just the ones with physical hardware. The same ethdev APIs work for them, so should the same tools.
Yes, very good point.
> > > *KNI PMD
> > > Patch is in the mail list, missing comments. If it gets some
> > > interest/comments/acks it may go in to next release.
> >
> > I'm not against KNI PMD but it looks strange to add more support to an old
> > dying approach.
>
> I think the main idea here is to clean up the API - at least for the data path. There is no reason why we need special KNI RX/TX functions, when ethdev RX/TX functions could do the job. However, at a higher level, the more basic requirement is that whatever solution for the data-path to kernel from dpdk is, it needs to appear as an ethdev, and not as a special library with different APIs, as KNI is now.
Yes I agree to unifiy (and reduce) API.
Why this PMD is not more commented?
KNI users should be interested to review it.
Please dear community, we need more reviews!
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Richardson, Bruce @ 2016-10-28 15:51 UTC (permalink / raw)
To: Thomas Monjalon, Yigit, Ferruh; +Cc: dev@dpdk.org
In-Reply-To: <16238883.tkNBRrfWjK@xps13>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Friday, October 28, 2016 4:13 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] KNI discussion in userspace event
>
> 2016-10-28 15:31, Ferruh Yigit:
> > * virtio-user + vhost-net
> > This can be valid alternative, removes the out of tree kernel module
> > need. But missing control path. Proof of concept work will be done.
>
> That's probably a smart alternative for packet injection.
> What do you mean exactly by "missing control path"?
We'll have to see how it performs - which is the key gap for data path that KNI fills. Until we get an alternative with (nearly) equivalent performance, there will be demand for KNI to stick around.
The "control path" is the ethtool part, to get stats and do operations on the NIC using command-line tools.
>
> > * Remove ethtool support ?
>
> That's the other part of KNI.
> It works only for e1000/ixgbe. That's a niche.
Yes, it's something we need to remove, but again, we need an alternative first.
>
> > Still there is some interest, will keep it. But not able to extend it
> > to other drivers with current design.
>
> It should be removed one day.
> We must seriously think about a generic alternative.
> Either we add DPDK support in ethtool or we create a dpdk-ethtool.
> (or at least a library as the one in examples/).
I don't view that as a great path forward. Sure, we can do our own ethtool, but then people will look for ifconfig to work, and "ip" to work, etc. I view having a kernel proxy module as the best path here as it is tool agnostic on the userspace side, rather than trying to make every tool for working with kernel netdevs also have support for dpdk ports.
> Or we do nothing and wait to have more hardware like Mellanox supporting a
> kernel bifurcated driver approach.
Given the lack of other NICs supporting that, I think it could be quite a wait! Also, it doesn't work for virtio ports, for pcap ports, or any other ports which don't have physical hardware backing them. No reason you shouldn't be able to pull stats from all your dpdk ethdevs, not just the ones with physical hardware. The same ethdev APIs work for them, so should the same tools.
>
> > *KNI PMD
> > Patch is in the mail list, missing comments. If it gets some
> > interest/comments/acks it may go in to next release.
>
> I'm not against KNI PMD but it looks strange to add more support to an old
> dying approach.
I think the main idea here is to clean up the API - at least for the data path. There is no reason why we need special KNI RX/TX functions, when ethdev RX/TX functions could do the job. However, at a higher level, the more basic requirement is that whatever solution for the data-path to kernel from dpdk is, it needs to appear as an ethdev, and not as a special library with different APIs, as KNI is now.
/Bruce
^ permalink raw reply
* Re: mbuf changes
From: Pattan, Reshma @ 2016-10-28 15:25 UTC (permalink / raw)
To: Morten Brørup, Olivier Matz; +Cc: dev@dpdk.org
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC359EA8DB@smartserver.smartshare.dk>
> -----Original Message-----
> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> Sent: Friday, October 28, 2016 3:12 PM
> To: Pattan, Reshma <reshma.pattan@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] mbuf changes
>
> Comments at the end.
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pattan, Reshma
> > Sent: Friday, October 28, 2016 3:35 PM
> > To: Olivier Matz
> > Cc: dev@dpdk.org; Morten Brørup
> > Subject: Re: [dpdk-dev] mbuf changes
> >
> > Hi Olivier,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > > Sent: Tuesday, October 25, 2016 1:49 PM
> > > To: Richardson, Bruce <bruce.richardson@intel.com>; Morten Brørup
> > > <mb@smartsharesystems.com>
> > > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Wiles, Keith
> > > <keith.wiles@intel.com>; dev@dpdk.org; Oleg Kuporosov
> > > <olegk@mellanox.com>
> > > Subject: Re: [dpdk-dev] mbuf changes
> > >
> > >
> > >
> > > On 10/25/2016 02:45 PM, Bruce Richardson wrote:
> > > > On Tue, Oct 25, 2016 at 02:33:55PM +0200, Morten Brørup wrote:
> > > >> Comments at the end.
> > > >>
> > > >> Med venlig hilsen / kind regards
> > > >> - Morten Brørup
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > >>> Sent: Tuesday, October 25, 2016 2:20 PM
> > > >>> To: Morten Brørup
> > > >>> Cc: Adrien Mazarguil; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > > >>> Oleg Kuporosov
> > > >>> Subject: Re: [dpdk-dev] mbuf changes
> > > >>>
> > > >>> On Tue, Oct 25, 2016 at 02:16:29PM +0200, Morten Brørup wrote:
> > > >>>> Comments inline.
> > > >>>>
> > > >>>>> -----Original Message-----
> > > >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> > > >>>>> Richardson
> > > >>>>> Sent: Tuesday, October 25, 2016 1:14 PM
> > > >>>>> To: Adrien Mazarguil
> > > >>>>> Cc: Morten Brørup; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > > >>>>> Oleg Kuporosov
> > > >>>>> Subject: Re: [dpdk-dev] mbuf changes
> > > >>>>>
> > > >>>>> On Tue, Oct 25, 2016 at 01:04:44PM +0200, Adrien Mazarguil
> > wrote:
> > > >>>>>> On Tue, Oct 25, 2016 at 12:11:04PM +0200, Morten Brørup wrote:
> > > >>>>>>> Comments inline.
> > > >>>>>>>
> > > >>>>>>> Med venlig hilsen / kind regards
> > > >>>>>>> - Morten Brørup
> > > >>>>>>>
> > > >>>>>>>
> > > >>>>>>>> -----Original Message-----
> > > >>>>>>>> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > >>>>>>>> Sent: Tuesday, October 25, 2016 11:39 AM
> > > >>>>>>>> To: Bruce Richardson
> > > >>>>>>>> Cc: Wiles, Keith; Morten Brørup; dev@dpdk.org; Olivier
> > > >>>>>>>> Matz; Oleg Kuporosov
> > > >>>>>>>> Subject: Re: [dpdk-dev] mbuf changes
> > > >>>>>>>>
> > > >>>>>>>> On Mon, Oct 24, 2016 at 05:25:38PM +0100, Bruce Richardson
> > > >>> wrote:
> > > >>>>>>>>> On Mon, Oct 24, 2016 at 04:11:33PM +0000, Wiles, Keith
> > > >>> wrote:
> > > >>>>>>>> [...]
> > > >>>>>>>>>>> On Oct 24, 2016, at 10:49 AM, Morten Brørup
> > > >>>>>>>> <mb@smartsharesystems.com> wrote:
> > > >>>>>>>> [...]
> > > >>>>
> > > >>>>>>>>> One other point I'll mention is that we need to have a
> > > >>>>>>>>> discussion on how/where to add in a timestamp value into
> > > >>> the
> > > >>>>>>>>> mbuf. Personally, I think it can be in a union with the
> > > >>>>> sequence
> > > >>>>>>>>> number value, but I also suspect that 32-bits of a
> > > >>> timestamp
> > > >>>>>>>>> is not going to be enough for
> > > >>>>>>>> many.
> > > >>>>>>>>>
> > > >>>>>>>>> Thoughts?
> > > >>>>>>>>
> > > >>>>>>>> If we consider that timestamp representation should use
> > > >>>>> nanosecond
> > > >>>>>>>> granularity, a 32-bit value may likely wrap around too
> > > >>> quickly
> > > >>>>>>>> to be useful. We can also assume that applications
> > requesting
> > > >>>>>>>> timestamps may care more about latency than throughput,
> > > >>>>>>>> Oleg
> > > >>>>> found
> > > >>>>>>>> that using the second cache line for this purpose had a
> > > >>>>> noticeable impact [1].
> > > >>>>>>>>
> > > >>>>>>>> [1] http://dpdk.org/ml/archives/dev/2016-
> > October/049237.html
> > > >>>>>>>
> > > >>>>>>> I agree with Oleg about the latency vs. throughput
> > > >>>>>>> importance for
> > > >>>>> such applications.
> > > >>>>>>>
> > > >>>>>>> If you need high resolution timestamps, consider them to be
> > > >>>>> generated by the NIC RX driver, possibly by the hardware
> > > >>>>> itself
> > > >>>>> (http://w3new.napatech.com/features/time-precision/hardware-
> > time
> > > >>>>> - stamp), so the timestamp belongs in the first cache line.
> > > >>>>> And I am proposing that it should have the highest possible
> > > >>>>> accuracy, which makes the value hardware dependent.
> > > >>>>>>>
> > > >>>>>>> Furthermore, I am arguing that we leave it up to the
> > > >>> application
> > > >>>>>>> to
> > > >>>>> keep track of the slowly moving bits (i.e. counting whole
> > > >>>>> seconds, hours and calendar date) out of band, so we don't use
> > > >>>>> precious
> > > >>> space
> > > >>>>> in the mbuf. The application doesn't need the NIC RX driver's
> > > >>>>> fast path to capture which date (or even which second) a
> > > >>>>> packet was received. Yes, it adds complexity to the
> > > >>>>> application, but
> > we
> > > >>>>> can't set aside 64 bit for a generic timestamp. Or as a weird
> > tradeoff:
> > > >>>>> Put the fast moving 32 bit in the first cache line and the
> > > >>>>> slow moving 32 bit in the second cache line, as a placeholder
> > > >>>>> for
> > the
> > > >>> application to fill out if needed.
> > > >>>>> Yes, it means that the application needs to check the time and
> > > >>>>> update its variable holding the slow moving time once every
> > > >>>>> second or so; but that should be doable without significant
> > effort.
> > > >>>>>>
> > > >>>>>> That's a good point, however without a 64 bit value, elapsed
> > > >>>>>> time between two arbitrary mbufs cannot be measured reliably
> > > >>>>>> due to
> > > >>> not
> > > >>>>>> enough context, one way or another the low resolution value
> > > >>>>>> is also
> > > >>>>> needed.
> > > >>>>>>
> > > >>>>>> Obviously latency-sensitive applications are unlikely to
> > > >>>>>> perform lengthy buffering and require this but I'm not sure
> > > >>>>>> about all the possible use-cases. Considering many NICs
> > > >>>>>> expose
> > > >>>>>> 64 bit
> > > >>> timestaps,
> > > >>>>>> I suggest we do not truncate them.
> > > >>>>>>
> > > >>>>>> I'm not a fan of the weird tradeoff either, PMDs will be
> > > >>>>>> tempted to fill the extra 32 bits whenever they can and
> > > >>>>>> negate the performance improvement of the first cache line.
> > > >>>>>
> > > >>>>> I would tend to agree, and I don't really see any convenient
> > way
> > > >>>>> to avoid putting in a 64-bit field for the timestamp in cache-
> > line 0.
> > > >>>>> If we are ok with having this overlap/partially overlap with
> > > >>>>> sequence number, it will use up an extra 4B of storage in that
> > > >>> cacheline.
> > > >>>>
> > > >>>> I agree about the lack of convenience! And Adrien certainly has
> > a
> > > >>> point about PMD temptations.
> > > >>>>
> > > >>>> However, I still don't think that a NICs ability to date-stamp
> > > >>>> a
> > > >>> packet is sufficient reason to put a date-stamp in cache line 0
> > of
> > > >>> the mbuf. Storing only the fast moving 32 bit in cache line 0
> > > >>> seems like a good compromise to me.
> > > >>>>
> > > >>>> Maybe you can find just one more byte, so it can hold 17
> > > >>>> minutes with nanosecond resolution. (I'm joking!)
> > > >>>>
> > > >>>> Please don't sacrifice the sequence number for the
> > > >>>> seconds/hours/days
> > > >>> part a timestamp. Maybe it could be configurable to use a 32 bit
> > > >>> or
> > > >>> 64 bit timestamp.
> > > >>>>
> > > >>> Do you see both timestamp and sequence numbers being used
> > together?
> > > >>> I would have thought that apps would either use one or the other?
> > > >>> However, your suggestion is workable in any case, to allow the
> > > >>> sequence number to overlap just the high 32 bits of the
> > timestamp,
> > > >>> rather than the low.
> > > >>
> > > >> In our case, I can foresee sequence numbers used for packet
> > > >> processing and
> > > timestamps for timing analysis (and possibly for packet capturing,
> > > when being used). For timing analysis, we don’t need long durations,
> > > e.g. 4 seconds with 32 bit nanosecond resolution suffices. And for
> > > packet capturing we are perfectly capable of adding the slowly
> > > moving
> > > 32 bit of the timestamp to our output data stream without fetching
> > > it
> > from the mbuf.
> > > >>
> > >
> > > We should keep in mind that today we have the seqn field but it is
> > not
> > > used by any PMD. In case it is implemented, would it be a per-queue
> > sequence number?
> > > Is it useful from an application view?
> > >
> > > This field is only used by the librte_reorder library, and in my
> > > opinion, we should consider moving it in the second cache line since
> > it is not filled by the PMD.
> > >
> > >
> > > > For the 32-bit timestamp case, it might be useful to have a
> > > > right-shift value passed in to the ethdev driver. If we assume a
> > NIC
> > > > with nanosecond resolution, (or TSC value with resolution of that
> > > > order of magnitude), then the app can choose to have 1 ns
> > resolution
> > > > with 4 second wraparound, or alternatively 4ns resolution with 16
> > > > second wraparound, or even microsecond resolution with wrap around
> > > > of over
> > > an hour.
> > > > The cost is obviously just a shift op in the driver code per
> > > > packet
> > > > - hopefully with multiple packets done at a time using vector
> > operations.
> > >
> > >
> > > About the timestamp, we can manage to find 64 bits in the first
> > > cache line, without sacrifying any field we have today. The question
> > > is
> > more
> > > for the fields we may want to add later.
> > >
> > > To answer to the question of the size of the timestamp, the first
> > > question is to know what is the precision required for the
> > applications using it?
> >
> > As part of the 17.02 latency calculation feature, the requirement is
> > to report latency in nanoseconds. So, +1 on keeping timestamp as 64bits.
> > Since the feature is planned for 17.02, can we finalize on the
> > timestamp position in the mbuf struct?
> > Based on the decision, I am planning to make the change and send ABI
> > notice if needed.
> >
> > Reshma
> >
> > >
> > > I don't quite like the idea of splitting the timestamp in the 2
> > > cache lines, I think it would not be easy to use.
> > >
> > >
> > > Olivier
>
> Nanosecond precision in latency calculations does not require more than 32 bit,
> unless we must also be able to measure more than 4 seconds of latency. And
> please consider how many packets we would need to hold in memory to keep
> track of 4 seconds of traffic on a 100 Gbit/s link (if we are measuring network
> latency): With an average packet size of 300 B that would be 40 million packets.
>
Ok, I got the point, For 17.02 feature, latency is not measured for a particular duration of time, instead
average latency is measured on the fly for every time stamped packet that is observed on the Tx and
on Rx side packets are marked with timestamps for every sampling intervals. I should be able to say
my opinion on the size again after having internal discussion.
Reshma
> If I recall correctly, the 17.02 feature is about measuring application latency, not
> network latency (as my calculation above is about). I don't consider application
> latency measuring a common feature for most applications, except for
> application debugging/profiling purposes (not network debugging purposes). So
> do we really need both nanosecond precision and the ability to measure 4+
> seconds of latency?
>
> Furthermore, if the timestamp isn't set by hardware, why not just take the entire
> array of packets pulled out from the NIC and set their timestamp to the same
> value, instead of setting them one by one in the PMD - that would also
> consolidate the expense of obtaining the time from some clock source.
>
> -Morten
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Thomas Monjalon @ 2016-10-28 15:13 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev
In-Reply-To: <8c7f9d25-b042-6b7e-b197-7873ea7425ef@intel.com>
2016-10-28 15:31, Ferruh Yigit:
> * virtio-user + vhost-net
> This can be valid alternative, removes the out of tree kernel module
> need. But missing control path. Proof of concept work will be done.
That's probably a smart alternative for packet injection.
What do you mean exactly by "missing control path"?
> * Remove ethtool support ?
That's the other part of KNI.
It works only for e1000/ixgbe. That's a niche.
> Still there is some interest, will keep it. But not able to extend it to
> other drivers with current design.
It should be removed one day.
We must seriously think about a generic alternative.
Either we add DPDK support in ethtool or we create a dpdk-ethtool.
(or at least a library as the one in examples/).
Or we do nothing and wait to have more hardware like Mellanox supporting
a kernel bifurcated driver approach.
> *KNI PMD
> Patch is in the mail list, missing comments. If it gets some
> interest/comments/acks it may go in to next release.
I'm not against KNI PMD but it looks strange to add more support
to an old dying approach.
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Igor Ryzhov @ 2016-10-28 15:03 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: DPDK
In-Reply-To: <8c7f9d25-b042-6b7e-b197-7873ea7425ef@intel.com>
Thank you, Ferruh.
As we are staying on the existing implementation, I think we can do some
improvements:
1. Implement more commands for net_device_ops.
2. Implement ethtool support the same way as net_device_ops are implemented
– send commands to application.
3. Add ability to set default MAC address for KNI interface. Now it is
random for all interfaces except those that work on igb or ixgbe.
4. Properly implement link state control feature. Now KNI interface is in
UNKNOWN state even after changing carrier flag to 1.
First two improvements are already done in KCP patches and can be easily
ported into the current code.
For the last two improvements I can send patches.
Best regards,
Igor
^ permalink raw reply
* Re: Solarflare PMD submission question
From: Andrew Rybchenko @ 2016-10-28 14:43 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
In-Reply-To: <1838661.tWJoShD1UF@xps13>
On 10/28/2016 03:33 PM, Thomas Monjalon wrote:
> 2016-10-28 13:50, Andrew Rybchenko:
>> The only thing which comes to my mind is to split libefx import on subsystem
>> basis (few files per subsystem). It is artificial and added files will
>> be abandoned
>> until the patch which adds them into build. It could be something like:
>> 1. External interfaces definition
>> 2. Internal interfaces definition
>> 3. Registers definition (hardware interface)
>> 4. Management CPU interface definition (it is one file, but still big
>> 650K)
>> 5. Management CPU interface implementation
>> and so on for NIC global controls, interrupts, event queue, transmit,
>> receive,
>> filtering etc.
> Yes it is artificial.
> The most valuable would be a transversal logical split, kind of feature
> per feature, in order to explain how the device works.
I'm not the main author of the libefx and personally would consider it
very useful.
From the other hand I understand that it is a huge amount of work to
make it.
> Such commit is also the opportunity to explain acronyms and so on.
Good. We'll go this way and 'll do my best to make it useful to understand
overall structure of the code and how the device works.
>>> It would be also really appreciated to provide a design documentation
>>> in doc/guides/nics. Are the datasheets open? A link in the doc would help.
>> We have a documentation which grows together with supported features,
>> but it is rather for users. Important design decisions (not so many) are
>> documented nearby corresponding code. Unfortunately there is no open
>> datasheets. Management CPU interface definition has comments.
> Without neither a datasheet, nor a comprehensive code introduction, it is
> almost impossible to dive in your code. So it misses the point about bringing
> it to an Open Source project.
> Please do the the effort to bring some knowledge to the community.
I've raised this internally and see what extra documentation we can
provide to
the community. But this may take some time and I hope it is OK to post
patches
in the interim. I use the management CPU interface (MCDI) definition
mentioned
above when I add features. It is shared by all drivers: [1], [2], [3].
[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/sfc/mcdi_pcol.h
[2]
https://svnweb.freebsd.org/base/head/sys/dev/sfxge/common/efx_regs_mcdi.h?view=markup
[3]
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/io/sfxge/common/efx_regs_mcdi.h
Andrew.
^ permalink raw reply
* [PATCH v7] app/testpmd: fix DCB configuration
From: Bernard Iremonger @ 2016-10-28 14:40 UTC (permalink / raw)
To: dev, rahul.r.shah, jingjing.wu, wenzhuo.lu
In-Reply-To: <1477414269-27156-3-git-send-email-bernard.iremonger@intel.com>
Data Centre Bridge (DCB) configuration fails when SRIOV is
enabled if nb_rxq and nb_txq are not set to 1.
When dcb_mode is DCB_VT_ENABLED and max_vfs is greater than
zero, set nb_rxq and nb_txq to 1.
The failure occurs during configuration of the ixgbe PMD when
it is started, in the ixgbe_check_mq_mode function, if nb_rxq
and nb_txq are not set to 1.
Fixes: 2a977b891f99 ("app/testpmd: fix DCB configuration")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Changes in v7:
restore nb_rxq and nb_txq setting when max_vfs is 0.
---
app/test-pmd/testpmd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6185be6..96f5011 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2002,8 +2002,13 @@ init_port_dcb_config(portid_t pid,
* and has the same number of rxq and txq in dcb mode
*/
if (dcb_mode == DCB_VT_ENABLED) {
- nb_rxq = rte_port->dev_info.max_rx_queues;
- nb_txq = rte_port->dev_info.max_tx_queues;
+ if (rte_port->dev_info.max_vfs > 0) {
+ nb_rxq = 1;
+ nb_txq = 1;
+ } else {
+ nb_rxq = rte_port->dev_info.max_rx_queues;
+ nb_txq = rte_port->dev_info.max_tx_queues;
+ }
} else {
/*if vt is disabled, use all pf queues */
if (rte_port->dev_info.vmdq_pool_base == 0) {
--
2.4.3
^ permalink raw reply related
* KNI discussion in userspace event
From: Ferruh Yigit @ 2016-10-28 14:31 UTC (permalink / raw)
To: DPDK
Hi,
There was an "Interworking with the Linux Kernel" discussion in the DPDK
userspace event, this mail is to summarize the output and to get more
comments from community.
Briefly, KNI mostly will stay as it is as an interworking with the Linux
kernel solution.
Out of tree kernel module concern is still there, but there is no clear
alternative to switch. And community still care about performance of KNI
and control path of KNI. Only KNI VHOST may go away. KNI PMD depends on
community interest. There was no modification request on KNI library and
sample app.
Discussed alternatives were:
* Tun/Tap
This won't be as fast as KNI and performance is an issue.
* virtio-user + vhost-net
This can be valid alternative, removes the out of tree kernel module
need. But missing control path. Proof of concept work will be done.
* Bifurcated driver
Not able to filter all traffic, not a full functional alternative.
* Upstreaming kernel module:
Stephen suggested upstreaming a generic shim layer and use it.
Future of the KNI:
* Remove ethtool support ?
Still there is some interest, will keep it. But not able to extend it to
other drivers with current design.
* Remove KNI VHOST?
There were no interest for this feature. I will send a deprecation
notice to remove this, and we can discuss more there.
* What to do with out of tree kernel module
It is still problem for OSVs and unfortunately it is staying.
* Switch completely to an alternative approach?
There won't be an action for a switch. virtio-user + vhost-net
alternative will be investigated.
*KNI PMD
Patch is in the mail list, missing comments. If it gets some
interest/comments/acks it may go in to next release.
* Any improvement on library or sample app?
Nothing listed.
Thanks,
ferruh
^ permalink raw reply
* Re: [RFC] [PATCH v2] libeventdev: event driven programming model framework for DPDK
From: Bruce Richardson @ 2016-10-28 14:16 UTC (permalink / raw)
To: Van Haaren, Harry
Cc: Vangati, Narender, dev@dpdk.org, Eads, Gage,
thomas.monjalon@6wind.com
In-Reply-To: <E923DB57A917B54B9182A2E928D00FA6129AE01B@IRSMSX102.ger.corp.intel.com>
On Fri, Oct 28, 2016 at 02:48:57PM +0100, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> > Sent: Tuesday, October 25, 2016 6:49 PM
> <snip>
> >
> > Hi Community,
> >
> > So far, I have received constructive feedback from Intel, NXP and Linaro folks.
> > Let me know, if anyone else interested in contributing to the definition of eventdev?
> >
> > If there are no major issues in proposed spec, then Cavium would like work on
> > implementing and up-streaming the common code(lib/librte_eventdev/) and
> > an associated HW driver.(Requested minor changes of v2 will be addressed
> > in next version).
>
>
> Hi All,
>
> I've been looking at the eventdev API from a use-case point of view, and I'm unclear on a how the API caters for two uses. I have simplified these as much as possible, think of them as a theoretical unit-test for the API :)
>
>
> Fragmentation:
> 1. Dequeue 8 packets
> 2. Process 2 packets
> 3. Processing 3rd, this packet needs fragmentation into two packets
> 4. Process remaining 5 packets as normal
>
> What function calls does the application make to achieve this?
> In particular, I'm referring to how can the scheduler know that the 3rd packet is the one being fragmented, and how to keep packet order valid.
>
>
> Dropping packets:
> 1. Dequeue 8 packets
> 2. Process 2 packets
> 3. Processing 3rd, this packet needs to be dropped
> 4. Process remaining 5 packets as normal
>
> What function calls does the application make to achieve this?
> Again, in particular how does the scheduler know that the 3rd packet is being dropped.
>
>
> Regards, -Harry
Hi,
these questions apply particularly to reordered which has a lot more
complications than the other types in terms of sending packets back into
the scheduler. However, atomic types will still suffer from problems
with things the way they are - again if we assume a burst of 8 packets,
then to forward those packets, we need to re-enqueue them again to the
scheduler, and also then send 8 releases to the scheduler as well, to
release the atomic locks for those packets.
This means that for each packet we have to send two messages to a
scheduler core, something that is really inefficient.
This number of messages is critical for any software implementation, as
the cost of moving items core-to-core is going to be a big bottleneck
(perhaps the biggest bottleneck) in the system. It's for this reason we
need to use burst APIs - as with rte_rings.
How we have solved this in our implementation, is to allow there to be
an event operation type. The four operations we implemented are as below
(using packet as a synonym for event here, since these would mostly
apply to packets flowing through a system):
* NEW - just a regular enqueue of a packet, without any previous context
* FORWARD - enqueue a packet, and mark the flow processing for the
equivalent packet that was dequeued as completed, i.e.
release any atomic locks, or reorder this packet with
respect to any other outstanding packets from the event queue.
* DROP - this is roughtly equivalent to the existing "release" API call,
except that having it as an enqueue type allows us to
release multiple items in a single call, and also to mix
releases with new packets and forwarded packets
* PARTIAL - this indicates that the packet being enqueued should be
treated according to the context of the current packet, but
that that context should not be released/completed by the
enqueue of this packet. This only really applies for
reordered events, and is needed to do fragmentation and or
multicast of packets with reordering.
Therefore, I think we need to use some of the bits just freed up in the
event structure to include an enqueue operation type. Without it, I just
can't see how the API can ever support burst operation on packets.
Regards,
/Bruce
^ permalink raw reply
* Re: Tcpdump
From: Pattan, Reshma @ 2016-10-28 14:12 UTC (permalink / raw)
To: Dror Birkman; +Cc: dev@dpdk.org
In-Reply-To: <CAMLkaUs7KENDryDZV1VZwNH+q8ZACkE32bX1Vm4O0xTCPvmNRg@mail.gmail.com>
Hi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Dror Birkman
> Sent: Thursday, October 27, 2016 1:25 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] Tcpdump
>
> Hi,
>
> I have a DPDK application that binds to an interface and processes packets.
> For debugging purposes I want to run tcpdump on this interface.
DPDK provides dpdk-pdump tool for capturing the packets to pcap file, which can be passed to tcpdump like tool to view the packets.
Please refer below DPDK documentation on the same. If you have further questions on the usage, please do send mail on to users@dpdk.org.
It does effect the performance so it is recommended to use it for debugging purposes.
http://dpdk.org/doc/guides/prog_guide/pdump_lib.html
http://dpdk.org/doc/guides/sample_app_ug/pdump.html
Thanks,
Reshma
>
> IYO, what is my best option with hurting the performance of the application too
> much?
>
> TIA,
> Dror
^ permalink raw reply
* Re: mbuf changes
From: Morten Brørup @ 2016-10-28 14:11 UTC (permalink / raw)
To: Pattan, Reshma, Olivier Matz; +Cc: dev
In-Reply-To: <3AEA2BF9852C6F48A459DA490692831F010BF015@IRSMSX109.ger.corp.intel.com>
Comments at the end.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pattan, Reshma
> Sent: Friday, October 28, 2016 3:35 PM
> To: Olivier Matz
> Cc: dev@dpdk.org; Morten Brørup
> Subject: Re: [dpdk-dev] mbuf changes
>
> Hi Olivier,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > Sent: Tuesday, October 25, 2016 1:49 PM
> > To: Richardson, Bruce <bruce.richardson@intel.com>; Morten Brørup
> > <mb@smartsharesystems.com>
> > Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Wiles, Keith
> > <keith.wiles@intel.com>; dev@dpdk.org; Oleg Kuporosov
> > <olegk@mellanox.com>
> > Subject: Re: [dpdk-dev] mbuf changes
> >
> >
> >
> > On 10/25/2016 02:45 PM, Bruce Richardson wrote:
> > > On Tue, Oct 25, 2016 at 02:33:55PM +0200, Morten Brørup wrote:
> > >> Comments at the end.
> > >>
> > >> Med venlig hilsen / kind regards
> > >> - Morten Brørup
> > >>
> > >>> -----Original Message-----
> > >>> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > >>> Sent: Tuesday, October 25, 2016 2:20 PM
> > >>> To: Morten Brørup
> > >>> Cc: Adrien Mazarguil; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > >>> Oleg Kuporosov
> > >>> Subject: Re: [dpdk-dev] mbuf changes
> > >>>
> > >>> On Tue, Oct 25, 2016 at 02:16:29PM +0200, Morten Brørup wrote:
> > >>>> Comments inline.
> > >>>>
> > >>>>> -----Original Message-----
> > >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> > >>>>> Richardson
> > >>>>> Sent: Tuesday, October 25, 2016 1:14 PM
> > >>>>> To: Adrien Mazarguil
> > >>>>> Cc: Morten Brørup; Wiles, Keith; dev@dpdk.org; Olivier Matz;
> > >>>>> Oleg Kuporosov
> > >>>>> Subject: Re: [dpdk-dev] mbuf changes
> > >>>>>
> > >>>>> On Tue, Oct 25, 2016 at 01:04:44PM +0200, Adrien Mazarguil
> wrote:
> > >>>>>> On Tue, Oct 25, 2016 at 12:11:04PM +0200, Morten Brørup wrote:
> > >>>>>>> Comments inline.
> > >>>>>>>
> > >>>>>>> Med venlig hilsen / kind regards
> > >>>>>>> - Morten Brørup
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>> -----Original Message-----
> > >>>>>>>> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > >>>>>>>> Sent: Tuesday, October 25, 2016 11:39 AM
> > >>>>>>>> To: Bruce Richardson
> > >>>>>>>> Cc: Wiles, Keith; Morten Brørup; dev@dpdk.org; Olivier Matz;
> > >>>>>>>> Oleg Kuporosov
> > >>>>>>>> Subject: Re: [dpdk-dev] mbuf changes
> > >>>>>>>>
> > >>>>>>>> On Mon, Oct 24, 2016 at 05:25:38PM +0100, Bruce Richardson
> > >>> wrote:
> > >>>>>>>>> On Mon, Oct 24, 2016 at 04:11:33PM +0000, Wiles, Keith
> > >>> wrote:
> > >>>>>>>> [...]
> > >>>>>>>>>>> On Oct 24, 2016, at 10:49 AM, Morten Brørup
> > >>>>>>>> <mb@smartsharesystems.com> wrote:
> > >>>>>>>> [...]
> > >>>>
> > >>>>>>>>> One other point I'll mention is that we need to have a
> > >>>>>>>>> discussion on how/where to add in a timestamp value into
> > >>> the
> > >>>>>>>>> mbuf. Personally, I think it can be in a union with the
> > >>>>> sequence
> > >>>>>>>>> number value, but I also suspect that 32-bits of a
> > >>> timestamp
> > >>>>>>>>> is not going to be enough for
> > >>>>>>>> many.
> > >>>>>>>>>
> > >>>>>>>>> Thoughts?
> > >>>>>>>>
> > >>>>>>>> If we consider that timestamp representation should use
> > >>>>> nanosecond
> > >>>>>>>> granularity, a 32-bit value may likely wrap around too
> > >>> quickly
> > >>>>>>>> to be useful. We can also assume that applications
> requesting
> > >>>>>>>> timestamps may care more about latency than throughput, Oleg
> > >>>>> found
> > >>>>>>>> that using the second cache line for this purpose had a
> > >>>>> noticeable impact [1].
> > >>>>>>>>
> > >>>>>>>> [1] http://dpdk.org/ml/archives/dev/2016-
> October/049237.html
> > >>>>>>>
> > >>>>>>> I agree with Oleg about the latency vs. throughput importance
> > >>>>>>> for
> > >>>>> such applications.
> > >>>>>>>
> > >>>>>>> If you need high resolution timestamps, consider them to be
> > >>>>> generated by the NIC RX driver, possibly by the hardware itself
> > >>>>> (http://w3new.napatech.com/features/time-precision/hardware-
> time
> > >>>>> - stamp), so the timestamp belongs in the first cache line. And
> > >>>>> I am proposing that it should have the highest possible
> > >>>>> accuracy, which makes the value hardware dependent.
> > >>>>>>>
> > >>>>>>> Furthermore, I am arguing that we leave it up to the
> > >>> application
> > >>>>>>> to
> > >>>>> keep track of the slowly moving bits (i.e. counting whole
> > >>>>> seconds, hours and calendar date) out of band, so we don't use
> > >>>>> precious
> > >>> space
> > >>>>> in the mbuf. The application doesn't need the NIC RX driver's
> > >>>>> fast path to capture which date (or even which second) a packet
> > >>>>> was received. Yes, it adds complexity to the application, but
> we
> > >>>>> can't set aside 64 bit for a generic timestamp. Or as a weird
> tradeoff:
> > >>>>> Put the fast moving 32 bit in the first cache line and the slow
> > >>>>> moving 32 bit in the second cache line, as a placeholder for
> the
> > >>> application to fill out if needed.
> > >>>>> Yes, it means that the application needs to check the time and
> > >>>>> update its variable holding the slow moving time once every
> > >>>>> second or so; but that should be doable without significant
> effort.
> > >>>>>>
> > >>>>>> That's a good point, however without a 64 bit value, elapsed
> > >>>>>> time between two arbitrary mbufs cannot be measured reliably
> > >>>>>> due to
> > >>> not
> > >>>>>> enough context, one way or another the low resolution value is
> > >>>>>> also
> > >>>>> needed.
> > >>>>>>
> > >>>>>> Obviously latency-sensitive applications are unlikely to
> > >>>>>> perform lengthy buffering and require this but I'm not sure
> > >>>>>> about all the possible use-cases. Considering many NICs expose
> > >>>>>> 64 bit
> > >>> timestaps,
> > >>>>>> I suggest we do not truncate them.
> > >>>>>>
> > >>>>>> I'm not a fan of the weird tradeoff either, PMDs will be
> > >>>>>> tempted to fill the extra 32 bits whenever they can and negate
> > >>>>>> the performance improvement of the first cache line.
> > >>>>>
> > >>>>> I would tend to agree, and I don't really see any convenient
> way
> > >>>>> to avoid putting in a 64-bit field for the timestamp in cache-
> line 0.
> > >>>>> If we are ok with having this overlap/partially overlap with
> > >>>>> sequence number, it will use up an extra 4B of storage in that
> > >>> cacheline.
> > >>>>
> > >>>> I agree about the lack of convenience! And Adrien certainly has
> a
> > >>> point about PMD temptations.
> > >>>>
> > >>>> However, I still don't think that a NICs ability to date-stamp a
> > >>> packet is sufficient reason to put a date-stamp in cache line 0
> of
> > >>> the mbuf. Storing only the fast moving 32 bit in cache line 0
> > >>> seems like a good compromise to me.
> > >>>>
> > >>>> Maybe you can find just one more byte, so it can hold 17 minutes
> > >>>> with nanosecond resolution. (I'm joking!)
> > >>>>
> > >>>> Please don't sacrifice the sequence number for the
> > >>>> seconds/hours/days
> > >>> part a timestamp. Maybe it could be configurable to use a 32 bit
> > >>> or
> > >>> 64 bit timestamp.
> > >>>>
> > >>> Do you see both timestamp and sequence numbers being used
> together?
> > >>> I would have thought that apps would either use one or the other?
> > >>> However, your suggestion is workable in any case, to allow the
> > >>> sequence number to overlap just the high 32 bits of the
> timestamp,
> > >>> rather than the low.
> > >>
> > >> In our case, I can foresee sequence numbers used for packet
> > >> processing and
> > timestamps for timing analysis (and possibly for packet capturing,
> > when being used). For timing analysis, we don’t need long durations,
> > e.g. 4 seconds with 32 bit nanosecond resolution suffices. And for
> > packet capturing we are perfectly capable of adding the slowly moving
> > 32 bit of the timestamp to our output data stream without fetching it
> from the mbuf.
> > >>
> >
> > We should keep in mind that today we have the seqn field but it is
> not
> > used by any PMD. In case it is implemented, would it be a per-queue
> sequence number?
> > Is it useful from an application view?
> >
> > This field is only used by the librte_reorder library, and in my
> > opinion, we should consider moving it in the second cache line since
> it is not filled by the PMD.
> >
> >
> > > For the 32-bit timestamp case, it might be useful to have a
> > > right-shift value passed in to the ethdev driver. If we assume a
> NIC
> > > with nanosecond resolution, (or TSC value with resolution of that
> > > order of magnitude), then the app can choose to have 1 ns
> resolution
> > > with 4 second wraparound, or alternatively 4ns resolution with 16
> > > second wraparound, or even microsecond resolution with wrap around
> > > of over
> > an hour.
> > > The cost is obviously just a shift op in the driver code per packet
> > > - hopefully with multiple packets done at a time using vector
> operations.
> >
> >
> > About the timestamp, we can manage to find 64 bits in the first cache
> > line, without sacrifying any field we have today. The question is
> more
> > for the fields we may want to add later.
> >
> > To answer to the question of the size of the timestamp, the first
> > question is to know what is the precision required for the
> applications using it?
>
> As part of the 17.02 latency calculation feature, the requirement is to
> report latency in nanoseconds. So, +1 on keeping timestamp as 64bits.
> Since the feature is planned for 17.02, can we finalize on the
> timestamp position in the mbuf struct?
> Based on the decision, I am planning to make the change and send ABI
> notice if needed.
>
> Reshma
>
> >
> > I don't quite like the idea of splitting the timestamp in the 2 cache
> > lines, I think it would not be easy to use.
> >
> >
> > Olivier
Nanosecond precision in latency calculations does not require more than 32 bit, unless we must also be able to measure more than 4 seconds of latency. And please consider how many packets we would need to hold in memory to keep track of 4 seconds of traffic on a 100 Gbit/s link (if we are measuring network latency): With an average packet size of 300 B that would be 40 million packets.
If I recall correctly, the 17.02 feature is about measuring application latency, not network latency (as my calculation above is about). I don't consider application latency measuring a common feature for most applications, except for application debugging/profiling purposes (not network debugging purposes). So do we really need both nanosecond precision and the ability to measure 4+ seconds of latency?
Furthermore, if the timestamp isn't set by hardware, why not just take the entire array of packets pulled out from the NIC and set their timestamp to the same value, instead of setting them one by one in the PMD - that would also consolidate the expense of obtaining the time from some clock source.
-Morten
^ permalink raw reply
* Re: [PATCH] app/test: fix wrong pointer values in crypto perftest
From: Thomas Monjalon @ 2016-10-28 14:01 UTC (permalink / raw)
To: Trahe, Fiona; +Cc: dev
In-Reply-To: <348A99DA5F5B7549AA880327E580B4358911659C@IRSMSX101.ger.corp.intel.com>
2016-10-28 13:42, Trahe, Fiona:
> From: Kusztal, ArkadiuszX
> > This commit fixes problem with device hanging because of wrong pointer
> > values in snow3g performance test
>
> Can you add to resolved issues section of release notes please.
I'm not sure we should comment about unit test fixes in the release notes.
^ permalink raw reply
* Re: [RFC] [PATCH v2] libeventdev: event driven programming model framework for DPDK
From: Van Haaren, Harry @ 2016-10-28 13:48 UTC (permalink / raw)
To: Jerin Jacob
Cc: Vangati, Narender, dev@dpdk.org, Eads, Gage,
thomas.monjalon@6wind.com
In-Reply-To: <20161025174904.GA18333@localhost.localdomain>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> Sent: Tuesday, October 25, 2016 6:49 PM
<snip>
>
> Hi Community,
>
> So far, I have received constructive feedback from Intel, NXP and Linaro folks.
> Let me know, if anyone else interested in contributing to the definition of eventdev?
>
> If there are no major issues in proposed spec, then Cavium would like work on
> implementing and up-streaming the common code(lib/librte_eventdev/) and
> an associated HW driver.(Requested minor changes of v2 will be addressed
> in next version).
Hi All,
I've been looking at the eventdev API from a use-case point of view, and I'm unclear on a how the API caters for two uses. I have simplified these as much as possible, think of them as a theoretical unit-test for the API :)
Fragmentation:
1. Dequeue 8 packets
2. Process 2 packets
3. Processing 3rd, this packet needs fragmentation into two packets
4. Process remaining 5 packets as normal
What function calls does the application make to achieve this?
In particular, I'm referring to how can the scheduler know that the 3rd packet is the one being fragmented, and how to keep packet order valid.
Dropping packets:
1. Dequeue 8 packets
2. Process 2 packets
3. Processing 3rd, this packet needs to be dropped
4. Process remaining 5 packets as normal
What function calls does the application make to achieve this?
Again, in particular how does the scheduler know that the 3rd packet is being dropped.
Regards, -Harry
^ permalink raw reply
* Re: [PATCH v11 1/6] ethdev: add Tx preparation
From: Thomas Monjalon @ 2016-10-28 13:42 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0CED77@irsmsx105.ger.corp.intel.com>
2016-10-28 12:59, Ananyev, Konstantin:
> > 2016-10-28 11:34, Ananyev, Konstantin:
> > > > > 2016-10-27 16:24, Ananyev, Konstantin:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > 2016-10-27 15:52, Ananyev, Konstantin:
> > > > > > > > > 2016-10-26 14:56, Tomasz Kulasek:
> > > > > > > > > > --- a/config/common_base
> > > > > > > > > > +++ b/config/common_base
> > > > > > > > > > +CONFIG_RTE_ETHDEV_TX_PREP=y
> > > > > > > > >
> > > > > > > > > We cannot enable it until it is implemented in every drivers.
> > > > > > > >
> > > > > > > > Not sure why?
> > > > > > > > If tx_pkt_prep == NULL, then rte_eth_tx_prep() would just act as noop.
> > > > > > > > Right now it is not mandatory for the PMD to implement it.
> > > > > > >
> > > > > > > If it is not implemented, the application must do the preparation by itself.
> > > > > > > From patch 6:
> > > > > > > "
> > > > > > > Removed pseudo header calculation for udp/tcp/tso packets from
> > > > > > > application and used Tx preparation API for packet preparation and
> > > > > > > verification.
> > > > > > > "
> > > > > > > So how does it behave with other drivers?
> > > > > >
> > > > > > Hmm so it seems that we broke testpmd csumonly mode for non-intel drivers..
> > > > > > My bad, missed that part completely.
> > > > > > Yes, then I suppose for now we'll need to support both (with and without) code paths for testpmd.
> > > > > > Probably a new fwd mode or just extra parameter for the existing one?
> > > > > > Any other suggestions?
> > > > >
> > > > > Please think how we can use it in every applications.
> > > > > It is not ready.
> > > > > Either we introduce the API without enabling it, or we implement it
> > > > > in every drivers.
> > > >
> > > > I understand your position here, but just like to point that:
> > > > 1) It is a new functionality optional to use.
> > > > The app is free not to use that functionality and still do the preparation itself
> > > > (as it has to do it now).
> > > > All existing apps would keep working as expected without using that function.
> > > > Though if the app developer knows that for all HW models he plans to run on
> > > > tx_prep is implemented - he is free to use it.
> > > > 2) It would be difficult for Tomasz (and other Intel guys) to implement tx_prep()
> > > > for all non-Intel HW that DPDK supports right now.
> > > > We just don't have all the actual HW in stock and probably adequate knowledge of it.
> > > > So we depend here on the good will of other PMD mainaners/developers to implement
> > > > tx_prep() for these devices.
> > > > From other side, if it will be disabled by default, then, I think,
> > > > PMD developers just wouldn't be motivated to implement it.
> > > > So it will be left untested and unused forever.
> > >
> > > Actually as another thought:
> > > Can we have it enabled by default, but mark it as experimental or so?
> > > If memory serves me right, we've done that for cryptodev in the past, no?
> >
> > Cryptodev was a whole new library.
> > We won't play the game "find which function is experimental or not".
> >
> > We should not enable a function until it is fully implemented.
> >
> > If the user really understands that it will work only with few drivers
> > then he can change the build configuration himself.
> > Enabling in the default configuration is a message to say that it works
> > everywhere without any risk.
> > It's so simple that I don't even understand why I must argue for.
> >
> > And by the way, it is late for 16.11.
>
> Ok, I understand your concern about enabling it by default and testpmd breakage,
> but what else you believe is not ready?
That's already a lot!
I commented also about function naming.
All these things are trivial to fix.
But it is late. After RC1, we should stop integrating new features.
> > I suggest to integrate it in the beginning of 17.02 cycle, with the hope
> > that you can convince other developers to implement it in other drivers,
> > so we could finally enable it in the default config.
>
> Ok, any insights then, how we can convince people to do that?
You just have to explain clearly what this new feature is bringing
and what will be the future possibilities.
> BTW, it means then that tx_prep() should become part of mandatory API
> to be implemented by each PMD doing TX offloads, right?
Right.
The question is "what means mandatory"?
Should we block some patches for non-compliant drivers?
Should we remove offloads capability from non-compliant drivers?
> > Oh, and I don't trust that nobody were thinking that it would break testpmd
> > for non-Intel drivers.
>
> Well, believe it or not, but yes, I missed that one.
> I think I already admitted that it was my fault, and apologized for that.
And it's my fault not having seen that before.
I was hoping that good reviews would be done by other contributors.
> But sure, it is your choice to trust me here or not.
Konstantin I trust you.
However if nobody else was reviewing this patchset at Intel,
this is probably an issue.
And more importantly we must encourage other vendors to review such
major patch for the ethdev API.
^ permalink raw reply
* Re: [PATCH] app/test: fix wrong pointer values in crypto perftest
From: Trahe, Fiona @ 2016-10-28 13:42 UTC (permalink / raw)
To: Kusztal, ArkadiuszX, dev@dpdk.org
In-Reply-To: <1477654629-22181-1-git-send-email-arkadiuszx.kusztal@intel.com>
Hi Arek,
> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, October 28, 2016 12:37 PM
> To: dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Griffin, John <john.griffin@intel.com>;
> Jain, Deepak K <deepak.k.jain@intel.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Subject: [PATCH] app/test: fix wrong pointer values in crypto perftest
>
> This commit fixes problem with device hanging because of wrong pointer
> values in snow3g performance test
>
> Fixes: 97fe6461c7cb ("app/test: add SNOW 3G performance test")
>
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
Can you add to resolved issues section of release notes please.
^ permalink raw reply
* Re: mbuf changes
From: Pattan, Reshma @ 2016-10-28 13:34 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev@dpdk.org, Morten Brørup
In-Reply-To: <4427b23b-e9c0-4e43-9c28-fb7e56da745c@6wind.com>
Hi Olivier,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, October 25, 2016 1:49 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>; Morten Brørup
> <mb@smartsharesystems.com>
> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Wiles, Keith
> <keith.wiles@intel.com>; dev@dpdk.org; Oleg Kuporosov
> <olegk@mellanox.com>
> Subject: Re: [dpdk-dev] mbuf changes
>
>
>
> On 10/25/2016 02:45 PM, Bruce Richardson wrote:
> > On Tue, Oct 25, 2016 at 02:33:55PM +0200, Morten Brørup wrote:
> >> Comments at the end.
> >>
> >> Med venlig hilsen / kind regards
> >> - Morten Brørup
> >>
> >>> -----Original Message-----
> >>> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> >>> Sent: Tuesday, October 25, 2016 2:20 PM
> >>> To: Morten Brørup
> >>> Cc: Adrien Mazarguil; Wiles, Keith; dev@dpdk.org; Olivier Matz; Oleg
> >>> Kuporosov
> >>> Subject: Re: [dpdk-dev] mbuf changes
> >>>
> >>> On Tue, Oct 25, 2016 at 02:16:29PM +0200, Morten Brørup wrote:
> >>>> Comments inline.
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> >>>>> Richardson
> >>>>> Sent: Tuesday, October 25, 2016 1:14 PM
> >>>>> To: Adrien Mazarguil
> >>>>> Cc: Morten Brørup; Wiles, Keith; dev@dpdk.org; Olivier Matz; Oleg
> >>>>> Kuporosov
> >>>>> Subject: Re: [dpdk-dev] mbuf changes
> >>>>>
> >>>>> On Tue, Oct 25, 2016 at 01:04:44PM +0200, Adrien Mazarguil wrote:
> >>>>>> On Tue, Oct 25, 2016 at 12:11:04PM +0200, Morten Brørup wrote:
> >>>>>>> Comments inline.
> >>>>>>>
> >>>>>>> Med venlig hilsen / kind regards
> >>>>>>> - Morten Brørup
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> >>>>>>>> Sent: Tuesday, October 25, 2016 11:39 AM
> >>>>>>>> To: Bruce Richardson
> >>>>>>>> Cc: Wiles, Keith; Morten Brørup; dev@dpdk.org; Olivier Matz;
> >>>>>>>> Oleg Kuporosov
> >>>>>>>> Subject: Re: [dpdk-dev] mbuf changes
> >>>>>>>>
> >>>>>>>> On Mon, Oct 24, 2016 at 05:25:38PM +0100, Bruce Richardson
> >>> wrote:
> >>>>>>>>> On Mon, Oct 24, 2016 at 04:11:33PM +0000, Wiles, Keith
> >>> wrote:
> >>>>>>>> [...]
> >>>>>>>>>>> On Oct 24, 2016, at 10:49 AM, Morten Brørup
> >>>>>>>> <mb@smartsharesystems.com> wrote:
> >>>>>>>> [...]
> >>>>
> >>>>>>>>> One other point I'll mention is that we need to have a
> >>>>>>>>> discussion on how/where to add in a timestamp value into
> >>> the
> >>>>>>>>> mbuf. Personally, I think it can be in a union with the
> >>>>> sequence
> >>>>>>>>> number value, but I also suspect that 32-bits of a
> >>> timestamp
> >>>>>>>>> is not going to be enough for
> >>>>>>>> many.
> >>>>>>>>>
> >>>>>>>>> Thoughts?
> >>>>>>>>
> >>>>>>>> If we consider that timestamp representation should use
> >>>>> nanosecond
> >>>>>>>> granularity, a 32-bit value may likely wrap around too
> >>> quickly
> >>>>>>>> to be useful. We can also assume that applications requesting
> >>>>>>>> timestamps may care more about latency than throughput, Oleg
> >>>>> found
> >>>>>>>> that using the second cache line for this purpose had a
> >>>>> noticeable impact [1].
> >>>>>>>>
> >>>>>>>> [1] http://dpdk.org/ml/archives/dev/2016-October/049237.html
> >>>>>>>
> >>>>>>> I agree with Oleg about the latency vs. throughput importance
> >>>>>>> for
> >>>>> such applications.
> >>>>>>>
> >>>>>>> If you need high resolution timestamps, consider them to be
> >>>>> generated by the NIC RX driver, possibly by the hardware itself
> >>>>> (http://w3new.napatech.com/features/time-precision/hardware-time-
> >>>>> stamp), so the timestamp belongs in the first cache line. And I am
> >>>>> proposing that it should have the highest possible accuracy, which
> >>>>> makes the value hardware dependent.
> >>>>>>>
> >>>>>>> Furthermore, I am arguing that we leave it up to the
> >>> application
> >>>>>>> to
> >>>>> keep track of the slowly moving bits (i.e. counting whole seconds,
> >>>>> hours and calendar date) out of band, so we don't use precious
> >>> space
> >>>>> in the mbuf. The application doesn't need the NIC RX driver's fast
> >>>>> path to capture which date (or even which second) a packet was
> >>>>> received. Yes, it adds complexity to the application, but we can't
> >>>>> set aside 64 bit for a generic timestamp. Or as a weird tradeoff:
> >>>>> Put the fast moving 32 bit in the first cache line and the slow
> >>>>> moving 32 bit in the second cache line, as a placeholder for the
> >>> application to fill out if needed.
> >>>>> Yes, it means that the application needs to check the time and
> >>>>> update its variable holding the slow moving time once every second
> >>>>> or so; but that should be doable without significant effort.
> >>>>>>
> >>>>>> That's a good point, however without a 64 bit value, elapsed time
> >>>>>> between two arbitrary mbufs cannot be measured reliably due to
> >>> not
> >>>>>> enough context, one way or another the low resolution value is
> >>>>>> also
> >>>>> needed.
> >>>>>>
> >>>>>> Obviously latency-sensitive applications are unlikely to perform
> >>>>>> lengthy buffering and require this but I'm not sure about all the
> >>>>>> possible use-cases. Considering many NICs expose 64 bit
> >>> timestaps,
> >>>>>> I suggest we do not truncate them.
> >>>>>>
> >>>>>> I'm not a fan of the weird tradeoff either, PMDs will be tempted
> >>>>>> to fill the extra 32 bits whenever they can and negate the
> >>>>>> performance improvement of the first cache line.
> >>>>>
> >>>>> I would tend to agree, and I don't really see any convenient way
> >>>>> to avoid putting in a 64-bit field for the timestamp in cache-line 0.
> >>>>> If we are ok with having this overlap/partially overlap with
> >>>>> sequence number, it will use up an extra 4B of storage in that
> >>> cacheline.
> >>>>
> >>>> I agree about the lack of convenience! And Adrien certainly has a
> >>> point about PMD temptations.
> >>>>
> >>>> However, I still don't think that a NICs ability to date-stamp a
> >>> packet is sufficient reason to put a date-stamp in cache line 0 of
> >>> the mbuf. Storing only the fast moving 32 bit in cache line 0 seems
> >>> like a good compromise to me.
> >>>>
> >>>> Maybe you can find just one more byte, so it can hold 17 minutes
> >>>> with nanosecond resolution. (I'm joking!)
> >>>>
> >>>> Please don't sacrifice the sequence number for the
> >>>> seconds/hours/days
> >>> part a timestamp. Maybe it could be configurable to use a 32 bit or
> >>> 64 bit timestamp.
> >>>>
> >>> Do you see both timestamp and sequence numbers being used together?
> >>> I would have thought that apps would either use one or the other?
> >>> However, your suggestion is workable in any case, to allow the
> >>> sequence number to overlap just the high 32 bits of the timestamp,
> >>> rather than the low.
> >>
> >> In our case, I can foresee sequence numbers used for packet processing and
> timestamps for timing analysis (and possibly for packet capturing, when being
> used). For timing analysis, we don’t need long durations, e.g. 4 seconds with 32
> bit nanosecond resolution suffices. And for packet capturing we are perfectly
> capable of adding the slowly moving 32 bit of the timestamp to our output data
> stream without fetching it from the mbuf.
> >>
>
> We should keep in mind that today we have the seqn field but it is not used by
> any PMD. In case it is implemented, would it be a per-queue sequence number?
> Is it useful from an application view?
>
> This field is only used by the librte_reorder library, and in my opinion, we should
> consider moving it in the second cache line since it is not filled by the PMD.
>
>
> > For the 32-bit timestamp case, it might be useful to have a
> > right-shift value passed in to the ethdev driver. If we assume a NIC
> > with nanosecond resolution, (or TSC value with resolution of that
> > order of magnitude), then the app can choose to have 1 ns resolution
> > with 4 second wraparound, or alternatively 4ns resolution with 16
> > second wraparound, or even microsecond resolution with wrap around of over
> an hour.
> > The cost is obviously just a shift op in the driver code per packet -
> > hopefully with multiple packets done at a time using vector operations.
>
>
> About the timestamp, we can manage to find 64 bits in the first cache line,
> without sacrifying any field we have today. The question is more for the fields
> we may want to add later.
>
> To answer to the question of the size of the timestamp, the first question is to
> know what is the precision required for the applications using it?
As part of the 17.02 latency calculation feature, the requirement is to report latency in nanoseconds. So, +1 on keeping timestamp as 64bits.
Since the feature is planned for 17.02, can we finalize on the timestamp position in the mbuf struct?
Based on the decision, I am planning to make the change and send ABI notice if needed.
Reshma
>
> I don't quite like the idea of splitting the timestamp in the 2 cache lines, I think it
> would not be easy to use.
>
>
> Olivier
^ permalink raw reply
* Re: Solarflare PMD submission question
From: Thomas Monjalon @ 2016-10-28 13:14 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: dev
In-Reply-To: <4e5189f0-d340-ae7c-0300-aefd64041022@solarflare.com>
2016-10-28 16:05, Andrew Rybchenko:
> On 10/28/2016 03:33 PM, Thomas Monjalon wrote:
> > 2016-10-28 13:50, Andrew Rybchenko:
> >> First of all I'd like to double check that it is clear that we discuss
> >> libefx
> >> (base driver in terms of DPDK) import here. The PMD itself is already split
> >> in 20+ patches.
> > I don't know libefx. In DPDK, a base driver is often a subdirectory
> > inside the PMD. Will it be the case?
>
> Yes. Just to be absolutely sure: are the discussed requirements to split
> applicable to base driver import?
Yes I'm talking about the base driver. But they are not some requirements.
Take them as advices.
The first priority is to welcome your new driver.
The second priority is to make sure that it is open and readable enough.
^ permalink raw reply
* Re: [RFC] [PATCH v2] libeventdev: event driven programming model framework for DPDK
From: Van Haaren, Harry @ 2016-10-28 13:10 UTC (permalink / raw)
To: Vincent Jardin, Jerin Jacob, dev@dpdk.org
Cc: Eads, Gage, Vangati, Narender, thomas.monjalon@6wind.com
In-Reply-To: <15802480868.27fc.bb328046f2889bc8f44aafa891a44dd2@6wind.com>
> From: Vincent Jardin [mailto:vincent.jardin@6wind.com]
> Sent: Wednesday, October 26, 2016 7:37 PM
> Le 26 octobre 2016 2:11:26 PM "Van Haaren, Harry"
> <harry.van.haaren@intel.com> a écrit :
>
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> >>
> >> So far, I have received constructive feedback from Intel, NXP and Linaro folks.
> >> Let me know, if anyone else interested in contributing to the definition of
> >> eventdev?
> >>
> >> If there are no major issues in proposed spec, then Cavium would like work on
> >> implementing and up-streaming the common code(lib/librte_eventdev/) and
> >> an associated HW driver.(Requested minor changes of v2 will be addressed
> >> in next version).
> >
> > Hi All,
> >
> > I will propose a minor change to the rte_event struct, allowing some bits
> > to be implementation specific. Currently the rte_event struct has no space
> > to allow an implementation store any metadata about the event. For software
> > performance it would be really helpful if there are some bits available for
> > the implementation to keep some flags about each event.
> >
> > I suggest to rework the struct as below which opens 6 bits that were
> > otherwise wasted, and define them as implementation specific. By
> > implementation specific it is understood that the implementation can
> > overwrite any information stored in those bits, and the application must
> > not expect the data to remain after the event is scheduled.
> >
> > OLD:
> > struct rte_event {
> > uint32_t flow_id:24;
> > uint32_t queue_id:8;
> > uint8_t sched_type; /* Note only 2 bits of 8 are required */
> >
> > NEW:
> > struct rte_event {
> > uint32_t flow_id:24;
> > uint32_t sched_type:2; /* reduced size : but 2 bits is enough for the
> > enqueue types Ordered,Atomic,Parallel.*/
> > uint32_t implementation:6; /* available for implementation specific
> > metadata */
> > uint8_t queue_id; /* still 8 bits as before */
>
> Bitfileds are efficients on Octeon. What's about other CPUs you have in
> mind? x86 is not as efficient.
Given the rte_event struct is 16 bytes and there's no free space to use, I see no alternative than using bitfields in this case. Wecloming suggestions of a better way to layout the structure to avoid the bitfield.
Regards, -Harry
^ permalink raw reply
* Re: Solarflare PMD submission question
From: Andrew Rybchenko @ 2016-10-28 13:05 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
In-Reply-To: <1838661.tWJoShD1UF@xps13>
On 10/28/2016 03:33 PM, Thomas Monjalon wrote:
> 2016-10-28 13:50, Andrew Rybchenko:
>> First of all I'd like to double check that it is clear that we discuss
>> libefx
>> (base driver in terms of DPDK) import here. The PMD itself is already split
>> in 20+ patches.
> I don't know libefx. In DPDK, a base driver is often a subdirectory
> inside the PMD. Will it be the case?
Yes. Just to be absolutely sure: are the discussed requirements to split
applicable to base driver import?
Andrew.
^ permalink raw reply
* Re: [PATCH v11 1/6] ethdev: add Tx preparation
From: Ananyev, Konstantin @ 2016-10-28 12:59 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org
In-Reply-To: <4589776.b4NVqTvzOG@xps13>
>
> 2016-10-28 11:34, Ananyev, Konstantin:
> > > > 2016-10-27 16:24, Ananyev, Konstantin:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > 2016-10-27 15:52, Ananyev, Konstantin:
> > > > > > > > 2016-10-26 14:56, Tomasz Kulasek:
> > > > > > > > > --- a/config/common_base
> > > > > > > > > +++ b/config/common_base
> > > > > > > > > +CONFIG_RTE_ETHDEV_TX_PREP=y
> > > > > > > >
> > > > > > > > We cannot enable it until it is implemented in every drivers.
> > > > > > >
> > > > > > > Not sure why?
> > > > > > > If tx_pkt_prep == NULL, then rte_eth_tx_prep() would just act as noop.
> > > > > > > Right now it is not mandatory for the PMD to implement it.
> > > > > >
> > > > > > If it is not implemented, the application must do the preparation by itself.
> > > > > > From patch 6:
> > > > > > "
> > > > > > Removed pseudo header calculation for udp/tcp/tso packets from
> > > > > > application and used Tx preparation API for packet preparation and
> > > > > > verification.
> > > > > > "
> > > > > > So how does it behave with other drivers?
> > > > >
> > > > > Hmm so it seems that we broke testpmd csumonly mode for non-intel drivers..
> > > > > My bad, missed that part completely.
> > > > > Yes, then I suppose for now we'll need to support both (with and without) code paths for testpmd.
> > > > > Probably a new fwd mode or just extra parameter for the existing one?
> > > > > Any other suggestions?
> > > >
> > > > Please think how we can use it in every applications.
> > > > It is not ready.
> > > > Either we introduce the API without enabling it, or we implement it
> > > > in every drivers.
> > >
> > > I understand your position here, but just like to point that:
> > > 1) It is a new functionality optional to use.
> > > The app is free not to use that functionality and still do the preparation itself
> > > (as it has to do it now).
> > > All existing apps would keep working as expected without using that function.
> > > Though if the app developer knows that for all HW models he plans to run on
> > > tx_prep is implemented - he is free to use it.
> > > 2) It would be difficult for Tomasz (and other Intel guys) to implement tx_prep()
> > > for all non-Intel HW that DPDK supports right now.
> > > We just don't have all the actual HW in stock and probably adequate knowledge of it.
> > > So we depend here on the good will of other PMD mainaners/developers to implement
> > > tx_prep() for these devices.
> > > From other side, if it will be disabled by default, then, I think,
> > > PMD developers just wouldn't be motivated to implement it.
> > > So it will be left untested and unused forever.
> >
> > Actually as another thought:
> > Can we have it enabled by default, but mark it as experimental or so?
> > If memory serves me right, we've done that for cryptodev in the past, no?
>
> Cryptodev was a whole new library.
> We won't play the game "find which function is experimental or not".
>
> We should not enable a function until it is fully implemented.
>
> If the user really understands that it will work only with few drivers
> then he can change the build configuration himself.
> Enabling in the default configuration is a message to say that it works
> everywhere without any risk.
> It's so simple that I don't even understand why I must argue for.
>
> And by the way, it is late for 16.11.
Ok, I understand your concern about enabling it by default and testpmd breakage,
but what else you believe is not ready?
> I suggest to integrate it in the beginning of 17.02 cycle, with the hope
> that you can convince other developers to implement it in other drivers,
> so we could finally enable it in the default config.
Ok, any insights then, how we can convince people to do that?
BTW, it means then that tx_prep() should become part of mandatory API
to be implemented by each PMD doing TX offloads, right?
>
> Oh, and I don't trust that nobody were thinking that it would break testpmd
> for non-Intel drivers.
Well, believe it or not, but yes, I missed that one.
I think I already admitted that it was my fault, and apologized for that.
But sure, it is your choice to trust me here or not.
Konstantin
^ permalink raw reply
* Re: [PATCH v7 00/21] Introduce SoC device/driver framework for EAL
From: Shreyansh Jain @ 2016-10-28 12:35 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon
In-Reply-To: <1477657598-826-1-git-send-email-shreyansh.jain@nxp.com>
On Friday 28 October 2016 05:56 PM, Shreyansh Jain wrote:
> Introduction:
> =============
>
> This patch set is direct derivative of Jan's original series [1],[2].
>
> - This version is based on master HEAD (ca41215)
>
> - In this, I am merging the series [11] back. It was initially part
> of this set but I had split considering that those changes in PCI
> were good standalone as well. But, 1) not much feedback was avail-
> able and 2) this patchset is a use-case for those patches making
> it easier to review. Just like what Jan had intended in original
> series.
>
> - SoC support is not enabled by default. It needs the 'enable-soc' toggle
> on command line. This is primarily because this patchset is still
> experimental and we would like to keep it isolated from non-SoC ops.
> Though, it does impact the ABI.
Sending v7 as patch 11/21 of v6 wasn't received by mailing list leading
to automated build failure. No other change has been done.
-
Shreyansh
^ permalink raw reply
* Re: Solarflare PMD submission question
From: Thomas Monjalon @ 2016-10-28 12:33 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: dev
In-Reply-To: <2de43e28-37f8-86f5-b45f-2e598137b3dc@solarflare.com>
2016-10-28 13:50, Andrew Rybchenko:
> First of all I'd like to double check that it is clear that we discuss
> libefx
> (base driver in terms of DPDK) import here. The PMD itself is already split
> in 20+ patches.
I don't know libefx. In DPDK, a base driver is often a subdirectory
inside the PMD. Will it be the case?
> The only thing which comes to my mind is to split libefx import on subsystem
> basis (few files per subsystem). It is artificial and added files will
> be abandoned
> until the patch which adds them into build. It could be something like:
> 1. External interfaces definition
> 2. Internal interfaces definition
> 3. Registers definition (hardware interface)
> 4. Management CPU interface definition (it is one file, but still big
> 650K)
> 5. Management CPU interface implementation
> and so on for NIC global controls, interrupts, event queue, transmit,
> receive,
> filtering etc.
Yes it is artificial.
The most valuable would be a transversal logical split, kind of feature
per feature, in order to explain how the device works.
Such commit is also the opportunity to explain acronyms and so on.
> > It would be also really appreciated to provide a design documentation
> > in doc/guides/nics. Are the datasheets open? A link in the doc would help.
>
> We have a documentation which grows together with supported features,
> but it is rather for users. Important design decisions (not so many) are
> documented nearby corresponding code. Unfortunately there is no open
> datasheets. Management CPU interface definition has comments.
Without neither a datasheet, nor a comprehensive code introduction, it is
almost impossible to dive in your code. So it misses the point about bringing
it to an Open Source project.
Please do the the effort to bring some knowledge to the community.
Thanks
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox