* 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: KNI discussion in userspace event
From: Igor Ryzhov @ 2016-10-28 17:29 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org, Yigit, Ferruh
In-Reply-To: <31440590.xYOza9ndpd@xps13>
On Fri, Oct 28, 2016 at 7:13 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:
> 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.
>
That's not true.
iproute can control a lot of things like MAC address, promiscuous, MTU,
etc. that cannot be controlled with ethtool.
Just compare net_device_ops and ethtool_ops to see the difference.
And the question is not only about tools, it is also about how Linux kernel
works with network devices.
And it uses net_device_ops, not ethtool_ops.
>
> > > 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
* [PATCH v2 1/1] mempool: Add sanity check when secondary link in less mempools than primary
From: Jean Tourrilhes @ 2016-10-28 18:37 UTC (permalink / raw)
To: dev, Thomas Monjalon, David Marchand, Sergio Gonzalez Monroy
If the mempool ops the caller wants to use is not registered, the
library will segfault in an obscure way when trying to use that
mempool. It's better to catch it early and warn the user.
If the primary and secondary process were build using different build
systems, the list of constructors included by the linker in each
binary might be different. Mempools are registered via constructors, so
the linker magic will directly impact which tailqs are registered with
the primary and the secondary.
DPDK currently assumes that the secondary has a superset of the
mempools registered at the primary, and they are in the same order
(same index in primary and secondary). In some build scenario, the
secondary might not initialise any mempools at all.
This would also catch cases where there is a bug in the mempool
registration, or some memory corruptions, but this has not been
observed.
Signed-off-by: Jean Tourrilhes <jt@labs.hpe.com>
---
lib/librte_mempool/rte_mempool.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 2e28e2e..82260cc 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -1275,6 +1275,25 @@ rte_mempool_lookup(const char *name)
return NULL;
}
+ /* Sanity check : secondary may have initialised less mempools
+ * than primary due to linker and constructor magic. Or maybe
+ * there is a mempool corruption or bug. In any case, we can't
+ * go on, we will segfault in an obscure way.
+ * This does not detect the case where the constructor order
+ * is different between primary and secondary and where the
+ * index points to the wrong ops. This would require more
+ * extensive changes, and is much less likely.
+ * Jean II */
+ if(mp->ops_index >= (int32_t) rte_mempool_ops_table.num_ops) {
+ unsigned i;
+ /* Dump list of mempool ops for further investigation. */
+ for (i = 0; i < rte_mempool_ops_table.num_ops; i++) {
+ RTE_LOG(ERR, EAL, "Registered mempool[%d] is %s\n", i, rte_mempool_ops_table.ops[i].name);
+ }
+ /* Do not dump mempool list itself, it will segfault. */
+ rte_panic("Cannot find ops for mempool, ops_index %d, num_ops %d - maybe due to build process or linker configuration\n", mp->ops_index, rte_mempool_ops_table.num_ops);
+ }
+
return mp;
}
^ permalink raw reply related
* Re: KNI discussion in userspace event
From: Thomas Monjalon @ 2016-10-28 18:40 UTC (permalink / raw)
To: Igor Ryzhov; +Cc: dev, Yigit, Ferruh
In-Reply-To: <CAF+s_FxXM2uds7RwxXab-w47ieG7mFEOUwW5T5E=NnO36epWoQ@mail.gmail.com>
2016-10-28 20:29, Igor Ryzhov:
> On Fri, Oct 28, 2016 Thomas Monjalon wrote:
> > 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:
> > > > > * 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.
>
> That's not true.
> iproute can control a lot of things like MAC address, promiscuous, MTU,
> etc. that cannot be controlled with ethtool.
> Just compare net_device_ops and ethtool_ops to see the difference.
Yes you're right. iproute was not a good example :)
> And the question is not only about tools, it is also about how Linux kernel
> works with network devices.
> And it uses net_device_ops, not ethtool_ops.
What do you mean exactly? I feel you have something in mind.
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Igor Ryzhov @ 2016-10-28 19:23 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org, Yigit, Ferruh
In-Reply-To: <9236278.Tlj3KysXEu@xps13>
On Fri, Oct 28, 2016 at 9:40 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:
> 2016-10-28 20:29, Igor Ryzhov:
> > On Fri, Oct 28, 2016 Thomas Monjalon wrote:
> > > 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:
> > > > > > * 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.
> >
> > That's not true.
> > iproute can control a lot of things like MAC address, promiscuous, MTU,
> > etc. that cannot be controlled with ethtool.
> > Just compare net_device_ops and ethtool_ops to see the difference.
>
> Yes you're right. iproute was not a good example :)
>
> > And the question is not only about tools, it is also about how Linux
> kernel
> > works with network devices.
> > And it uses net_device_ops, not ethtool_ops.
>
> What do you mean exactly? I feel you have something in mind.
>
My main point is that if we want to control DPDK ports from Linux, it
should be done with standard utilities.
Every standard utility like iproute just uses existing Linux kernel
interfaces and kernel in its turn uses net_device_ops to control the device.
For example, you want to set MTU of the network device.
Regardless of the utility you use to do that (even if you write your own),
there are two options – ioctl or netlink.
And regardless of the method you choose, Linux kernel will then call
"ndo_change_mtu".
^ permalink raw reply
* Re: mbuf changes
From: Morten Brørup @ 2016-10-28 20:27 UTC (permalink / raw)
To: Richardson, Bruce, Adrien Mazarguil; +Cc: dev
In-Reply-To: <59AF69C657FD0841A61C55336867B5B035B32000@IRSMSX103.ger.corp.intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Richardson, Bruce
> Sent: Friday, October 28, 2016 7:01 PM
> To: Adrien Mazarguil; Morten Brørup
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] mbuf changes
>
> > -----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/hardwar
> > > > > >>>>> e-
> > > > 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.
Agreed. And I would say that this exception path can add the higher 32 bit itself; it is not required by the PMD or even in the mbuf.
> >
> > > 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).
It would make some applications easier to develop if the mbuf contained a 64 bit timestamp; but I don't think these are very common applications. It's a tradeoff, and I'm advocating for conserving the very precious space in the mbuf, at the cost of requiring some extra work in these uncommon applications.
> >
> > > 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.
Agree. And I would go so far as to say that this should be a guiding principle for DPDK. If the NIC HW doesn't support something, the PMD shouldn't try to emulate it in software; instead, DPDK could provide various libraries to offer these features in software, like the packet type library mentioned by Olivier at the recent DPDK Userspace conference.
> >
> > 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.
Agree.
> >
>
> 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.
Agreed about 32/64 bit choice not being affected by this.
But take note of Adrien's comment about the potential delay from the packet arrives on the NIC until the application gets round to calling the PMD to fetch it. Software based timestamps may give a false impression about the application latency. Assume that the application polls the PMD and bulk fetches all the packets, then proceeds to processing them, and then starts over. If a packet arrives just after the first batch was fetched, a software timestamp in the PMD will set the time when the application starts fetching the next batch of packets, ignoring all the time the packet spent waiting for being fetched while the application processed the first batch of packets.
>
> /Bruce
-Morten
^ permalink raw reply
* Re: [PATCH] net/qede: fix gcc compiler option checks
From: Stephen Hemminger @ 2016-10-28 22:12 UTC (permalink / raw)
To: Rasesh Mody; +Cc: dev, Dept-EngDPDKDev, thomas.monjalon
In-Reply-To: <1477636677-18041-1-git-send-email-rasesh.mody@qlogic.com>
On Thu, 27 Oct 2016 23:37:57 -0700
Rasesh Mody <rasesh.mody@qlogic.com> wrote:
> From: Rasesh Mody <Rasesh.Mody@cavium.com>
>
> Using GCC_VERSION to check gcc version and decide whether to include
> that compiler option.
>
> Fixes: ec94dbc57362 ("qede: add base driver")
> Fixes: ecc7a5a27ffe ("net/qede/base: fix 32-bit build")
>
> Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
> ---
> drivers/net/qede/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/qede/Makefile b/drivers/net/qede/Makefile
> index 39751e4..29b443d 100644
> --- a/drivers/net/qede/Makefile
> +++ b/drivers/net/qede/Makefile
> @@ -46,11 +46,11 @@ endif
> endif
>
> ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> -ifeq ($(shell gcc -Wno-unused-but-set-variable -Werror -E - < /dev/null > /dev/null 2>&1; echo $$?),0)
> +ifeq ($(shell test $(GCC_VERSION) -ge 44 && echo 1), 1)
> CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
> endif
> CFLAGS_BASE_DRIVER += -Wno-missing-declarations
> -ifeq ($(shell gcc -Wno-maybe-uninitialized -Werror -E - < /dev/null > /dev/null 2>&1; echo $$?),0)
> +ifeq ($(shell test $(GCC_VERSION) -ge 46 && echo 1), 1)
> CFLAGS_BASE_DRIVER += -Wno-maybe-uninitialized
> endif
> CFLAGS_BASE_DRIVER += -Wno-strict-prototypes
Does this mean that less compiler checking is done or more?
It seems lots of drivers make the excuse:
"the base driver comes from another group and is known buggy but can't be fixed"
That doesn't reflect well on the quality of the DPDK.
^ permalink raw reply
* Re: [PATCH] net/qede: fix gcc compiler option checks
From: Mody, Rasesh @ 2016-10-28 22:49 UTC (permalink / raw)
To: Stephen Hemminger, Rasesh Mody
Cc: dev@dpdk.org, Dept-EngDPDKDev@qlogic.com,
thomas.monjalon@6wind.com
In-Reply-To: <20161028151203.1a47d895@xeon-e3>
Hi Stephen,
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> Hemminger
> Sent: Friday, October 28, 2016 3:12 PM
>
> On Thu, 27 Oct 2016 23:37:57 -0700
> Rasesh Mody <rasesh.mody@qlogic.com> wrote:
>
> > From: Rasesh Mody <Rasesh.Mody@cavium.com>
> >
> > Using GCC_VERSION to check gcc version and decide whether to include
> > that compiler option.
> >
> > Fixes: ec94dbc57362 ("qede: add base driver")
> > Fixes: ecc7a5a27ffe ("net/qede/base: fix 32-bit build")
> >
> > Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
> > ---
> > drivers/net/qede/Makefile | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/qede/Makefile b/drivers/net/qede/Makefile
> > index 39751e4..29b443d 100644
> > --- a/drivers/net/qede/Makefile
> > +++ b/drivers/net/qede/Makefile
> > @@ -46,11 +46,11 @@ endif
> > endif
> >
> > ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> > -ifeq ($(shell gcc -Wno-unused-but-set-variable -Werror -E - <
> > /dev/null > /dev/null 2>&1; echo $$?),0)
> > +ifeq ($(shell test $(GCC_VERSION) -ge 44 && echo 1), 1)
> > CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable endif
> > CFLAGS_BASE_DRIVER += -Wno-missing-declarations -ifeq ($(shell gcc
> > -Wno-maybe-uninitialized -Werror -E - < /dev/null > /dev/null 2>&1;
> > echo $$?),0)
> > +ifeq ($(shell test $(GCC_VERSION) -ge 46 && echo 1), 1)
> > CFLAGS_BASE_DRIVER += -Wno-maybe-uninitialized endif
> > CFLAGS_BASE_DRIVER += -Wno-strict-prototypes
>
> Does this mean that less compiler checking is done or more?
With higher version of compilers more compiler checking is done, for older compilers less checking is done. As some of the older compiles do not have newly added checking capabilities. Testing with latest compilers ensures we do lot more checking.
Thanks!
-Rasesh
> It seems lots of drivers make the excuse:
> "the base driver comes from another group and is known buggy but can't be
> fixed"
> That doesn't reflect well on the quality of the DPDK.
^ permalink raw reply
* Re: KNI discussion in userspace event
From: Vincent Jardin @ 2016-10-28 23:09 UTC (permalink / raw)
To: Igor Ryzhov, Thomas Monjalon; +Cc: dev, Yigit, Ferruh
In-Reply-To: <CAF+s_FwVWxbbF9Yf8B=hR9ZqOiTO5SEYaO0LipyXcaHQeos8Sg@mail.gmail.com>
Le 28 octobre 2016 9:23:06 PM Igor Ryzhov <iryzhov@nfware.com> a écrit :
> On Fri, Oct 28, 2016 at 9:40 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
>
>> 2016-10-28 20:29, Igor Ryzhov:
>> > On Fri, Oct 28, 2016 Thomas Monjalon wrote:
>> > > 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:
>> > > > > > * 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.
>> >
>> > That's not true.
>> > iproute can control a lot of things like MAC address, promiscuous, MTU,
>> > etc. that cannot be controlled with ethtool.
>> > Just compare net_device_ops and ethtool_ops to see the difference.
>>
>> Yes you're right. iproute was not a good example :)
>>
>> > And the question is not only about tools, it is also about how Linux
>> kernel
>> > works with network devices.
>> > And it uses net_device_ops, not ethtool_ops.
>>
>> What do you mean exactly? I feel you have something in mind.
>>
>
> My main point is that if we want to control DPDK ports from Linux, it
> should be done with standard utilities.
> Every standard utility like iproute just uses existing Linux kernel
> interfaces and kernel in its turn uses net_device_ops to control the device.
>
> For example, you want to set MTU of the network device.
> Regardless of the utility you use to do that (even if you write your own),
> there are two options – ioctl or netlink.
> And regardless of the method you choose, Linux kernel will then call
> "ndo_change_mtu".
DPDK runs on non Linux too (FreeBSD, maybe Windows some days). So we should
avoid creating such dependencies.
I do like the kind of bifurcated models (à la RDMA like mlx PMDs) so port
management is still done using plain OS drivers of the kernel. We should
rather push toward proper bifurcated drivers:
- so we decrease maintenance and avoid code duplication
- we benefit of kernel's interface management
I am not expert of RDMA on FreeBSD and Windows but it seems available.
^ permalink raw reply
* [PATCH 0/2] net/ena: check for free descriptors
From: Jakub Palider @ 2016-10-29 1:06 UTC (permalink / raw)
To: dev; +Cc: evgenys, matua, netanel
In some configurations there is mismatch between declared and actual
descriptor count which under heavy load may lead to resource shortage.
The first patch unifies the way head and tail indexes are handled and
is crucial for compactness and succeeding patch correctness.
The second patch runs check for available descriptor count.
Jakub Palider (2):
net/ena: use unmasked head/tail values
net/ena: check for free buffers prior to xmit
drivers/net/ena/ena_ethdev.c | 83 ++++++++++++++++++++++++++------------------
drivers/net/ena/ena_ethdev.h | 24 ++-----------
2 files changed, 52 insertions(+), 55 deletions(-)
--
2.5.0
^ permalink raw reply
* [PATCH 1/2] net/ena: use unmasked head/tail values
From: Jakub Palider @ 2016-10-29 1:06 UTC (permalink / raw)
To: dev; +Cc: evgenys, matua, Tal Avraham, netanel
In-Reply-To: <1477703175-21344-1-git-send-email-jpa@semihalf.com>
The next_to_clean and next_to_use ring pointers sometimes were kept
wrapped around ring size, sometimes used beyond this limit. From now
on we increment them without regard to ring size limits, but when
reading the values they are wrapped accordingly. Moreover unit16_t
are unsed whenever possible.
Signed-off-by: Tal Avraham <talavr@annapurnalabs.com>
Signed-off-by: Jakub Palider <jpa@semihalf.com>
---
drivers/net/ena/ena_ethdev.c | 77 ++++++++++++++++++++++++++------------------
drivers/net/ena/ena_ethdev.h | 24 ++------------
2 files changed, 47 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 737ae87..adf94f2 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -676,8 +676,7 @@ static void ena_rx_queue_release_bufs(struct ena_ring *ring)
if (m)
__rte_mbuf_raw_free(m);
- ring->next_to_clean =
- ENA_CIRC_INC(ring->next_to_clean, 1, ring->ring_size);
+ ring->next_to_clean++;
}
}
@@ -692,8 +691,7 @@ static void ena_tx_queue_release_bufs(struct ena_ring *ring)
if (tx_buf->mbuf)
rte_pktmbuf_free(tx_buf->mbuf);
- ring->next_to_clean =
- ENA_CIRC_INC(ring->next_to_clean, 1, ring->ring_size);
+ ring->next_to_clean++;
}
}
@@ -926,8 +924,8 @@ static int ena_queue_restart(struct ena_ring *ring)
if (ring->type == ENA_RING_TYPE_TX)
return 0;
- rc = ena_populate_rx_queue(ring, ring->ring_size - 1);
- if ((unsigned int)rc != ring->ring_size - 1) {
+ rc = ena_populate_rx_queue(ring, ring->ring_size);
+ if ((unsigned int)rc != ring->ring_size) {
PMD_INIT_LOG(ERR, "Failed to populate rx ring !\n");
return (-1);
}
@@ -962,6 +960,13 @@ static int ena_tx_queue_setup(struct rte_eth_dev *dev,
return -1;
}
+ if (!rte_is_power_of_2(nb_desc)) {
+ RTE_LOG(ERR, PMD,
+ "Unsupported size of RX queue: %d is not a power of 2.",
+ nb_desc);
+ return -EINVAL;
+ }
+
if (nb_desc > adapter->tx_ring_size) {
RTE_LOG(ERR, PMD,
"Unsupported size of TX queue (max size: %d)\n",
@@ -1056,6 +1061,13 @@ static int ena_rx_queue_setup(struct rte_eth_dev *dev,
return -1;
}
+ if (!rte_is_power_of_2(nb_desc)) {
+ RTE_LOG(ERR, PMD,
+ "Unsupported size of TX queue: %d is not a power of 2.",
+ nb_desc);
+ return -EINVAL;
+ }
+
if (nb_desc > adapter->rx_ring_size) {
RTE_LOG(ERR, PMD,
"Unsupported size of RX queue (max size: %d)\n",
@@ -1115,23 +1127,25 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
{
unsigned int i;
int rc;
- unsigned int ring_size = rxq->ring_size;
- unsigned int ring_mask = ring_size - 1;
- int next_to_use = rxq->next_to_use & ring_mask;
+ uint16_t ring_size = rxq->ring_size;
+ uint16_t ring_mask = ring_size - 1;
+ uint16_t next_to_use = rxq->next_to_use;
+ uint16_t in_use;
struct rte_mbuf **mbufs = &rxq->rx_buffer_info[0];
if (unlikely(!count))
return 0;
- ena_assert_msg((((ENA_CIRC_COUNT(rxq->next_to_use, rxq->next_to_clean,
- rxq->ring_size)) +
- count) < rxq->ring_size), "bad ring state");
+ in_use = rxq->next_to_use - rxq->next_to_clean;
+ ena_assert_msg(((in_use + count) <= ring_size), "bad ring state");
- count = RTE_MIN(count, ring_size - next_to_use);
+ count = RTE_MIN(count,
+ (uint16_t)(ring_size - (next_to_use & ring_mask)));
/* get resources for incoming packets */
rc = rte_mempool_get_bulk(rxq->mb_pool,
- (void **)(&mbufs[next_to_use]), count);
+ (void **)(&mbufs[next_to_use & ring_mask]),
+ count);
if (unlikely(rc < 0)) {
rte_atomic64_inc(&rxq->adapter->drv_stats->rx_nombuf);
PMD_RX_LOG(DEBUG, "there are no enough free buffers");
@@ -1139,7 +1153,8 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
}
for (i = 0; i < count; i++) {
- struct rte_mbuf *mbuf = mbufs[next_to_use];
+ uint16_t next_to_use_masked = next_to_use & ring_mask;
+ struct rte_mbuf *mbuf = mbufs[next_to_use_masked];
struct ena_com_buf ebuf;
rte_prefetch0(mbufs[((next_to_use + 4) & ring_mask)]);
@@ -1148,12 +1163,12 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
ebuf.len = mbuf->buf_len - RTE_PKTMBUF_HEADROOM;
/* pass resource to device */
rc = ena_com_add_single_rx_desc(rxq->ena_com_io_sq,
- &ebuf, next_to_use);
+ &ebuf, next_to_use_masked);
if (unlikely(rc)) {
RTE_LOG(WARNING, PMD, "failed adding rx desc\n");
break;
}
- next_to_use = ENA_RX_RING_IDX_NEXT(next_to_use, ring_size);
+ next_to_use++;
}
/* When we submitted free recources to device... */
@@ -1475,7 +1490,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
unsigned int ring_size = rx_ring->ring_size;
unsigned int ring_mask = ring_size - 1;
uint16_t next_to_clean = rx_ring->next_to_clean;
- int desc_in_use = 0;
+ uint16_t desc_in_use = 0;
unsigned int recv_idx = 0;
struct rte_mbuf *mbuf = NULL;
struct rte_mbuf *mbuf_head = NULL;
@@ -1493,8 +1508,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
return 0;
}
- desc_in_use = ENA_CIRC_COUNT(rx_ring->next_to_use,
- next_to_clean, ring_size);
+ desc_in_use = rx_ring->next_to_use - next_to_clean;
if (unlikely(nb_pkts > desc_in_use))
nb_pkts = desc_in_use;
@@ -1535,8 +1549,7 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
mbuf_prev = mbuf;
segments++;
- next_to_clean =
- ENA_RX_RING_IDX_NEXT(next_to_clean, ring_size);
+ next_to_clean++;
}
/* fill mbuf attributes if any */
@@ -1549,10 +1562,10 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
}
/* Burst refill to save doorbells, memory barriers, const interval */
- if (ring_size - desc_in_use - 1 > ENA_RING_DESCS_RATIO(ring_size))
- ena_populate_rx_queue(rx_ring, ring_size - desc_in_use - 1);
+ if (ring_size - desc_in_use > ENA_RING_DESCS_RATIO(ring_size))
+ ena_populate_rx_queue(rx_ring, ring_size - desc_in_use);
- rx_ring->next_to_clean = next_to_clean & ring_mask;
+ rx_ring->next_to_clean = next_to_clean;
return recv_idx;
}
@@ -1561,7 +1574,8 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
uint16_t nb_pkts)
{
struct ena_ring *tx_ring = (struct ena_ring *)(tx_queue);
- unsigned int next_to_use = tx_ring->next_to_use;
+ uint16_t next_to_use = tx_ring->next_to_use;
+ uint16_t next_to_clean = tx_ring->next_to_clean;
struct rte_mbuf *mbuf;
unsigned int ring_size = tx_ring->ring_size;
unsigned int ring_mask = ring_size - 1;
@@ -1582,7 +1596,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
mbuf = tx_pkts[sent_idx];
- req_id = tx_ring->empty_tx_reqs[next_to_use];
+ req_id = tx_ring->empty_tx_reqs[next_to_use & ring_mask];
tx_info = &tx_ring->tx_buffer_info[req_id];
tx_info->mbuf = mbuf;
tx_info->num_of_bufs = 0;
@@ -1645,7 +1659,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
tx_info->tx_descs = nb_hw_desc;
- next_to_use = ENA_TX_RING_IDX_NEXT(next_to_use, ring_size);
+ next_to_use++;
}
/* If there are ready packets to be xmitted... */
@@ -1668,10 +1682,8 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
rte_pktmbuf_free(mbuf);
/* Put back descriptor to the ring for reuse */
- tx_ring->empty_tx_reqs[tx_ring->next_to_clean] = req_id;
- tx_ring->next_to_clean =
- ENA_TX_RING_IDX_NEXT(tx_ring->next_to_clean,
- tx_ring->ring_size);
+ tx_ring->empty_tx_reqs[next_to_clean & ring_mask] = req_id;
+ next_to_clean++;
/* If too many descs to clean, leave it for another run */
if (unlikely(total_tx_descs > ENA_RING_DESCS_RATIO(ring_size)))
@@ -1681,6 +1693,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
if (total_tx_descs > 0) {
/* acknowledge completion of sent packets */
ena_com_comp_ack(tx_ring->ena_com_io_sq, total_tx_descs);
+ tx_ring->next_to_clean = next_to_clean;
}
return sent_idx;
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index 61390a9..4c7edbb 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -42,33 +42,13 @@
#define ENA_MEM_BAR 2
#define ENA_MAX_NUM_QUEUES 128
-
-#define ENA_DEFAULT_TX_SW_DESCS (1024)
-#define ENA_DEFAULT_TX_HW_DESCS (1024)
#define ENA_DEFAULT_RING_SIZE (1024)
-
#define ENA_MIN_FRAME_LEN 64
-
-#define ENA_NAME_MAX_LEN 20
-#define ENA_IRQNAME_SIZE 40
-
-#define ENA_PKT_MAX_BUFS 17
+#define ENA_NAME_MAX_LEN 20
+#define ENA_PKT_MAX_BUFS 17
#define ENA_MMIO_DISABLE_REG_READ BIT(0)
-#define ENA_CIRC_COUNT(head, tail, size) \
- (((uint16_t)((uint16_t)(head) - (uint16_t)(tail))) & ((size) - 1))
-
-#define ENA_CIRC_INC(index, step, size) \
- ((uint16_t)(index) + (uint16_t)(step))
-#define ENA_CIRC_INC_WRAP(index, step, size) \
- (((uint16_t)(index) + (uint16_t)(step)) & ((size) - 1))
-
-#define ENA_TX_RING_IDX_NEXT(idx, ring_size) \
- ENA_CIRC_INC_WRAP(idx, 1, ring_size)
-#define ENA_RX_RING_IDX_NEXT(idx, ring_size) \
- ENA_CIRC_INC_WRAP(idx, 1, ring_size)
-
struct ena_adapter;
enum ena_ring_type {
--
2.5.0
^ permalink raw reply related
* [PATCH 2/2] net/ena: check for free buffers prior to xmit
From: Jakub Palider @ 2016-10-29 1:06 UTC (permalink / raw)
To: dev; +Cc: evgenys, matua, Tal Avraham, netanel
In-Reply-To: <1477703175-21344-1-git-send-email-jpa@semihalf.com>
Signed-off-by: Tal Avraham <talavr@annapurnalabs.com>
Signed-off-by: Jakub Palider <jpa@semihalf.com>
---
drivers/net/ena/ena_ethdev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index adf94f2..ab9a178 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1583,7 +1583,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
struct ena_tx_buffer *tx_info;
struct ena_com_buf *ebuf;
uint16_t rc, req_id, total_tx_descs = 0;
- uint16_t sent_idx = 0;
+ uint16_t sent_idx = 0, empty_tx_reqs;
int nb_hw_desc;
/* Check adapter state */
@@ -1593,6 +1593,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
return 0;
}
+ empty_tx_reqs = ring_size - (next_to_use - next_to_clean);
+ if (nb_pkts > empty_tx_reqs)
+ nb_pkts = empty_tx_reqs;
+
for (sent_idx = 0; sent_idx < nb_pkts; sent_idx++) {
mbuf = tx_pkts[sent_idx];
--
2.5.0
^ permalink raw reply related
* Re: [PATCH] net/qede: fix advertising link speed capability
From: Harish Patil @ 2016-10-29 1:11 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org, Dept-Eng DPDK Dev
In-Reply-To: <9851708.0H4Kn83q5Z@xps13>
>2016-10-27 23:42, Rasesh Mody:
>> From: Harish Patil <harish.patil@qlogic.com>
>>
>> Fix to advertise device's link speed capability based on current
>> link speed rather than returning driver supported speeds.
>[...]
>> - dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G |
>> - ETH_LINK_SPEED_100G;
>> + memset(&link, 0, sizeof(struct qed_link_output));
>> + qdev->ops->common->get_link(edev, &link);
>> + dev_info->speed_capa = rte_eth_speed_bitflag(link.speed, 0);
>> }
>
>No, that's wrong.
>You must advertise a capability!
Got it. It was not very clear to me. Let me send out v2 patch that would
advertise native link speed based on the NIC personality configured (and
not based on current/negotiated link speed).
>So what the device supports?
10, 25, 40, 50, 100G speeds
>Are every qede devices support ETH_LINK_SPEED_100G?
No
^ permalink raw reply
* [PATCH v2] net/qede: fix advertising link speed capability
From: Rasesh Mody @ 2016-10-29 6:14 UTC (permalink / raw)
To: thomas.monjalon; +Cc: dev, Dept-EngDPDKDev
In-Reply-To: <9851708.0H4Kn83q5Z@xps13>
From: Harish Patil <harish.patil@qlogic.com>
Fix to advertise device's link speed capability based on NVM
port configuration instead of returning driver supported speeds.
Fixes: 95e67b479506 ("net/qede: add 100G link speed capability")
Signed-off-by: Harish Patil <harish.patil@qlogic.com>
---
drivers/net/qede/qede_ethdev.c | 6 ++++--
drivers/net/qede/qede_ethdev.h | 1 +
drivers/net/qede/qede_if.h | 1 +
drivers/net/qede/qede_main.c | 24 ++++++++++++++++++++++++
4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index b91b478..59129f2 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -646,6 +646,7 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
{
struct qede_dev *qdev = eth_dev->data->dev_private;
struct ecore_dev *edev = &qdev->edev;
+ struct qed_link_output link;
PMD_INIT_FUNC_TRACE(edev);
@@ -678,8 +679,9 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev,
DEV_TX_OFFLOAD_UDP_CKSUM |
DEV_TX_OFFLOAD_TCP_CKSUM);
- dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G |
- ETH_LINK_SPEED_100G;
+ memset(&link, 0, sizeof(struct qed_link_output));
+ qdev->ops->common->get_link(edev, &link);
+ dev_info->speed_capa = rte_eth_speed_bitflag(link.adv_speed, 0);
}
/* return 0 means link status changed, -1 means not changed */
diff --git a/drivers/net/qede/qede_ethdev.h b/drivers/net/qede/qede_ethdev.h
index 5eb3f52..a97e3d9 100644
--- a/drivers/net/qede/qede_ethdev.h
+++ b/drivers/net/qede/qede_ethdev.h
@@ -30,6 +30,7 @@
#include "base/ecore_dev_api.h"
#include "base/ecore_iov_api.h"
#include "base/ecore_cxt.h"
+#include "base/nvm_cfg.h"
#include "qede_logs.h"
#include "qede_if.h"
diff --git a/drivers/net/qede/qede_if.h b/drivers/net/qede/qede_if.h
index 2d38b1b..4936349 100644
--- a/drivers/net/qede/qede_if.h
+++ b/drivers/net/qede/qede_if.h
@@ -70,6 +70,7 @@ struct qed_link_output {
uint32_t advertised_caps; /* In ADVERTISED defs */
uint32_t lp_caps; /* In ADVERTISED defs */
uint32_t speed; /* In Mb/s */
+ uint32_t adv_speed; /* In Mb/s */
uint8_t duplex; /* In DUPLEX defs */
uint8_t port; /* In PORT defs */
bool autoneg;
diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
index 2d354e1..e6501d0 100644
--- a/drivers/net/qede/qede_main.c
+++ b/drivers/net/qede/qede_main.c
@@ -488,6 +488,7 @@ static void qed_fill_link(struct ecore_hwfn *hwfn,
struct ecore_mcp_link_state link;
struct ecore_mcp_link_capabilities link_caps;
uint32_t media_type;
+ uint32_t adv_speed;
uint8_t change = 0;
memset(if_link, 0, sizeof(*if_link));
@@ -514,6 +515,29 @@ static void qed_fill_link(struct ecore_hwfn *hwfn,
if_link->speed = link.speed;
if_link->duplex = QEDE_DUPLEX_FULL;
+
+ /* Fill up the native advertised speed */
+ switch (params.speed.advertised_speeds) {
+ case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_10G:
+ adv_speed = 10000;
+ break;
+ case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_25G:
+ adv_speed = 25000;
+ break;
+ case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_40G:
+ adv_speed = 40000;
+ break;
+ case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_50G:
+ adv_speed = 50000;
+ break;
+ case NVM_CFG1_PORT_DRV_SPEED_CAPABILITY_MASK_BB_100G:
+ adv_speed = 100000;
+ break;
+ default:
+ DP_NOTICE(hwfn, false, "Unknown speed\n");
+ adv_speed = 0;
+ }
+ if_link->adv_speed = adv_speed;
if (params.speed.autoneg)
if_link->supported_caps |= QEDE_SUPPORTED_AUTONEG;
--
1.8.3.1
^ permalink raw reply related
* [PATCH] net/bonding: not handle vlan slow packet
From: linhaifeng @ 2016-10-31 1:34 UTC (permalink / raw)
To: dev, declan.doherty
From: ZengGanghui <ZengGanghui@huawei.com>
if rx vlan offload is enable we should not handle vlan slow
packets too.
Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 43334f7..6c74bba 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -169,7 +169,7 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
/* Remove packet from array if it is slow packet or slave is not
* in collecting state or bondign interface is not in promiscus
* mode and packet address does not match. */
- if (unlikely((hdr->ether_type == ether_type_slow_be ||
+ if (unlikely(((hdr->ether_type == ether_type_slow_be && !bufs[j]->vlan_tci) ||
!collecting || (!promisc &&
!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac, &hdr->d_addr))) &&
--
1.8.3.1
^ permalink raw reply related
* net/bonding: not handle vlan slow packet
From: linhaifeng @ 2016-10-31 2:14 UTC (permalink / raw)
To: dev@dpdk.org, declan.doherty
if rx vlan offload is enable we should not handle vlan slow
packets too.
Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 43334f7..6c74bba 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -169,7 +169,7 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
/* Remove packet from array if it is slow packet or slave is not
* in collecting state or bondign interface is not in promiscus
* mode and packet address does not match. */
- if (unlikely((hdr->ether_type == ether_type_slow_be ||
+ if (unlikely(((hdr->ether_type == ether_type_slow_be && !bufs[j]->vlan_tci) ||
!collecting || (!promisc &&
!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac, &hdr->d_addr))) &&
^ permalink raw reply related
* [PATCH] net/bonding: not handle vlan slow packet
From: linhaifeng @ 2016-10-31 2:39 UTC (permalink / raw)
To: dev, declan.doherty
From: Haifeng Lin <haifeng.lin@huawei.com>
if rx vlan offload is enable we should not handle vlan slow
packets too.
Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 09ce7bf..9e99442 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -169,7 +169,7 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
/* Remove packet from array if it is slow packet or slave is not
* in collecting state or bondign interface is not in promiscus
* mode and packet address does not match. */
- if (unlikely(hdr->ether_type == ether_type_slow_be ||
+ if (unlikely((hdr->ether_type == ether_type_slow_be && && !bufs[j]->vlan_tci) ||
!collecting || (!promisc &&
!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac, &hdr->d_addr)))) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH] net/bonding: not handle vlan slow packet
From: linhaifeng @ 2016-10-31 3:52 UTC (permalink / raw)
To: dev, declan.doherty
From: Haifeng Lin <haifeng.lin@huawei.com>
if rx vlan offload is enable we should not handle vlan slow
packets too.
Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com>
---
drivers/net/bonding/rte_eth_bond_pmd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 09ce7bf..ca17898 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -169,7 +169,8 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
/* Remove packet from array if it is slow packet or slave is not
* in collecting state or bondign interface is not in promiscus
* mode and packet address does not match. */
- if (unlikely(hdr->ether_type == ether_type_slow_be ||
+ if (unlikely((hdr->ether_type == ether_type_slow_be &&
+ !bufs[j]->vlan_tci) ||
!collecting || (!promisc &&
!is_multicast_ether_addr(&hdr->d_addr) &&
!is_same_ether_addr(&bond_mac, &hdr->d_addr)))) {
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC v2] Generic flow director/filtering/classification API
From: Zhang, Helin @ 2016-10-31 7:19 UTC (permalink / raw)
To: Adrien Mazarguil, dev@dpdk.org
In-Reply-To: <20160929171053.GP17252@6wind.com>
Hi Adrien
Just a double check, do you have any update on the v1 patch set, as now it is the end of October?
We are extremly eager to see the v1 patch set for development.
I don't think we need full validation on the v1 patch set for API. It should be together with PMD and example application.
If we can see the v1 API patch set earlier, we can help to validate it with our code changes. That's should be more efficient and helpful.
Any comments on my personal understanding?
Thank you very much for the hard work and kind helps!
Regards,
Helin
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Friday, September 30, 2016 1:11 AM
> To: dev@dpdk.org
> Cc: Thomas Monjalon
> Subject: Re: [dpdk-dev] [RFC v2] Generic flow director/filtering/classification
> API
>
> On Fri, Aug 19, 2016 at 08:50:44PM +0200, Adrien Mazarguil wrote:
> > Hi All,
> >
> > Thanks to many for the positive and constructive feedback I've
> > received so far. Here is the updated specification (v0.7) at last.
> >
> > I've attempted to address as many comments as possible but could not
> > process them all just yet. A new section "Future evolutions" has been
> > added for the remaining topics.
> >
> > This series adds rte_flow.h to the DPDK tree. Next time I will attempt
> > to convert the specification as a documentation commit part of the
> > patchset and actually implement API functions.
> [...]
>
> A quick update, we initially targeted 16.11 as the DPDK release this API would
> be available for, turns out this goal was somewhat too optimistic as
> September is ending and we are about to overshoot the deadline for
> integration (basically everything took longer than expected, big surprise).
>
> So instead of rushing things now to include a botched API in 16.11 with no
> PMD support, we simply modified the target, now set to 17.02. On the plus
> side this should leave developers more time to refine and test the API before
> applications and PMDs start to use it.
>
> I intend to send the patchset for the first non-draft version mid-October
> worst case (ASAP in fact). I still haven't replied to several comments but did
> take them into account, thanks for your feedback.
>
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply
* Re: [PATCH v4] vhost: Add indirect descriptors support to the TX path
From: Wang, Zhihong @ 2016-10-31 10:01 UTC (permalink / raw)
To: Maxime Coquelin, Yuanhan Liu
Cc: mst@redhat.com, dev@dpdk.org, vkaplans@redhat.com
In-Reply-To: <88169067-290d-a7bb-ab2c-c9b8ec1b1ded@redhat.com>
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, October 28, 2016 3:42 PM
> To: Wang, Zhihong <zhihong.wang@intel.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>
> Cc: stephen@networkplumber.org; Pierre Pfister (ppfister)
> <ppfister@cisco.com>; Xie, Huawei <huawei.xie@intel.com>; dev@dpdk.org;
> vkaplans@redhat.com; mst@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v4] vhost: Add indirect descriptors support
> to the TX path
>
>
>
> On 10/28/2016 02:49 AM, Wang, Zhihong wrote:
> >
> >> > -----Original Message-----
> >> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> >> > Sent: Thursday, October 27, 2016 6:46 PM
> >> > To: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> > Cc: Wang, Zhihong <zhihong.wang@intel.com>;
> >> > stephen@networkplumber.org; Pierre Pfister (ppfister)
> >> > <ppfister@cisco.com>; Xie, Huawei <huawei.xie@intel.com>;
> dev@dpdk.org;
> >> > vkaplans@redhat.com; mst@redhat.com
> >> > Subject: Re: [dpdk-dev] [PATCH v4] vhost: Add indirect descriptors
> support
> >> > to the TX path
> >> >
> >> > On Thu, Oct 27, 2016 at 12:35:11PM +0200, Maxime Coquelin wrote:
> >>> > >
> >>> > >
> >>> > > On 10/27/2016 12:33 PM, Yuanhan Liu wrote:
> >>>> > > >On Thu, Oct 27, 2016 at 11:10:34AM +0200, Maxime Coquelin
> wrote:
> >>>>> > > >>Hi Zhihong,
> >>>>> > > >>
> >>>>> > > >>On 10/27/2016 11:00 AM, Wang, Zhihong wrote:
> >>>>>> > > >>>Hi Maxime,
> >>>>>> > > >>>
> >>>>>> > > >>>Seems indirect desc feature is causing serious performance
> >>>>>> > > >>>degradation on Haswell platform, about 20% drop for both
> >>>>>> > > >>>mrg=on and mrg=off (--txqflags=0xf00, non-vector version),
> >>>>>> > > >>>both iofwd and macfwd.
> >>>>> > > >>I tested PVP (with macswap on guest) and Txonly/Rxonly on an
> Ivy
> >> > Bridge
> >>>>> > > >>platform, and didn't faced such a drop.
> >>>> > > >
> >>>> > > >I was actually wondering that may be the cause. I tested it with
> >>>> > > >my IvyBridge server as well, I saw no drop.
> >>>> > > >
> >>>> > > >Maybe you should find a similar platform (Haswell) and have a try?
> >>> > > Yes, that's why I asked Zhihong whether he could test Txonly in guest
> to
> >>> > > see if issue is reproducible like this.
> >> >
> >> > I have no Haswell box, otherwise I could do a quick test for you. IIRC,
> >> > he tried to disable the indirect_desc feature, then the performance
> >> > recovered. So, it's likely the indirect_desc is the culprit here.
> >> >
> >>> > > I will be easier for me to find an Haswell machine if it has not to be
> >>> > > connected back to back to and HW/SW packet generator.
> > In fact simple loopback test will also do, without pktgen.
> >
> > Start testpmd in both host and guest, and do "start" in one
> > and "start tx_first 32" in another.
> >
> > Perf drop is about 24% in my test.
> >
>
> Thanks, I never tried this test.
> I managed to find an Haswell platform (Intel(R) Xeon(R) CPU E5-2699 v3
> @ 2.30GHz), and can reproduce the problem with the loop test you
> mention. I see a performance drop about 10% (8.94Mpps/8.08Mpps).
> Out of curiosity, what are the numbers you get with your setup?
Hi Maxime,
Let's align our test case to RC2, mrg=on, loopback, on Haswell.
My results below:
1. indirect=1: 5.26 Mpps
2. indirect=0: 6.54 Mpps
It's about 24% drop.
>
> As I never tried this test, I run it again on my Sandy Bridge setup, and
> I also see a performance regression, this time of 4%.
>
> If I understand correctly the test, only 32 packets are allocated,
> corresponding to a single burst, which is less than the queue size.
> So it makes sense that the performance is lower with this test case.
Actually it's 32 burst, so 1024 packets in total, enough to
fill the queue.
Thanks
Zhihong
>
> Thanks,
> Maxime
^ permalink raw reply
* Red Hat 6.5 compilation failure
From: Nélio Laranjeiro @ 2016-10-31 10:43 UTC (permalink / raw)
To: dev, Thomas Monjalon, Bruce Richardson
Hi all,
Am facing an issue with compilation on redhat 6.5 of DPDK v16.11-rc2,
compilation fails with:
cc1: warnings being treated as errors
/root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c: In function ‘i40evf_dev_interrupt_handler’:
/root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1391: error: dereferencing pointer ‘v_msg’ does break strict-aliasing rules
/root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1397: error: dereferencing pointer ‘v_msg’ does break strict-aliasing rules
/root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1398: error: dereferencing pointer ‘v_msg’ does break strict-aliasing rules
/root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1376: note: initialized from here
cc1: warnings being treated as errors
/root/dpdk/drivers/net/bnxt/bnxt_hwrm.c: In function ‘bnxt_hwrm_func_driver_unregister’:
/root/dpdk/drivers/net/bnxt/bnxt_hwrm.c:105: error: dereferencing pointer ‘req’ does break strict-aliasing rules
/root/dpdk/drivers/net/bnxt/bnxt_hwrm.c:66: note: initialized from here
cc1: warnings being treated as errors
/root/dpdk/drivers/net/qede/base/ecore_mcp.c: In function ‘ecore_mcp_nvm_rd_cmd’:
/root/dpdk/build/include/rte_memcpy.h:740: error: array subscript is above array bounds
/root/dpdk/drivers/net/qede/base/ecore_mcp.c: In function ‘ecore_mcp_nvm_wr_cmd’:
/root/dpdk/build/include/rte_memcpy.h:740: error: array subscript is above array bounds
At top level:
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
In doc/guides/rel_notes/supported_os.rst we still support
"Red Hat Enterprise Linux 6.5".
Do we still want to support it, or should we update the documentation to
remove it?
Regards,
--
Nélio Laranjeiro
6WIND
^ permalink raw reply
* Re: mbuf changes
From: Morten Brørup @ 2016-10-31 10:55 UTC (permalink / raw)
To: Richardson, Bruce, Adrien Mazarguil, Pattan, Reshma; +Cc: dev
In-Reply-To: <59AF69C657FD0841A61C55336867B5B035B32000@IRSMSX103.ger.corp.intel.com>
About the mbuf timestamp (regardless of size etc.) accuracy:
We should also consider the clock source. If the NIC HW is the clock source, there will be a fixed offset from the CPU's clock, but there will also be varying drift (they have separate XTALs). And if there are multiple NICs, there will be offset and varying drift between their clocks too.
If I remember correctly, the IEEE Ethernet specifications require a clock accuracy of 100 ppm. So XTALs of this quality are probably being used in the hardware. This means that the clock drift between two NICs (or a NIC and the actual time) can be in the magnitude of 100.000 ns per second.
BTW: If you are interested in NTP, you should take a look at Poul-Henning Kamp's ntimed research: http://phk.freebsd.dk/time/index.html
Med venlig hilsen / kind regards
- Morten Brørup
^ permalink raw reply
* Re: Red Hat 6.5 compilation failure
From: Thomas Monjalon @ 2016-10-31 11:27 UTC (permalink / raw)
To: Nélio Laranjeiro; +Cc: dev
In-Reply-To: <20161031104318.GC4823@autoinstall.dev.6wind.com>
2016-10-31 11:43, Nélio Laranjeiro:
> Hi all,
>
> Am facing an issue with compilation on redhat 6.5 of DPDK v16.11-rc2,
Thanks Nelio for raising the issue.
> compilation fails with:
[...]
> In doc/guides/rel_notes/supported_os.rst we still support
> "Red Hat Enterprise Linux 6.5".
>
> Do we still want to support it, or should we update the documentation to
> remove it?
I vote for dropping RHEL6 support in DPDK 16.11.
Any other comment in the community?
Please give your voice, whether you agree or disagree.
^ permalink raw reply
* Re: Red Hat 6.5 compilation failure
From: Neil Horman @ 2016-10-31 14:16 UTC (permalink / raw)
To: Nélio Laranjeiro; +Cc: dev
In-Reply-To: <20161031104318.GC4823@autoinstall.dev.6wind.com>
On Mon, Oct 31, 2016 at 11:43:18AM +0100, Nélio Laranjeiro wrote:
> Hi all,
>
> Am facing an issue with compilation on redhat 6.5 of DPDK v16.11-rc2,
> compilation fails with:
>
> cc1: warnings being treated as errors
> /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c: In function ‘i40evf_dev_interrupt_handler’:
> /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1391: error: dereferencing pointer ‘v_msg’ does break strict-aliasing rules
> /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1397: error: dereferencing pointer ‘v_msg’ does break strict-aliasing rules
> /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1398: error: dereferencing pointer ‘v_msg’ does break strict-aliasing rules
> /root/dpdk/drivers/net/i40e/i40e_ethdev_vf.c:1376: note: initialized from here
>
>
> cc1: warnings being treated as errors
> /root/dpdk/drivers/net/bnxt/bnxt_hwrm.c: In function ‘bnxt_hwrm_func_driver_unregister’:
> /root/dpdk/drivers/net/bnxt/bnxt_hwrm.c:105: error: dereferencing pointer ‘req’ does break strict-aliasing rules
> /root/dpdk/drivers/net/bnxt/bnxt_hwrm.c:66: note: initialized from here
>
>
> cc1: warnings being treated as errors
> /root/dpdk/drivers/net/qede/base/ecore_mcp.c: In function ‘ecore_mcp_nvm_rd_cmd’:
> /root/dpdk/build/include/rte_memcpy.h:740: error: array subscript is above array bounds
> /root/dpdk/drivers/net/qede/base/ecore_mcp.c: In function ‘ecore_mcp_nvm_wr_cmd’:
> /root/dpdk/build/include/rte_memcpy.h:740: error: array subscript is above array bounds
> At top level:
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
>
> In doc/guides/rel_notes/supported_os.rst we still support
> "Red Hat Enterprise Linux 6.5".
>
Most of these errors stems from the fact that in i40e there is alot of casting
from structs which lay out data in typed members to structs which have flat
memory buffers, which can cause problems if the alignment of the former isn't
what you expect. You can disable strict aliasing if you want, but its a much
better idea to fix it properly.
The subscript above array options is pretty straightforward. rte_memcpy has
a case statement that blocks memory copies to be more efficient. I would
presume that there is a call site for a ecore read operation where the buffer
array is smaller than the number of bytes copied, and the compiler noticed.
> Do we still want to support it, or should we update the documentation to
> remove it?
>
I think support probably doesn't matter, but regardless of that decision, you
should likely look a bit more closely at these errors.
Neil
> Regards,
>
> --
> Nélio Laranjeiro
> 6WIND
>
^ permalink raw reply
* Re: mbuf changes
From: Morten Brørup @ 2016-10-31 16:05 UTC (permalink / raw)
To: Olivier Matz, Bruce Richardson; +Cc: dev
In-Reply-To: <4427b23b-e9c0-4e43-9c28-fb7e56da745c@6wind.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, October 25, 2016 2:49 PM
>
> 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.
+1 because it is not filled by the NIC HW or PMD. If our argument is "better performance", I would prefer having the generic 64 bit m->userdata in cacheline0 than m->seqn.
^ 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