[dpdk-dev] [PATCH] e1000/base: fix the wrong assignment to msgbuf[0]
In function e1000_update_mc_addr_list_vf(), "msgbuf[0]" is used prior
to initialization at "msgbuf[0] |= E1000_VF_SET_MULTICAST_OVERFLOW".
And "msgbuf[0]" is overwritten at "msgbuf[0] = E1000_VF_SET_MULTICAST".
Fix it by moving the second line prior to the first one that mentioned
above.
Signed-off-by: Yong Wang
---
drivers/net/e1000/base/e1000_vf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/e1000/base/e1000_vf.c
b/drivers/net/e1000/base/e1000_vf.c
index 7845b48..44ab018 100644
--- a/drivers/net/e1000/base/e1000_vf.c
+++ b/drivers/net/e1000/base/e1000_vf.c
@@ -421,12 +421,13 @@ void e1000_update_mc_addr_list_vf(struct e1000_hw *hw,
DEBUGOUT1("MC Addr Count = %d\n", mc_addr_count);
+ msgbuf[0] = E1000_VF_SET_MULTICAST;
+
if (mc_addr_count > 30) {
msgbuf[0] |= E1000_VF_SET_MULTICAST_OVERFLOW;
mc_addr_count = 30;
}
- msgbuf[0] = E1000_VF_SET_MULTICAST;
msgbuf[0] |= mc_addr_count << E1000_VT_MSGINFO_SHIFT;
for (i = 0; i < mc_addr_count; i++) {
--
1.8.3.1
Re: [dpdk-dev] [PATCH] doc: postpone API change in ethdev
On Mon, 13 Feb 2017 17:05:00 +, Ferruh Yigit wrote: > On 2/13/2017 2:26 PM, Thomas Monjalon wrote: > > The change of _rte_eth_dev_callback_process has not been done in > > 17.02. Let's postpone to 17.05. > > > > Signed-off-by: Thomas Monjalon > > Acked-by: Ferruh Yigit Acked-by: Olivier Matz
Re: [dpdk-dev] [PATCH] doc:add tested Intel platforms with Intel NICs.
> -Original Message- > From: Pei, Yulong > Sent: Tuesday, February 14, 2017 4:14 AM > To: dev@dpdk.org > Cc: Mcnamara, John ; thomas.monja...@6wind.com; > Pei, Yulong > Subject: [PATCH] doc:add tested Intel platforms with Intel NICs. > > Add tested Intel platforms with Intel NICs to the release note. > > Signed-off-by: Yulong Pei Thank you for the rework. Note for the future, the patch should have been v2 and the title should have a space between "doc:" and "add". Otherwise: Acked-by: John McNamara
Re: [dpdk-dev] [PATCH] e1000/base: fix the wrong assignment to msgbuf[0]
Hi Yong,
> -Original Message-
> From: Yong Wang [mailto:wang.yon...@zte.com.cn]
> Sent: Tuesday, February 14, 2017 5:14 PM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org; Yong Wang
> Subject: [PATCH] e1000/base: fix the wrong assignment to msgbuf[0]
>
> In function e1000_update_mc_addr_list_vf(), "msgbuf[0]" is used prior to
> initialization at "msgbuf[0] |= E1000_VF_SET_MULTICAST_OVERFLOW".
> And "msgbuf[0]" is overwritten at "msgbuf[0] = E1000_VF_SET_MULTICAST".
> Fix it by moving the second line prior to the first one that mentioned above.
>
> Signed-off-by: Yong Wang
> ---
> drivers/net/e1000/base/e1000_vf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/e1000/base/e1000_vf.c
> b/drivers/net/e1000/base/e1000_vf.c
> index 7845b48..44ab018 100644
> --- a/drivers/net/e1000/base/e1000_vf.c
> +++ b/drivers/net/e1000/base/e1000_vf.c
> @@ -421,12 +421,13 @@ void e1000_update_mc_addr_list_vf(struct
> e1000_hw *hw,
>
> DEBUGOUT1("MC Addr Count = %d\n", mc_addr_count);
>
> + msgbuf[0] = E1000_VF_SET_MULTICAST;
> +
> if (mc_addr_count > 30) {
> msgbuf[0] |= E1000_VF_SET_MULTICAST_OVERFLOW;
> mc_addr_count = 30;
> }
>
> - msgbuf[0] = E1000_VF_SET_MULTICAST;
> msgbuf[0] |= mc_addr_count << E1000_VT_MSGINFO_SHIFT;
>
> for (i = 0; i < mc_addr_count; i++) {
> --
> 1.8.3.1
Thanks for the patch. I believe it's a good fix.
As normally we don't change the base code. I'll double confirm it with my
colleague.
Re: [dpdk-dev] [PATCH RFCv3 00/19] ring cleanup and generalization
Hi Bruce, On Tue, 7 Feb 2017 14:12:38 +, Bruce Richardson wrote: > This patchset make a set of, sometimes non-backward compatible, > cleanup changes to the rte_ring code in order to improve it. The > resulting code is shorter*, since the existing functions are > restructured to reduce code duplication, as well as being more > consistent in behaviour. The specific changes made are explained in > each patch which makes that change. > > Key incompatibilities: > * The biggest, and probably most controversial change is that to the > enqueue and dequeue APIs. The enqueue/deq burst and bulk functions > have their function prototypes changed so that they all return an > additional parameter, indicating the size of next call which is > guaranteed to succeed. In case on enq, this is the number of > available slots on the ring, and in case of deq, it is the number of > objects which can be pulled. As well as this, the return value from > the bulk functions have been changed to make them compatible with the > burst functions. In all cases, the functions to enq/deq a set of objs > now return the number of objects processed, 0 or N, in the case of > bulk functions, 0, N or any value in between in the case of the burst > ones. [Due to the extra parameter, the compiler will flag all > instances of the function to allow the user to also change the return > value logic at the same time] > * The parameters to the single object enq/deq functions have not been > changed. Because of that, the return value is also unmodified - as > the compiler cannot automatically flag this to the user. > > Potential further cleanups: > * To a certain extent the rte_ring structure has gone from being a > whole ring structure, including a "ring" element itself, to just > being a header which can be reused, along with the head/tail update > functions to create new rings. For now, the enqueue code works by > assuming that the ring data goes immediately after the header, but > that can be changed to allow specialised ring implementations to put > additional metadata of their own after the ring header. I didn't see > this as being needed right now, but it may be worth considering for a > V1 patchset. > * There are 9 enqueue functions and 9 dequeue functions in > rte_ring.h. I suspect not all of those are used, so personally I > would consider dropping the functions to enqueue/dequeue a single > value using single or multi semantics, i.e. drop > rte_ring_sp_enqueue > rte_ring_mp_enqueue > rte_ring_sc_dequeue > rte_ring_mc_dequeue > That would still leave a single enqueue and dequeue function for > working with a single object at a time. > * It should be possible to merge the head update code for enqueue and > dequeue into a single function. The key difference between the two > is the calculation of how far the index can be moved. I felt that the > functions for moving the head index are sufficiently complicated > with many parameters to them already, that trying to merge in more > code would impede readability. However, if so desired this change can > be made at a later stage without affecting ABI or API. > > PERFORMANCE: > I've run performance autotests on a couple of (Intel) platforms. > Looking particularly at the core-2-core results, which I expect are > the main ones of interest, the performance after this patchset is a > few cycles per packet faster in my testing. I'm hoping it should be > at least neutral perf-wise. > > REQUEST FOR FEEDBACK: > * Are all of these changes worth making? I've quickly browsed all the patches. I think yes, we should do it: it brings a good cleanup, removing features we don't need, restructuring the code, and also adding the feature you need :) > * Should they be made in existing ring code, or do we look to provide > a new fifo library to completely replace the ring one? I think it's ok to have it in the existing code. Breaking the ABI is never suitable, but I think having 2 libs would be even more confusing. > * How does the implementation of new ring types using this code > compare vs that of the previous RFCs? I prefer this version, especially compared to the first RFC. Thanks for this big rework. I'll dive into the patches a do a more exhaustive review soon. Regards, Olivier
Re: [dpdk-dev] [PATCH] doc: add ABI change notification for ring library
On Mon, 13 Feb 2017 17:38:30 +, Bruce Richardson wrote: > Document proposed changes for the rings code in the next release. > > Signed-off-by: Bruce Richardson Acked-by: Olivier Matz
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > Sent: Monday, February 13, 2017 10:53 AM > To: Zhigang Lu ; Liming Sun > Cc: dev@dpdk.org > Subject: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal > > Signed-off-by: Thomas Monjalon > --- > doc/guides/rel_notes/deprecation.rst | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/doc/guides/rel_notes/deprecation.rst > b/doc/guides/rel_notes/deprecation.rst > index b49e0a0..d40b137 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -62,3 +62,6 @@ Deprecation Notices >PMDs that implement the latter. >Target release for removal of the legacy API will be defined once most >PMDs have switched to rte_flow. > + > +* The architecture TILE-Gx and the associated mpipe driver are not > + maintained and will be removed in 17.05. The notification should probably be for removal in 17.08, to one release cycle to respond/fix/update. It would also be good to hear from EZChip/Mellanox on this. John
Re: [dpdk-dev] [PATCH] e1000/base: fix the wrong assignment to msgbuf[0]
2017-02-14 04:14, Yong Wang: > In function e1000_update_mc_addr_list_vf(), "msgbuf[0]" is used prior > to initialization at "msgbuf[0] |= E1000_VF_SET_MULTICAST_OVERFLOW". > And "msgbuf[0]" is overwritten at "msgbuf[0] = E1000_VF_SET_MULTICAST". > Fix it by moving the second line prior to the first one that mentioned > above. > > Signed-off-by: Yong Wang Note about the title: msgbuf[0] does not explain anything in the title because we have no context when browsing the titles. It could be "e1000/base: fix multicast setting in VF".
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
2017-02-14 08:50, Mcnamara, John: > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > > Signed-off-by: Thomas Monjalon > > --- > > doc/guides/rel_notes/deprecation.rst | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > > b/doc/guides/rel_notes/deprecation.rst > > index b49e0a0..d40b137 100644 > > --- a/doc/guides/rel_notes/deprecation.rst > > +++ b/doc/guides/rel_notes/deprecation.rst > > @@ -62,3 +62,6 @@ Deprecation Notices > >PMDs that implement the latter. > >Target release for removal of the legacy API will be defined once most > >PMDs have switched to rte_flow. > > + > > +* The architecture TILE-Gx and the associated mpipe driver are not > > + maintained and will be removed in 17.05. > > The notification should probably be for removal in 17.08, to one release > cycle to respond/fix/update. There were some previous messages/warnings and there were no answer. I have asked a working free toolchain, I did not have it. We have asked some questions about drivers support, no reply. I have asked about removal, no reply. I really do not see why we should keep it longer. > It would also be good to hear from EZChip/Mellanox on this. They are mute since a long time on this topic. Olga, do you confirm?
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
2017-02-14 09:58, Thomas Monjalon: > 2017-02-14 08:50, Mcnamara, John: > > The notification should probably be for removal in 17.08, to one release > > cycle to respond/fix/update. > > There were some previous messages/warnings and there were no answer. > I have asked a working free toolchain, I did not have it. > We have asked some questions about drivers support, no reply. > I have asked about removal, no reply. > I really do not see why we should keep it longer. Some public links: Recent warnings: http://dpdk.org/ml/archives/dev/2017-February/056981.html Recent patch from Jerin without any comment: http://dpdk.org/commit/dff9071 Bug in 16.04 without any comment: https://mail-archive.com/dev@dpdk.org/msg31596.html Work stopped in March, last year: https://mail-archive.com/dev@dpdk.org/msg43536.html No update for free toolchain: https://mail-archive.com/dev@dpdk.org/msg42651.html One year without any news is enough. Hope it convinces you. > > It would also be good to hear from EZChip/Mellanox on this. > > They are mute since a long time on this topic. > Olga, do you confirm?
Re: [dpdk-dev] [PATCH RFCv3 00/19] ring cleanup and generalization
On Tue, Feb 14, 2017 at 09:32:20AM +0100, Olivier Matz wrote: > Hi Bruce, > > On Tue, 7 Feb 2017 14:12:38 +, Bruce Richardson > wrote: > > This patchset make a set of, sometimes non-backward compatible, > > cleanup changes to the rte_ring code in order to improve it. The > > resulting code is shorter*, since the existing functions are > > restructured to reduce code duplication, as well as being more > > consistent in behaviour. The specific changes made are explained in > > each patch which makes that change. > > > > Key incompatibilities: > > * The biggest, and probably most controversial change is that to the > > enqueue and dequeue APIs. The enqueue/deq burst and bulk functions > > have their function prototypes changed so that they all return an > > additional parameter, indicating the size of next call which is > > guaranteed to succeed. In case on enq, this is the number of > > available slots on the ring, and in case of deq, it is the number of > > objects which can be pulled. As well as this, the return value from > > the bulk functions have been changed to make them compatible with the > > burst functions. In all cases, the functions to enq/deq a set of objs > > now return the number of objects processed, 0 or N, in the case of > > bulk functions, 0, N or any value in between in the case of the burst > > ones. [Due to the extra parameter, the compiler will flag all > > instances of the function to allow the user to also change the return > > value logic at the same time] > > * The parameters to the single object enq/deq functions have not been > > changed. Because of that, the return value is also unmodified - as > > the compiler cannot automatically flag this to the user. > > > > Potential further cleanups: > > * To a certain extent the rte_ring structure has gone from being a > > whole ring structure, including a "ring" element itself, to just > > being a header which can be reused, along with the head/tail update > > functions to create new rings. For now, the enqueue code works by > > assuming that the ring data goes immediately after the header, but > > that can be changed to allow specialised ring implementations to put > > additional metadata of their own after the ring header. I didn't see > > this as being needed right now, but it may be worth considering for a > > V1 patchset. > > * There are 9 enqueue functions and 9 dequeue functions in > > rte_ring.h. I suspect not all of those are used, so personally I > > would consider dropping the functions to enqueue/dequeue a single > > value using single or multi semantics, i.e. drop > > rte_ring_sp_enqueue > > rte_ring_mp_enqueue > > rte_ring_sc_dequeue > > rte_ring_mc_dequeue > > That would still leave a single enqueue and dequeue function for > > working with a single object at a time. > > * It should be possible to merge the head update code for enqueue and > > dequeue into a single function. The key difference between the two > > is the calculation of how far the index can be moved. I felt that the > > functions for moving the head index are sufficiently complicated > > with many parameters to them already, that trying to merge in more > > code would impede readability. However, if so desired this change can > > be made at a later stage without affecting ABI or API. > > > > PERFORMANCE: > > I've run performance autotests on a couple of (Intel) platforms. > > Looking particularly at the core-2-core results, which I expect are > > the main ones of interest, the performance after this patchset is a > > few cycles per packet faster in my testing. I'm hoping it should be > > at least neutral perf-wise. > > > > REQUEST FOR FEEDBACK: > > * Are all of these changes worth making? > > I've quickly browsed all the patches. I think yes, we should do it: it > brings a good cleanup, removing features we don't need, restructuring > the code, and also adding the feature you need :) > > > > * Should they be made in existing ring code, or do we look to provide > > a new fifo library to completely replace the ring one? > > I think it's ok to have it in the existing code. Breaking the ABI > is never suitable, but I think having 2 libs would be even more > confusing. > > > > * How does the implementation of new ring types using this code > > compare vs that of the previous RFCs? > > I prefer this version, especially compared to the first RFC. > > > Thanks for this big rework. I'll dive into the patches a do a more > exhaustive review soon. > Great, thanks. I'm aware of a few things that already need to be cleaned up for V1 e.g. comments are not always correctly updated on functions. /Bruce
[dpdk-dev] i40e queues per VF
Hi, When reading the documentation, it is not easy to understand the capability of i40evf for the number of queues. First, please could you explain why we need a build-time config option? In the doc, there is neither justification nor tuning guidelines: http://dpdk.org/doc/guides/nics/i40e.html#config-file-options " CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_PF (default 64) Number of queues reserved for PF. CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF (default 4) Number of queues reserved for each SR-IOV VF. " I feel these are hard limits and should be some constants in the code, not some build configuration options. The other doc to look at is: http://dpdk.org/doc/guides/nics/intel_vf.html#intel-fortville-10-40-gigabit-ethernet-controller-vf-infrastructure " Each VF can have a maximum of 16 queue pairs. " Do we agree that a queue pair is 1 Rx queue / 1 Tx queue? Note: the concept of queue pairs in Intel VF should be explained somewhere. Below, a different limitation is given: " The available queue number(at most 4) per VF depends on the total number of pool, which is determined by the max number of VF at PF initialization stage and the number of queue specified in config " So what is the real maximum of queue pairs? 4 or 16? The datasheet talks about 16 queues. Is it 8 pairs? Is there something to configure the number of queues when creating VF with the kernel driver?
Re: [dpdk-dev] [PATCH] doc: postpone API change in ethdev
> -Original Message- > From: Olivier Matz [mailto:olivier.m...@6wind.com] > Sent: Tuesday, February 14, 2017 8:12 AM > To: Yigit, Ferruh > Cc: Thomas Monjalon ; Iremonger, Bernard > ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: postpone API change in ethdev > > On Mon, 13 Feb 2017 17:05:00 +, Ferruh Yigit > wrote: > > On 2/13/2017 2:26 PM, Thomas Monjalon wrote: > > > The change of _rte_eth_dev_callback_process has not been done in > > > 17.02. Let's postpone to 17.05. > > > > > > Signed-off-by: Thomas Monjalon > > > > Acked-by: Ferruh Yigit > > Acked-by: Olivier Matz Acked-by: Bernard Iremonger
Re: [dpdk-dev] [PATCH] doc: add deprecation note to add parameter in rte_cryptodev_info.sym
On 14/02/2017 12:11 PM, akhil.go...@nxp.com wrote: From: Akhil Goyal A new parameter is planned to be added in 17.05 release in rte_cryptodev_info.sym - max_nb_sessions_per_qp. This will allow applications to know the maximum number of session which can be attached to queue_pairs of device. Signed-off-by: Akhil Goyal --- ... Acked-by: Declan Doherty
Re: [dpdk-dev] [PATCH v2] doc: announce API and ABI change for ethdev
> -Original Message- > From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com] > Sent: Tuesday, February 14, 2017 3:17 AM > To: Thomas Monjalon > Cc: Iremonger, Bernard ; dev@dpdk.org; > Mcnamara, John > Subject: Re: [dpdk-dev] [PATCH v2] doc: announce API and ABI change for > ethdev > > On Mon, Feb 13, 2017 at 06:57:20PM +0100, Thomas Monjalon wrote: > > 2017-01-05 15:25, Bernard Iremonger: > > > In 17.05 nine rte_eth_dev_* functions will be removed from > > > librte_ether, renamed and moved to the ixgbe PMD. > > > > > > Signed-off-by: Bernard Iremonger > > > > "ixgbe bypass" should be in the title and the description. > > I'll reword to: > > > > doc: announce move of ethdev bypass function to ixgbe API > > > > In 17.05, nine rte_eth_dev_* functions for bypass control, and > > implemented only in ixgbe, will be removed from ethdev, renamed and > > moved to the ixgbe PMD-specific API. > > > > Acked-by: Thomas Monjalon > > Acked-by: Jerin Jacob Acked-by: Bernard Iremonger
[dpdk-dev] [PATCH v2] doc: annouce ABI change for cryptodev ops structure
Signed-off-by: Fan Zhang Acked-by: Hemant Agrawal --- v2: Rework the grammar doc/guides/rel_notes/deprecation.rst | 4 1 file changed, 4 insertions(+) diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst index b49e0a0..d64858f 100644 --- a/doc/guides/rel_notes/deprecation.rst +++ b/doc/guides/rel_notes/deprecation.rst @@ -62,3 +62,7 @@ Deprecation Notices PMDs that implement the latter. Target release for removal of the legacy API will be defined once most PMDs have switched to rte_flow. + +* ABI changes are planned for 17.05 in the ``rte_cryptodev_ops`` structure. + A pointer to a rte_cryptodev_config structure will be added to the + function prototype "cryptodev_configuret_t, as a new parameter. -- 2.7.4
Re: [dpdk-dev] crypto drivers in the API
On 13/02/2017 1:25 PM, Thomas Monjalon wrote: In the crypto API, the drivers are listed. In my opinion, it is a wrong designed and these lists should be removed. Do we need a deprecation notice to plan this removal in 17.05, while working on bus abstraction? ... Hey Thomas, I agree that these need to be removed, and I had planned on doing this for 17.05 but I have a concern on the requirements for ABI breakage in relation to this. This enum is unfortunately used in both the rte_cryptodev and rte_crypto_sym_session structures which are part of the libraries public API. I don't think it would be feasible to maintain a set of 17.02 compatible APIs with the changes this would introduce, as it would require a large number of functions to have 2 versions? Is it OK to break the ABI for this case?
Re: [dpdk-dev] [PATCH v2] doc: fix unreadable images
2017-02-13 10:45, Thomas Monjalon:
> 2017-02-13 07:45, Jianfeng Tan:
> > The images by below two commits are very unclear. Fix it.
> >
> > Fixes: 50665deebda ("doc: add guide to use virtio-user for container
> > networking")
> > Fixes: 0ba3870e755 ("doc: add guide to use virtio-user as exceptional path")
> >
> > Suggested-by: Thomas Monjalon
> > Signed-off-by: Jianfeng Tan
>
> That's a lot better!
>
> > +> transform="translate(2.9044,-261.723)">
> > + Sheet.63
> > + Contanier/App
> > +
> > +> height="22.5"/>
> > +> class="st8"/>
> > +> v:langID="1033">Contanier/App
>
> Please check wording. I've caught a typo above (Contanier).
No reply. What should I do? Apply with s/Contanier/Container/?
Re: [dpdk-dev] [PATCH] doc: announce kni_vhost removal
2016-11-17 13:27, Ferruh Yigit: > +* kni: Remove :ref:`kni_vhost_backend-label` feature (KNI_VHOST) in 17.05 > release. > + :doc:`Vhost Library ` is currently preferred method > for > + guest - host communication. Just for clarification, this is not to remove > KNI > + or VHOST feature, but KNI_VHOST which is a KNI feature enabled via a > compile > + time option, and disabled by default. Acked-by: Thomas Monjalon
Re: [dpdk-dev] [PATCH v2] doc: annouce ABI change for cryptodev ops structure
On 14/02/2017 10:41 AM, Fan Zhang wrote: Signed-off-by: Fan Zhang Acked-by: Hemant Agrawal --- ... Acked-by: Declan Doherty
Re: [dpdk-dev] [PATCH] doc: add deprecation note to add parameter in rte_cryptodev_info.sym
> -Original Message- > From: akhil.go...@nxp.com [mailto:akhil.go...@nxp.com] > Sent: Tuesday, February 14, 2017 12:11 PM > To: Doherty, Declan ; Trahe, Fiona > ; De Lara Guarch, Pablo > > Cc: thomas.monja...@6wind.com; dev@dpdk.org; > hemant.agra...@nxp.com; Jain, Deepak K ; Akhil > Goyal > Subject: [PATCH] doc: add deprecation note to add parameter in > rte_cryptodev_info.sym > > From: Akhil Goyal > > A new parameter is planned to be added in 17.05 release in > rte_cryptodev_info.sym - max_nb_sessions_per_qp. > > This will allow applications to know the maximum number of session > which can be attached to queue_pairs of device. > > Signed-off-by: Akhil Goyal Acked-by: Fiona Trahe
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
On Tue, Feb 14, 2017 at 10:28:39AM +0100, Thomas Monjalon wrote: > 2017-02-14 09:58, Thomas Monjalon: > > 2017-02-14 08:50, Mcnamara, John: > > > The notification should probably be for removal in 17.08, to one release > > > cycle to respond/fix/update. > > > > There were some previous messages/warnings and there were no answer. > > I have asked a working free toolchain, I did not have it. > > We have asked some questions about drivers support, no reply. > > I have asked about removal, no reply. > > I really do not see why we should keep it longer. > > Some public links: > Recent warnings: http://dpdk.org/ml/archives/dev/2017-February/056981.html > Recent patch from Jerin without any comment: http://dpdk.org/commit/dff9071 > Bug in 16.04 without any comment: > https://mail-archive.com/dev@dpdk.org/msg31596.html > Work stopped in March, last year: > https://mail-archive.com/dev@dpdk.org/msg43536.html > No update for free toolchain: > https://mail-archive.com/dev@dpdk.org/msg42651.html > > One year without any news is enough. > Hope it convinces you. > > > > It would also be good to hear from EZChip/Mellanox on this. > > > > They are mute since a long time on this topic. > > Olga, do you confirm? > Regretfully, I must agree with Thomas on this. In the absense of a response from someone to maintain this, it should be removed. Acked-by: Bruce Richardson
[dpdk-dev] Further fun with ABI tracking
Hi, when moving to DPDK 16.11 Debian/Ubuntu packaging of DPDK has hit a new twist on the (it seems reoccurring) topic of DPDK ABI tracking. I have found, ... well I don't want to call it solution ..., let's say a crutch to get around it for the moment. But I wanted to use the example I had to share a few thoughts on it and to kick off a wider discussion. *## In library cross-dependencies plus partial ABI bumps ##* Since the day moving away from the combined shared library we had several improvements on tracking the ABI versions. These days [1] we have LIBABIVER per library and it gets bumped to reflect it is breaking with former versions e.g. removing symbols. Now in the 16.11 release the ABIs for cryptodev, eal and ethdev got bumped by [2] and [3]. OTOH please remember that in general two versions of a shared library in the usual sense are meant to be able to stay alongside on a system without hurting each other. I picked a random one on my system. Package Library libisc-export160: /lib/x86_64-linux-gnu/libisc-export.so.160 libisc-export160: /lib/x86_64-linux-gnu/libisc-export.so.160.0.0 libisc-export95: /lib/x86_64-linux-gnu/libisc-export.so.95 libisc-export95: /lib/x86_64-linux-gnu/libisc-export.so.95.5.0 Some link against the new, some against the old library - all fine. Usually most programs can just be rebuilt against the new library and after some time the old one can be dropped. That mechanism gives downstream distributions a way to handle transitions and consumers of libraries which might not all be ready for the same version every time. And since the per lib versioning with LIBABIVER and and the version maps we are good - in fact we qualify for all common cases on [4]. Now in DPDK of those libraries that got an ABI bump eal and ethdev are part of those which most of us consider "core libraries" and most other libs and pmds link to them. And here DPDK continues to be special, due to that inter-dependency with old and new libraries installed on the same system the following happens on openvswitch built for an older version of dpdk: ovs-vswitchd-dpdk librte_eal.so.2 => /usr/lib/x86_64-linux-gnu/librte_eal.so.2 librte_pdump.so.1 => /usr/lib/x86_64-linux-gnu/librte_pdump.so.1 librte_eal.so.3 => /usr/lib/x86_64-linux-gnu/librte_eal.so.3 You can see that Openvswitch itself depends on the "old" librte_eal.so.2. But because librte_pdump.so.1 did not get an ABI bump it got upgraded to the newer version from DPDK 16.11. But since the "new" pdump got built with the new DPDK 16.11 it depends on the "new" librte_eal.so.3. And having both in the same executable space at the same time causes segfaults and pain. As I said for now I have passed the issue with a crutch that I'm not proud of and I'd like to avoid in the future. For that I'm reaching out to you with several suggestions to discuss. *## Thoughts ##* None of these seems like a perfect solution to me yet, but clearly good to start discussions on them. Options that were in discussion so far and that we might adopt next cycle (some of these are upstream changes, some downstream, some require both to change - but any of them should have an ack upstream so that we are agreeing how to proceed with those cases). 1. Downstreams to insert Major version into soname Distributions could insert the DPDK major version (like 16.11) into the soname and package names. A common example of this is libboost [5]. That would perfectly allow 16.07. to coexist with 16.11. even if for a given library LIBABIVER did not change. Yet it would mean that anything depending on the old library will have to be recompiled to pick up the new code, even if it depends on an ABI that is still present in the new release. Also - not a technical reason - but it is clearly more work to force update all dependencies and clean out old packages for every release. 2. ABI Ranges One could argue that due to the detailed tracking of functions DPDK is already close to track not ABI levels but actually ABI ranges. DPDK could track LIBABIVERMIN and LIBABIVER. Every time functionality is added LIBABIVER would get bumped, but LIBABIVERMIN only gets moved to the OLDEST still supported ABI when things are dropped. So on a given library librte_foo you could have LIBABIVER=5 and LIBABIVERMIN=3. The make install would then install the shared lib as: librte_foo.so.5 and additionally links for all compatible versions: librte_foo.so.3 -> librte_foo.so.5 librte_foo.so.4 -> librte_foo.so.5 Yet, while is has some nice attributes this might make DPDK even more special and cause ABI level proliferation over time. Also even with this in place, changes moving LIBABIVERMIN "too fast" (too fast is different for each downstream) could still cause an issue like the one I initially described. 3. A lot of conflicts In packaging one can declare a package to conflict with another package [6]. Now we could declare e.g. librte_eal3 to conflict with librte_eal2 (and the same for all oth
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
On Tue, Feb 14, 2017 at 10:50:19AM +, Bruce Richardson wrote: > On Tue, Feb 14, 2017 at 10:28:39AM +0100, Thomas Monjalon wrote: > > 2017-02-14 09:58, Thomas Monjalon: > > > 2017-02-14 08:50, Mcnamara, John: > > > > The notification should probably be for removal in 17.08, to one > > > > release cycle to respond/fix/update. > > > > > > There were some previous messages/warnings and there were no answer. > > > I have asked a working free toolchain, I did not have it. > > > We have asked some questions about drivers support, no reply. > > > I have asked about removal, no reply. > > > I really do not see why we should keep it longer. > > > > Some public links: > > Recent warnings: http://dpdk.org/ml/archives/dev/2017-February/056981.html > > Recent patch from Jerin without any comment: http://dpdk.org/commit/dff9071 > > Bug in 16.04 without any comment: > > https://mail-archive.com/dev@dpdk.org/msg31596.html > > Work stopped in March, last year: > > https://mail-archive.com/dev@dpdk.org/msg43536.html > > No update for free toolchain: > > https://mail-archive.com/dev@dpdk.org/msg42651.html > > > > One year without any news is enough. > > Hope it convinces you. > > > > > > It would also be good to hear from EZChip/Mellanox on this. > > > > > > They are mute since a long time on this topic. > > > Olga, do you confirm? > > > > Regretfully, I must agree with Thomas on this. In the absense of a > response from someone to maintain this, it should be removed. > > Acked-by: Bruce Richardson Acked-by: Jerin Jacob >
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
On Tue, 14 Feb 2017 10:50:19 +, Bruce Richardson wrote: > On Tue, Feb 14, 2017 at 10:28:39AM +0100, Thomas Monjalon wrote: > > 2017-02-14 09:58, Thomas Monjalon: > > > 2017-02-14 08:50, Mcnamara, John: > > > > The notification should probably be for removal in 17.08, to > > > > one release cycle to respond/fix/update. > > > > > > There were some previous messages/warnings and there were no > > > answer. I have asked a working free toolchain, I did not have it. > > > We have asked some questions about drivers support, no reply. > > > I have asked about removal, no reply. > > > I really do not see why we should keep it longer. > > > > Some public links: > > Recent warnings: > > http://dpdk.org/ml/archives/dev/2017-February/056981.html Recent > > patch from Jerin without any comment: > > http://dpdk.org/commit/dff9071 Bug in 16.04 without any comment: > > https://mail-archive.com/dev@dpdk.org/msg31596.html Work stopped in > > March, last year: > > https://mail-archive.com/dev@dpdk.org/msg43536.html No update for > > free toolchain: https://mail-archive.com/dev@dpdk.org/msg42651.html > > > > One year without any news is enough. > > Hope it convinces you. > > > > > > It would also be good to hear from EZChip/Mellanox on this. > > > > > > They are mute since a long time on this topic. > > > Olga, do you confirm? > > > > Regretfully, I must agree with Thomas on this. In the absense of a > response from someone to maintain this, it should be removed. > > Acked-by: Bruce Richardson > Same here. Acked-by: Olivier Matz
Re: [dpdk-dev] [PATCH v2] doc: annouce ABI change for cryptodev ops structure
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Doherty, Declan > Sent: Tuesday, February 14, 2017 10:48 AM > To: Zhang, Roy Fan; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2] doc: annouce ABI change for cryptodev > ops structure > > On 14/02/2017 10:41 AM, Fan Zhang wrote: > > Signed-off-by: Fan Zhang > > Acked-by: Hemant Agrawal > > --- > ... > > > > Acked-by: Declan Doherty Acked-by: Pablo de Lara
Re: [dpdk-dev] crypto drivers in the API
2017-02-14 10:44, Doherty, Declan: > On 13/02/2017 1:25 PM, Thomas Monjalon wrote: > > In the crypto API, the drivers are listed. > > In my opinion, it is a wrong designed and these lists should be removed. > > Do we need a deprecation notice to plan this removal in 17.05, while > > working on bus abstraction? > > > ... > > > > Hey Thomas, > I agree that these need to be removed, and I had planned on doing this > for 17.05 but I have a concern on the requirements for ABI breakage in > relation to this. This enum is unfortunately used in both the > rte_cryptodev and rte_crypto_sym_session structures which are part of > the libraries public API. I don't think it would be feasible to maintain > a set of 17.02 compatible APIs with the changes this would introduce, as > it would require a large number of functions to have 2 versions? Is it > OK to break the ABI for this case? Yes If you were planning to do this, you should have sent a deprecation notice few weeks ago. Please send it now and we'll see if we have enough supporters shortly.
Re: [dpdk-dev] [PATCH] doc: add deprecation note to add parameter in rte_cryptodev_info.sym
> -Original Message- > From: Trahe, Fiona > Sent: Tuesday, February 14, 2017 10:49 AM > To: akhil.go...@nxp.com; Doherty, Declan; De Lara Guarch, Pablo > Cc: thomas.monja...@6wind.com; dev@dpdk.org; > hemant.agra...@nxp.com; Jain, Deepak K; Trahe, Fiona > Subject: RE: [PATCH] doc: add deprecation note to add parameter in > rte_cryptodev_info.sym > > > > > -Original Message- > > From: akhil.go...@nxp.com [mailto:akhil.go...@nxp.com] > > Sent: Tuesday, February 14, 2017 12:11 PM > > To: Doherty, Declan ; Trahe, Fiona > > ; De Lara Guarch, Pablo > > > > Cc: thomas.monja...@6wind.com; dev@dpdk.org; > > hemant.agra...@nxp.com; Jain, Deepak K ; > Akhil > > Goyal > > Subject: [PATCH] doc: add deprecation note to add parameter in > > rte_cryptodev_info.sym > > > > From: Akhil Goyal > > > > A new parameter is planned to be added in 17.05 release in > > rte_cryptodev_info.sym - max_nb_sessions_per_qp. > > > > This will allow applications to know the maximum number of session > > which can be attached to queue_pairs of device. > > > > Signed-off-by: Akhil Goyal > > Acked-by: Fiona Trahe Acked-by: Pablo de Lara
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
On Tue, Feb 14, 2017 at 08:50:07AM +, Mcnamara, John wrote: > > > > -Original Message- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > > Sent: Monday, February 13, 2017 10:53 AM > > To: Zhigang Lu ; Liming Sun > > Cc: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal > > > > Signed-off-by: Thomas Monjalon > > --- > > doc/guides/rel_notes/deprecation.rst | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > > b/doc/guides/rel_notes/deprecation.rst > > index b49e0a0..d40b137 100644 > > --- a/doc/guides/rel_notes/deprecation.rst > > +++ b/doc/guides/rel_notes/deprecation.rst > > @@ -62,3 +62,6 @@ Deprecation Notices > >PMDs that implement the latter. > >Target release for removal of the legacy API will be defined once most > >PMDs have switched to rte_flow. > > + > > +* The architecture TILE-Gx and the associated mpipe driver are not > > + maintained and will be removed in 17.05. > > The notification should probably be for removal in 17.08, to one release > cycle to respond/fix/update. > > It would also be good to hear from EZChip/Mellanox on this. > > John If - as seems likely - this deprecation notice makes it into the 17.02 release, I would just ask that the actual removal of the code *not* take place at the start of the 17.05 release cycle, to be sure to give further additional grace period for someone to respond to this. I think it is only fair to wait till just before the RC's are produced to remove the code, given it is such a drastic step, and the official notice of removal is only inserted right before 17.02 goes out. Regards, /Bruce
[dpdk-dev] [PATCH 1/2] net/ena: remove redundant variable
Signed-off-by: Yong Wang
---
drivers/net/ena/base/ena_com.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index bd6f3c6..39356d2 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -2242,7 +2242,6 @@ int ena_com_set_hash_ctrl(struct ena_com_dev *ena_dev)
{
struct ena_com_admin_queue *admin_queue = &ena_dev->admin_queue;
struct ena_rss *rss = &ena_dev->rss;
- struct ena_admin_feature_rss_hash_control *hash_ctrl = rss->hash_ctrl;
struct ena_admin_set_feat_cmd cmd;
struct ena_admin_set_feat_resp resp;
int ret;
@@ -2269,7 +2268,8 @@ int ena_com_set_hash_ctrl(struct ena_com_dev *ena_dev)
ena_trc_err("memory address set failed\n");
return ret;
}
- cmd.control_buffer.length = sizeof(*hash_ctrl);
+ cmd.control_buffer.length =
+ sizeof(struct ena_admin_feature_rss_hash_control);
ret = ena_com_execute_admin_command(admin_queue,
(struct ena_admin_aq_entry *)&cmd,
--
1.8.3.1
[dpdk-dev] [PATCH 2/2] net/ena: fix return of hash control flushing
In function ena_com_set_hash_ctrl(), the return value is assigned to
"ret" variable, but it is not returned. Fix it by adding the return.
Signed-off-by: Yong Wang
---
drivers/net/ena/base/ena_com.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 39356d2..38a0587 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -2278,7 +2278,7 @@ int ena_com_set_hash_ctrl(struct ena_com_dev *ena_dev)
sizeof(resp));
if (unlikely(ret)) {
ena_trc_err("Failed to set hash input. error: %d\n", ret);
- ret = ENA_COM_INVAL;
+ return ENA_COM_INVAL;
}
return 0;
--
1.8.3.1
Re: [dpdk-dev] [PATCH] doc: deprecation note for renaming vfio symbols for exporting
On 2/13/2017 12:01 PM, Shreyansh Jain wrote: > Some vfio symbols need to be exported outside librte_eal. For that, they > need to be renamed to rte_* naming convention. > > Signed-off-by: Shreyansh Jain Acked-by: Ferruh Yigit
Re: [dpdk-dev] [PATCH] doc: announce kni_vhost removal
On Tue, Feb 14, 2017 at 11:48:29AM +0100, Thomas Monjalon wrote: > 2016-11-17 13:27, Ferruh Yigit: > > +* kni: Remove :ref:`kni_vhost_backend-label` feature (KNI_VHOST) in 17.05 > > release. > > + :doc:`Vhost Library ` is currently preferred > > method for > > + guest - host communication. Just for clarification, this is not to > > remove KNI > > + or VHOST feature, but KNI_VHOST which is a KNI feature enabled via a > > compile > > + time option, and disabled by default. > > Acked-by: Thomas Monjalon Acked-by: Bruce Richardson
Re: [dpdk-dev] [PATCH] doc: add ABI change notification for ring library
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Olivier Matz > Sent: Tuesday, February 14, 2017 2:34 AM > To: Bruce Richardson > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: add ABI change notification for ring > library > > On Mon, 13 Feb 2017 17:38:30 +, Bruce Richardson > wrote: > > Document proposed changes for the rings code in the next release. > > > > Signed-off-by: Bruce Richardson > > Acked-by: Olivier Matz Acked-by: Hemant Agrawal
Re: [dpdk-dev] cryptodev - Session and queue pair relationship
Hi Akhil, Thanks for your feedback Declan, The suggestion from Fiona looks good. Should I send the patch for this or is it already in discussion in some different thread? Also, if this new API is added, there would be corresponding change in the ipsec-secgw application as well. I am looking forward to see those changes in ipsec-secgw. Mostly I am interesting in seeing how you plan to allocate resources (queue_pairs) when having multiple cores/workers. Regards, Sergio This API should be optional and underlying implementation may or may not implement this API. Regards, Akhil
[dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
Hi, We (trex traffic generator project) upgrade DPDK version from 16.07 to 17.02arc-3 and we experienced a performance degradation on the following NICS: XL710 : 10-15% ixgbe : 8% in one case mlx5: 8% 2 case X710: no impact (same driver as XL710) VIC : no impact It might be related to DPDK infra change as it affects more than one driver (maybe mbuf?). Wanted to know if this is expected before investing more into this. The Y axis numbers in all the following charts (from Grafana) are MPPS/core which means how many cycles per packet the CPU invest. Bigger MPPS/core means less CPU cycles to send packet. Labeles: trex-08 (XL710) trex-09 (X710) trex-07 (mlx5) kiwi02 (ixgbe) Grafanna charts can be seen here: (we can't share pointers to Grafanna) https://snag.gy/wGYp3c.jpg More information on the tests and setups can be found in this document https://trex-tgn.cisco.com/trex/doc/trex_analytics.html link to the project https://github.com/cisco-system-traffic-generator/trex-core Thanks, Hanoh
Re: [dpdk-dev] [PATCH v2] doc: fix unreadable images
> > No reply. What should I do? Apply with s/Contanier/Container/? There is an update on the way. Very shortly.
Re: [dpdk-dev] [PATCH] doc: announce kni_vhost removal
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson > Sent: Tuesday, February 14, 2017 11:42 AM > To: Thomas Monjalon > Cc: Yigit, Ferruh; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: announce kni_vhost removal > > On Tue, Feb 14, 2017 at 11:48:29AM +0100, Thomas Monjalon wrote: > > 2016-11-17 13:27, Ferruh Yigit: > > > +* kni: Remove :ref:`kni_vhost_backend-label` feature (KNI_VHOST) in > 17.05 release. > > > + :doc:`Vhost Library ` is currently preferred > method for > > > + guest - host communication. Just for clarification, this is not to > remove KNI > > > + or VHOST feature, but KNI_VHOST which is a KNI feature enabled via > a compile > > > + time option, and disabled by default. > > > > Acked-by: Thomas Monjalon > > Acked-by: Bruce Richardson Acked-by: Pablo de Lara
[dpdk-dev] [PATCH v2] doc/contributing: add description of review tags
This commit details what is meant by the various email tags that the DPDK community use regularly. The descriptions state what each tag means, drawing from the kernel's understanding[1], and the discussion on the DPDK mailing list[2]. Signed-off-by: Harry van Haaren Acked-by: John McNamara [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-and-cc [2] http://dpdk.org/ml/archives/dev/2017-January/thread.html#56300 --- doc/guides/contributing/patches.rst | 62 +++-- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst index a6b2753..4fe9025 100644 --- a/doc/guides/contributing/patches.rst +++ b/doc/guides/contributing/patches.rst @@ -225,13 +225,9 @@ Here are some guidelines for the body of a commit message: * Use correct capitalization, punctuation and spelling. -In addition to the ``Signed-off-by:`` name the commit messages can also have one or more of the following: - -* ``Reported-by:`` The reporter of the issue. -* ``Tested-by:`` The tester of the change. -* ``Reviewed-by:`` The reviewer of the change. -* ``Suggested-by:`` The person who suggested the change. -* ``Acked-by:`` When a previous version of the patch was acked and the ack is still relevant. +In addition to the ``Signed-off-by:`` name the commit messages can also have +tags for who reported, suggested, tested and reviewed the patch being +posted. Please refer to the `Tested, Acked and Reviewed by`_ section. Creating Patches @@ -427,9 +423,57 @@ The options ``--annotate`` and ``confirm = always`` are recommended for checking The Review Process -- -The more work you put into the previous steps the easier it will be to get a patch accepted. +Patches are reviewed by the community, relying on the experience and +collaboration of the members to double-check each other's work. There are a +number of ways to indicate that you have checked a patch on the mailing list. + + +Tested, Acked and Reviewed by +~ + +To indicate that you have interacted with a patch on the mailing list you +should respond to the patch in an email with one of the following tags: + + * Reviewed-by: + * Acked-by: + * Tested-by: + * Reported-by: + * Suggested-by: + +The tag should be on a separate line as follows:: + + tag-here: Name Surname + +Each of these tags has a specific meaning. In general, the DPDK community +follows the kernel usage of the tags. A short summary of the meanings of each +tag is given here for reference: + +.. _statement: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#reviewer-s-statement-of-oversight + +``Reviewed-by:`` is a strong statement_ that the patch is an appropriate state +for merging without any remaining serious technical issues. Reviews from +community members who are known to understand the subject area and to perform +thorough reviews will increase the likelihood of the patch getting merged. + +``Acked-by:`` is a record that the person named was not directly involved in +the preparation of the patch but wishes to signify and record their acceptance +and approval of it. + +``Tested-by:`` indicates that the patch has been successfully tested (in some +environment) by the person named. + +``Reported-by:`` is used to acknowledge person who found or reported the bug. + +``Suggested-by:`` indicates that the patch idea was suggested by the named +person. + + + +Steps to getting your patch merged +~~ -The general cycle for patch review and acceptance is: +The more work you put into the previous steps the easier it will be to get a +patch accepted. The general cycle for patch review and acceptance is: #. Submit the patch. -- 2.7.4
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
> -Original Message- > From: Richardson, Bruce > Sent: Tuesday, February 14, 2017 11:25 AM > To: Mcnamara, John > Cc: Thomas Monjalon ; Zhigang Lu > ; Liming Sun ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal > > On Tue, Feb 14, 2017 at 08:50:07AM +, Mcnamara, John wrote: > > > > > > > -Original Message- > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > > > Sent: Monday, February 13, 2017 10:53 AM > > > To: Zhigang Lu ; Liming Sun > > > Cc: dev@dpdk.org > > > Subject: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal > > > > > > Signed-off-by: Thomas Monjalon > > > --- > > > doc/guides/rel_notes/deprecation.rst | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/doc/guides/rel_notes/deprecation.rst > > > b/doc/guides/rel_notes/deprecation.rst > > > index b49e0a0..d40b137 100644 > > > --- a/doc/guides/rel_notes/deprecation.rst > > > +++ b/doc/guides/rel_notes/deprecation.rst > > > @@ -62,3 +62,6 @@ Deprecation Notices > > >PMDs that implement the latter. > > >Target release for removal of the legacy API will be defined once > most > > >PMDs have switched to rte_flow. > > > + > > > +* The architecture TILE-Gx and the associated mpipe driver are not > > > + maintained and will be removed in 17.05. > > > > The notification should probably be for removal in 17.08, to one release > cycle to respond/fix/update. > > > > It would also be good to hear from EZChip/Mellanox on this. > > > > John > > If - as seems likely - this deprecation notice makes it into the 17.02 > release, I would just ask that the actual removal of the code *not* take > place at the start of the 17.05 release cycle, to be sure to give further > additional grace period for someone to respond to this. I think it is only > fair to wait till just before the RC's are produced to remove the code, > given it is such a drastic step, and the official notice of removal is > only inserted right before 17.02 goes out. > Hi, That seems like a fair compromise. John
Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hanoch Haim (hhaim) > Sent: Tuesday, February 14, 2017 11:45 AM > To: dev@dpdk.org > Cc: Ido Barnea (ibarnea) ; Hanoch Haim (hhaim) > > Subject: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10% > > Hi, > > We (trex traffic generator project) upgrade DPDK version from 16.07 to > 17.02arc-3 and we experienced a performance degradation on the following > NICS: > > XL710 : 10-15% > ixgbe : 8% in one case > mlx5: 8% 2 case > X710: no impact (same driver as XL710) > VIC : no impact > > It might be related to DPDK infra change as it affects more than one > driver (maybe mbuf?). > Wanted to know if this is expected before investing more into this. The Y > axis numbers in all the following charts (from Grafana) are MPPS/core > which means how many cycles per packet the CPU invest. Bigger MPPS/core > means less CPU cycles to send packet. Hi, Thanks for the update. From a quick check with the Intel test team they haven't seen this. They would have flagged it if they had. Maybe someone from Mellanox/Netronome could confirm as well. Could you do a git-bisect to identify the change that caused this? John
Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
Hi John, thank you for the fast response. I assume that Intel tests are more like rx->tx tests. In our case we are doing mostly tx, which is more similar to dpdk-pkt-gen The cases that we cached the mbuf was affected the most. We expect to see the same issue with a simple DPDK application Thanks, Hanoh -Original Message- From: Mcnamara, John [mailto:john.mcnam...@intel.com] Sent: Tuesday, February 14, 2017 2:19 PM To: Hanoch Haim (hhaim); dev@dpdk.org Cc: Ido Barnea (ibarnea) Subject: RE: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10% > -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Hanoch Haim > (hhaim) > Sent: Tuesday, February 14, 2017 11:45 AM > To: dev@dpdk.org > Cc: Ido Barnea (ibarnea) ; Hanoch Haim (hhaim) > > Subject: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10% > > Hi, > > We (trex traffic generator project) upgrade DPDK version from 16.07 to > 17.02arc-3 and we experienced a performance degradation on the > following > NICS: > > XL710 : 10-15% > ixgbe : 8% in one case > mlx5: 8% 2 case > X710: no impact (same driver as XL710) > VIC : no impact > > It might be related to DPDK infra change as it affects more than one > driver (maybe mbuf?). > Wanted to know if this is expected before investing more into this. > The Y axis numbers in all the following charts (from Grafana) are > MPPS/core which means how many cycles per packet the CPU invest. > Bigger MPPS/core means less CPU cycles to send packet. Hi, Thanks for the update. From a quick check with the Intel test team they haven't seen this. They would have flagged it if they had. Maybe someone from Mellanox/Netronome could confirm as well. Could you do a git-bisect to identify the change that caused this? John
Re: [dpdk-dev] [PATCH] doc: add EAL bus support in release notes
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shreyansh Jain > Sent: Monday, February 13, 2017 5:30 AM > To: dev@dpdk.org > Cc: thomas.monja...@6wind.com; Shreyansh Jain > > Subject: [dpdk-dev] [PATCH] doc: add EAL bus support in release notes > > Signed-off-by: Shreyansh Jain > --- > doc/guides/rel_notes/release_17_02.rst | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/doc/guides/rel_notes/release_17_02.rst > b/doc/guides/rel_notes/release_17_02.rst > index ec9143c..b5bf0a9 100644 > --- a/doc/guides/rel_notes/release_17_02.rst > +++ b/doc/guides/rel_notes/release_17_02.rst > @@ -237,6 +237,16 @@ Resolved Issues > EAL > ~~~ > > +* **Added support for representing buses in EAL** > + > + A new structure ``rte_bus`` is introduced in EAL. This allows for > + devices to be represented by buses they are connected to. A new bus > + can be added to DPDK by extending the ``rte_bus`` structure and > + implementing the scan and probe functions. Once a new bus is > + registered using provided APIs, new devices can be detected and > initialized using bus scan and probe callbacks. > + > + With this change, devices other than PCI or VDEV type can also be > + represented in DPDK framework. > > Drivers > ~~~ > -- > 2.7.4 Acked-by: Hemant Agrawal
Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
2017-02-14 12:19, Mcnamara, John: > From: Hanoch Haim (hhaim) > > We (trex traffic generator project) upgrade DPDK version from 16.07 to > > 17.02arc-3 and we experienced a performance degradation on the following > > NICS: [...] > > Could you do a git-bisect to identify the change that caused this? +1 for the bisect. Please could you try to isolate the change causing the regression?
Re: [dpdk-dev] [PATCH v3] doc: fix unreadable images
> -Original Message-
> From: Van Haaren, Harry
> Sent: Tuesday, February 14, 2017 11:56 AM
> To: Mcnamara, John
> Cc: thomas.monja...@6wind.com; dev@dpdk.org; Tan, Jianfeng
> ; Van Haaren, Harry
> Subject: [PATCH v3] doc: fix unreadable images
>
> From: Jianfeng Tan
>
> The images by below two commits are very unclear. Fix it.
>
> Fixes: 50665deebda ("doc: add guide to use virtio-user for container
> networking")
> Fixes: 0ba3870e755 ("doc: add guide to use virtio-user as exceptional
> path")
>
> Suggested-by: Thomas Monjalon
> Signed-off-by: Jianfeng Tan
> Signed-off-by: Harry van Haaren
Acked-by: John McNamara
Re: [dpdk-dev] [PATCH 0/3] doc: update release notes
2017-02-09 09:31, Nelio Laranjeiro: > Biggest change is in the first patch to provide a better visibility for users > on which NIC has been tested on which platform. > > Others two are just updates for Mellanox PMDs. > > Nelio Laranjeiro (3): > doc: merge release notes sections > doc: update release notes for mlx4 > doc: update release notes for mlx5 Applied, thanks
Re: [dpdk-dev] [PATCH] doc:add tested Intel platforms with Intel NICs.
> > Add tested Intel platforms with Intel NICs to the release note. > > > > Signed-off-by: Yulong Pei > > Thank you for the rework. > > Note for the future, the patch should have been v2 and the title should > have a space between "doc:" and "add". > > Otherwise: > > > Acked-by: John McNamara Applied, thanks
Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
Hi,
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, February 14, 2017 1:05 PM
> To: Hanoch Haim (hhaim)
> Cc: dev@dpdk.org; Mcnamara, John; Ido Barnea (ibarnea)
> Subject: Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
>
> 2017-02-14 12:19, Mcnamara, John:
> > From: Hanoch Haim (hhaim)
> > > We (trex traffic generator project) upgrade DPDK version from 16.07 to
> > > 17.02arc-3 and we experienced a performance degradation on the
> following
> > > NICS:
> [...]
> >
> > Could you do a git-bisect to identify the change that caused this?
>
> +1 for the bisect.
> Please could you try to isolate the change causing the regression?
I reproduced the regression for i40e driver (3-4% degradation, not 10%). The
commit that caused it was the following:
5ea0942129a4 ("net/i40e: add packet type metadata in vector Rx")
This commit was introduced in 16.11, and not in 17.02. This caused some
performance degradation
in the i40e vector RX, due to the addition of packet type parsing in the PMD.
Thanks,
Pablo
Re: [dpdk-dev] [PATCH] doc: announce TILE-Gx removal
On 02/14/2017 10:28 AM, Thomas Monjalon wrote: 2017-02-14 09:58, Thomas Monjalon: 2017-02-14 08:50, Mcnamara, John: The notification should probably be for removal in 17.08, to one release cycle to respond/fix/update. There were some previous messages/warnings and there were no answer. I have asked a working free toolchain, I did not have it. We have asked some questions about drivers support, no reply. I have asked about removal, no reply. I really do not see why we should keep it longer. Some public links: Recent warnings: http://dpdk.org/ml/archives/dev/2017-February/056981.html Recent patch from Jerin without any comment: http://dpdk.org/commit/dff9071 Bug in 16.04 without any comment: https://mail-archive.com/dev@dpdk.org/msg31596.html Work stopped in March, last year: https://mail-archive.com/dev@dpdk.org/msg43536.html No update for free toolchain: https://mail-archive.com/dev@dpdk.org/msg42651.html One year without any news is enough. Hope it convinces you. It does for me: Acked-by: Maxime Coquelin As suggested by Bruce, let's remove it just before v17.05-rc1 to let some time, even if they would already have replied in case they had a plan for maintaining it. Thanks, Maxime
Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Tuesday, February 14, 2017 1:28 PM
> To: Thomas Monjalon; Hanoch Haim (hhaim)
> Cc: dev@dpdk.org; Mcnamara, John; Ido Barnea (ibarnea)
> Subject: Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of ~10%
>
> Hi,
>
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas
> Monjalon
> > Sent: Tuesday, February 14, 2017 1:05 PM
> > To: Hanoch Haim (hhaim)
> > Cc: dev@dpdk.org; Mcnamara, John; Ido Barnea (ibarnea)
> > Subject: Re: [dpdk-dev] DPDK 17.02 RC-3 performance degradation of
> ~10%
> >
> > 2017-02-14 12:19, Mcnamara, John:
> > > From: Hanoch Haim (hhaim)
> > > > We (trex traffic generator project) upgrade DPDK version from 16.07
> to
> > > > 17.02arc-3 and we experienced a performance degradation on the
> > following
> > > > NICS:
> > [...]
> > >
> > > Could you do a git-bisect to identify the change that caused this?
> >
> > +1 for the bisect.
> > Please could you try to isolate the change causing the regression?
>
> I reproduced the regression for i40e driver (3-4% degradation, not 10%).
> The commit that caused it was the following:
> 5ea0942129a4 ("net/i40e: add packet type metadata in vector Rx")
>
> This commit was introduced in 16.11, and not in 17.02. This caused some
> performance degradation
> in the i40e vector RX, due to the addition of packet type parsing in the
> PMD.
>
> Thanks,
> Pablo
Sorry, I was talking about RX and not TX, so this is probably not valid then.
Pablo
Re: [dpdk-dev] [PATCH] doc: announce API/ABI changes for vhost
Hi Yuanhan, On 01/23/2017 02:04 PM, Yuanhan Liu wrote: I made a vhost ABI/API refactoring at v16.04, meant to avoid such issue forever. Well, apparently, I lied. People are looking for more vhost-user options now days, other than vhost-user net only. For example, SPDK (Storage Performance Development Kit) are looking for chance of vhost-user SCSI and vhost-user block. Apparently, they also need a vhost-user backend, while DPDK already has a (mature enough) backend, they don't want to implement it again from scratch. They want to leverage the one DPDK provides. However, the last refactoring hasn't done that right, at least it's not friendly for extending vhost-user to add more devices support. For example, different virtio devices has its own feature set, while APIs like rte_vhost_feature_disable(feature_mask) have no option to tell the device type. Thus, a more proper API should look like: rte_vhost_feature_disable(device_type, feature_mask); I wonder if we could also change it to be per-instance, instead of disabling features globally: rte_vhost_feature_disable(vid, device_type, feature_mask); It could be useful for live-migration with different backend versions on the hosts, as it would allow to run instances with different compat modes (like running vhost's DPDK v17.08 with v17.05-only supported features). I made a proposal about cross-version migration, but we are far from a conclusion on the design. Besides that, few public files and structures should be renamed, to not let it bind to virtio-net. Specifically, they are: - virtio_net_device_ops --> vhost_device_ops - rte_virtio_net.h --> rte_vhost.h Signed-off-by: Yuanhan Liu Anyway, the change you propose is necessary: Acked-by: Maxime Coquelin Thanks, Maxime
[dpdk-dev] [PATCH] doc: introduce tested platforms as combinations
Add some text and rearrange lists to make sure it is clear that the tested platforms listed in the release notes are some combinations of the items in each group. Signed-off-by: Thomas Monjalon --- doc/guides/rel_notes/release_17_02.rst | 104 +++-- 1 file changed, 34 insertions(+), 70 deletions(-) diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst index 817f8f3..836cbfe 100644 --- a/doc/guides/rel_notes/release_17_02.rst +++ b/doc/guides/rel_notes/release_17_02.rst @@ -394,77 +394,43 @@ The libraries prepended with a plus sign were incremented in this version. Tested Platforms -.. This section should contain a list of systems (properties, NIC, OS) that were tested with this release. +.. This section should contain a list of platforms that were tested with this release. The format is: - #. (Intel, IBM, ARM,...) platform with NICs. + * platform with combinations - * Platform details. - -* Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz -* Intel(R) Xeon(R) CPU E5-2697 v3 @ 2.70GHz -* Intel(R) Xeon(R) CPU E5-2699 v3 @ 2.30GHz - - * Intel(R) Server board S2600WTT - * BIOS: SE5C610.86B.01.01.0005.101720141054 - -* ... - -* OS - - * CentOS 7.0 - * Fedora 23 - * Fedora 24 - * FreeBSD 10.3 - * Red Hat Enterprise Linux 7.2 - * SUSE Enterprise Linux 12 - * Ubuntu 15.10 - * Ubuntu 16.04 LTS - * Wind River Linux 8 - * ... - - * List of NICs - -* Intel(R) Ethernet Controller X540-AT2 - - * Firmware version: 0x8389 - * Device id (pf): 8086:1528 - * Driver version: 3.23.2 (ixgbe) - * ... - -* Intel(R) 82599ES 10 Gigabit Ethernet Controller - - * Firmware version: 0x61bf0001 - * Device id (pf/vf): 8086:10fb / 8086:10ed - * Driver version: 4.0.1-k (ixgbe) - -* ... + * List of CPU + * List of OS + * List of devices This section is a comment. do not overwrite or remove it. Also, make sure to start the actual text at the margin. = -#. Intel(R) platforms with Mellanox(R) NICs. +This release has been tested with the below list of CPU/device/firmware/OS. +Each section describes a different set of combinations. + +* Intel(R) platforms with Mellanox(R) NICs combinations - * Platform details. + * Platform details * Intel(R) Xeon(R) CPU E5-2697 v2 @ 2.70GHz * Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz * Intel(R) Xeon(R) CPU E5-2697 v3 @ 2.60GHz - * OS: + * OS: - * CentOS 7.0 - * Fedora 23 - * Fedora 24 - * FreeBSD 10.3 - * Red Hat Enterprise Linux 7.2 - * SUSE Enterprise Linux 12 - * Ubuntu 14.04 LTS - * Ubuntu 15.10 - * Ubuntu 16.04 LTS - * Wind River Linux 8 + * CentOS 7.0 + * Fedora 23 + * Fedora 24 + * FreeBSD 10.3 + * Red Hat Enterprise Linux 7.2 + * SUSE Enterprise Linux 12 + * Ubuntu 14.04 LTS + * Ubuntu 15.10 + * Ubuntu 16.04 LTS + * Wind River Linux 8 * MLNX_OFED: 4.0-1.0.1.0 @@ -566,7 +532,7 @@ Tested Platforms * Device ID: 15b3:1019 * Firmware version: 16.18.1000 -#. IBM(R) Power8(R) with Mellanox(R) NICs. +* IBM(R) Power8(R) with Mellanox(R) NICs combinations * Machine: @@ -575,9 +541,7 @@ Tested Platforms * type-model: 8247-22L * Firmware FW810.21 (SV810_108) - * OS: - - * Ubuntu 16.04 LTS PPC le + * OS: Ubuntu 16.04 LTS PPC le * MLNX_OFED: 4.0-1.0.1.0 @@ -667,9 +631,9 @@ Tested Platforms * Device ID: 15b3:1017 * Firmware version: 16.18.1000 -#. Intel(R) platforms with Intel(R) NICs. +* Intel(R) platforms with Intel(R) NICs combinations - * Platform details. + * Platform details * Intel(R) Atom(TM) CPU C2758 @ 2.40GHz * Intel(R) Xeon(R) CPU D-1540 @ 2.00GHz @@ -679,16 +643,16 @@ Tested Platforms * Intel(R) Xeon(R) CPU E5-2695 v4 @ 2.10GHz * Intel(R) Xeon(R) CPU E5-2658 v2 @ 2.40GHz - * OS: + * OS: - * CentOS 7.2 - * Fedora 25 - * FreeBSD 11 - * Red Hat Enterprise Linux Server release 7.3 - * SUSE Enterprise Linux 12 - * Wind River Linux 8 - * Ubuntu 16.04 - * Ubuntu 16.10 + * CentOS 7.2 + * Fedora 25 + * FreeBSD 11 + * Red Hat Enterprise Linux Server release 7.3 + * SUSE Enterprise Linux 12 + * Wind River Linux 8 + * Ubuntu 16.04 + * Ubuntu 16.10 * NICs: -- 2.7.0
Re: [dpdk-dev] [PATCH] doc: introduce tested platforms as combinations
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] > Sent: Tuesday, February 14, 2017 2:06 PM > To: Mcnamara, John > Cc: dev@dpdk.org; Pei, Yulong ; Nelio Laranjeiro > > Subject: [PATCH] doc: introduce tested platforms as combinations > > Add some text and rearrange lists to make sure it is clear that the tested > platforms listed in the release notes are some combinations of the items > in each group. > > Signed-off-by: Thomas Monjalon Acked-by: John McNamara
Re: [dpdk-dev] [PATCH] doc: add EAL bus support in release notes
2017-02-14 12:56, Hemant Agrawal: > > Signed-off-by: Shreyansh Jain > > @@ -237,6 +237,16 @@ Resolved Issues > > EAL > > ~~~ > > > > +* **Added support for representing buses in EAL** > > + > > + A new structure ``rte_bus`` is introduced in EAL. This allows for > > + devices to be represented by buses they are connected to. A new bus > > + can be added to DPDK by extending the ``rte_bus`` structure and > > + implementing the scan and probe functions. Once a new bus is > > + registered using provided APIs, new devices can be detected and > > initialized using bus scan and probe callbacks. > > + > > + With this change, devices other than PCI or VDEV type can also be > > + represented in DPDK framework. > > > Acked-by: Hemant Agrawal It was in "Resolved Issues". Moved and applied, thanks
Re: [dpdk-dev] [PATCH] doc: introduce tested platforms as combinations
> > Add some text and rearrange lists to make sure it is clear that the tested > > platforms listed in the release notes are some combinations of the items > > in each group. > > > > Signed-off-by: Thomas Monjalon > > Acked-by: John McNamara Applied
Re: [dpdk-dev] [PATCH v3] doc: fix unreadable images
2017-02-14 13:14, Mcnamara, John:
> > -Original Message-
> > From: Van Haaren, Harry
> > Sent: Tuesday, February 14, 2017 11:56 AM
> > To: Mcnamara, John
> > Cc: thomas.monja...@6wind.com; dev@dpdk.org; Tan, Jianfeng
> > ; Van Haaren, Harry
> > Subject: [PATCH v3] doc: fix unreadable images
> >
> > From: Jianfeng Tan
> >
> > The images by below two commits are very unclear. Fix it.
> >
> > Fixes: 50665deebda ("doc: add guide to use virtio-user for container
> > networking")
> > Fixes: 0ba3870e755 ("doc: add guide to use virtio-user as exceptional
> > path")
> >
> > Suggested-by: Thomas Monjalon
> > Signed-off-by: Jianfeng Tan
> > Signed-off-by: Harry van Haaren
>
>
> Acked-by: John McNamara
Applied, thanks
[dpdk-dev] [PATCH] net/mlx5: add out of buffer counter to extended statistic
This commit adds RX out of buffer counter to xstats report.
The counter counts the number of dropped occurred due to lack of buffers
on device RX queues.
Signed-off-by: Shahaf Shuler
Acked-by: Nelio Laranjeiro
---
drivers/net/mlx5/mlx5.h| 2 ++
drivers/net/mlx5/mlx5_ethdev.c | 54 ++
drivers/net/mlx5/mlx5_stats.c | 18 +++---
3 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 879da5e..2b4345a 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -197,6 +197,8 @@ struct mlx5_secondary_data {
int mlx5_is_secondary(void);
int priv_get_ifname(const struct priv *, char (*)[IF_NAMESIZE]);
int priv_ifreq(const struct priv *, int req, struct ifreq *);
+int priv_is_ib_cntr(const char *);
+int priv_get_cntr_sysfs(struct priv *, const char *, uint64_t *);
int priv_get_num_vfs(struct priv *, uint16_t *);
int priv_get_mtu(struct priv *, uint16_t *);
int priv_set_flags(struct priv *, unsigned int, unsigned int);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 2145965..6b64f44 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -234,6 +234,23 @@ struct priv *
}
/**
+ * Check if the counter is located on ib counters file.
+ *
+ * @param[in] cntr
+ * Counter name.
+ *
+ * @return
+ * 1 if counter is located on ib counters file , 0 otherwise.
+ */
+int
+priv_is_ib_cntr(const char *cntr)
+{
+ if (!strcmp(cntr, "out_of_buffer"))
+ return 1;
+ return 0;
+}
+
+/**
* Read from sysfs entry.
*
* @param[in] priv
@@ -260,10 +277,15 @@ struct priv *
if (priv_get_ifname(priv, &ifname))
return -1;
- MKSTR(path, "%s/device/net/%s/%s", priv->ctx->device->ibdev_path,
- ifname, entry);
-
- file = fopen(path, "rb");
+ if (priv_is_ib_cntr(entry)) {
+ MKSTR(path, "%s/ports/1/hw_counters/%s",
+ priv->ctx->device->ibdev_path, entry);
+ file = fopen(path, "rb");
+ } else {
+ MKSTR(path, "%s/device/net/%s/%s",
+ priv->ctx->device->ibdev_path, ifname, entry);
+ file = fopen(path, "rb");
+ }
if (file == NULL)
return -1;
ret = fread(buf, 1, size, file);
@@ -469,6 +491,30 @@ struct priv *
}
/**
+ * Read device counter from sysfs.
+ *
+ * @param priv
+ * Pointer to private structure.
+ * @param name
+ * Counter name.
+ * @param[out] cntr
+ * Counter output buffer.
+ *
+ * @return
+ * 0 on success, -1 on failure and errno is set.
+ */
+int
+priv_get_cntr_sysfs(struct priv *priv, const char *name, uint64_t *cntr)
+{
+ unsigned long ulong_ctr;
+
+ if (priv_get_sysfs_ulong(priv, name, &ulong_ctr) == -1)
+ return -1;
+ *cntr = ulong_ctr;
+ return 0;
+}
+
+/**
* Set device MTU.
*
* @param priv
diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
index 20c957e..a48ebea 100644
--- a/drivers/net/mlx5/mlx5_stats.c
+++ b/drivers/net/mlx5/mlx5_stats.c
@@ -125,6 +125,10 @@ struct mlx5_counter_ctrl {
.dpdk_name = "tx_errors_phy",
.ctr_name = "tx_errors_phy",
},
+ {
+ .dpdk_name = "rx_out_of_buffer",
+ .ctr_name = "out_of_buffer",
+ },
};
static const unsigned int xstats_n = RTE_DIM(mlx5_counters_init);
@@ -159,9 +163,15 @@ struct mlx5_counter_ctrl {
WARN("unable to read statistic values from device");
return -1;
}
- for (i = 0; i != xstats_n; ++i)
- stats[i] = (uint64_t)
- et_stats->data[xstats_ctrl->dev_table_idx[i]];
+ for (i = 0; i != xstats_n; ++i) {
+ if (priv_is_ib_cntr(mlx5_counters_init[i].ctr_name))
+ priv_get_cntr_sysfs(priv,
+ mlx5_counters_init[i].ctr_name,
+ &stats[i]);
+ else
+ stats[i] = (uint64_t)
+ et_stats->data[xstats_ctrl->dev_table_idx[i]];
+ }
return 0;
}
@@ -233,6 +243,8 @@ struct mlx5_counter_ctrl {
}
}
for (j = 0; j != xstats_n; ++j) {
+ if (priv_is_ib_cntr(mlx5_counters_init[i].ctr_name))
+ continue;
if (xstats_ctrl->dev_table_idx[j] >= dev_stats_n) {
WARN("counter \"%s\" is not recognized",
mlx5_counters_init[j].dpdk_name);
--
1.8.3.1
[dpdk-dev] [PATCH 1/2] Fix container_of() macro to work with const members
This fixes the usage of structure members that are declared const to get
a pointer to the embedding parent structure.
Signed-off-by: Jan Blunck
---
lib/librte_eal/common/include/rte_common.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_common.h
b/lib/librte_eal/common/include/rte_common.h
index 8dda3e2..c421708 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -347,8 +347,9 @@ rte_bsf32(uint32_t v)
*/
#ifndef container_of
#define container_of(ptr, type, member)__extension__ ({
\
- typeof(((type *)0)->member) *_ptr = (ptr); \
- (type *)(((char *)_ptr) - offsetof(type, member)); })
+ const typeof(((type *)0)->member) *_ptr = (ptr); \
+ (type *)(((uintptr_t)_ptr) - offsetof(type, member)); \
+ })
#endif
#define _RTE_STR(x) #x
--
2.7.4
[dpdk-dev] [PATCH 2/2] Ensure constness of target pointer type
This adds a check to ensure that the container_of() macro is not used to
cast away (remove) constness.
Signed-off-by: Jan Blunck
---
lib/librte_eal/common/include/rte_common.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/librte_eal/common/include/rte_common.h
b/lib/librte_eal/common/include/rte_common.h
index c421708..e057f6e 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -348,6 +348,8 @@ rte_bsf32(uint32_t v)
#ifndef container_of
#define container_of(ptr, type, member)__extension__ ({
\
const typeof(((type *)0)->member) *_ptr = (ptr); \
+ __attribute__((unused)) type *_target_ptr = \
+ (type *)(ptr); \
(type *)(((uintptr_t)_ptr) - offsetof(type, member)); \
})
#endif
--
2.7.4
Re: [dpdk-dev] crypto drivers in the API
On 14/02/2017 11:04 AM, Thomas Monjalon wrote: 2017-02-14 10:44, Doherty, Declan: On 13/02/2017 1:25 PM, Thomas Monjalon wrote: In the crypto API, the drivers are listed. In my opinion, it is a wrong designed and these lists should be removed. Do we need a deprecation notice to plan this removal in 17.05, while working on bus abstraction? ... ... Yes If you were planning to do this, you should have sent a deprecation notice few weeks ago. Please send it now and we'll see if we have enough supporters shortly. Thomas, there are a couple of other changes we are looking at in the cryptodev which would require API changes as well as break ABI including adding support for a multi-device sessions, and changes to crypto operation layout and field changes for performance but these but will require RFCs or at least more discussion of the proposals. Given the time constrains for the V1 deadline for 17.05 I would prefer to work on the RFCs and get them out as soon as possible over the next few weeks and then make all the ABI breaking changes in R17.08 in a single release. Otherwise we will end up breaking ABI 2 release in a row which I would like to avoid if possible.
[dpdk-dev] [PATCH] doc: add thread-safety information about EFD library
Signed-off-by: Pablo de Lara --- doc/guides/prog_guide/efd_lib.rst | 15 +++ lib/librte_efd/rte_efd.h | 6 ++ 2 files changed, 21 insertions(+) diff --git a/doc/guides/prog_guide/efd_lib.rst b/doc/guides/prog_guide/efd_lib.rst index 5b8e4e3..3f90fa9 100644 --- a/doc/guides/prog_guide/efd_lib.rst +++ b/doc/guides/prog_guide/efd_lib.rst @@ -270,6 +270,11 @@ failed to find a suitable perfect hash or the group was full). The function will return ``EFD_UPDATE_NO_CHANGE (3)`` if there is no change to the EFD table (i.e, same value already exists). +.. Note:: + + This function is not multi-thread safe and should only be called + from one thread. + EFD Lookup ~~ @@ -285,6 +290,11 @@ lookup function. ``rte_efd_lookup_bulk()`` is the bulk lookup function, that looks up num_keys simultaneously stored in the key_list and the corresponding return values will be returned in the value_list. +.. Note:: + + This function is multi-thread safe, but there should not be other threads + writing in the EFD table, unless locks are used. + EFD Delete ~~ @@ -295,6 +305,11 @@ use to lookup the existing value, which is ideally the caller's socket id. The previous value associated with this key will be returned in the prev_value argument. +.. Note:: + + This function is not multi-thread safe and should only be called + from one thread. + .. _Efd_internals: Library Internals diff --git a/lib/librte_efd/rte_efd.h b/lib/librte_efd/rte_efd.h index 1a1cb5b..6d31e18 100644 --- a/lib/librte_efd/rte_efd.h +++ b/lib/librte_efd/rte_efd.h @@ -198,6 +198,8 @@ rte_efd_find_existing(const char *name); * Computes an updated table entry for the supplied key/value pair. * The update is then immediately applied to the provided table and * all socket-local copies of the chunks are updated. + * This operation is not multi-thread safe + * and should only be called one from thread. * * @param table * EFD table to reference @@ -227,6 +229,8 @@ rte_efd_update(struct rte_efd_table *table, unsigned int socket_id, /** * Removes any value currently associated with the specified key from the table + * This operation is not multi-thread safe + * and should only be called from one thread. * * @param table * EFD table to reference @@ -247,6 +251,7 @@ rte_efd_delete(struct rte_efd_table *table, unsigned int socket_id, /** * Looks up the value associated with a key + * This operation is multi-thread safe. * * NOTE: Lookups will *always* succeed - this is a property of * using a perfect hash table. @@ -270,6 +275,7 @@ rte_efd_lookup(const struct rte_efd_table *table, unsigned int socket_id, /** * Looks up the value associated with several keys. + * This operation is multi-thread safe. * * NOTE: Lookups will *always* succeed - this is a property of * using a perfect hash table. -- 2.7.4
[dpdk-dev] [RFC 17.05] test: move tests to separate folder
Tests are part of app folder and compiled with library every time.
Moving tests into a "test" folder which won't be compiled by default.
To compile tests, need to give explicit "make test" command.
"make test" was previously used to run tests, which renamed to
"make test_run" with this patch.
This makes default compilation ~30% faster,
[clang, make -j8, old]: real1m04.355s
[clang, make -j8, new]: real0m41.740s
For new case, test needs to built separately, which takes,
[clang, make -j8 test]: real0m24.293s
The point is tests are not required always and by every one.
Signed-off-by: Ferruh Yigit
---
app/Makefile | 4 ---
mk/rte.sdkbuild.mk | 4 +--
mk/rte.sdkroot.mk | 9 +++--
mk/rte.sdktest.mk | 8 ++---
test/Makefile | 39 +
{app => test}/cmdline_test/Makefile| 0
{app => test}/cmdline_test/cmdline_test.c | 0
{app => test}/cmdline_test/cmdline_test.h | 0
{app => test}/cmdline_test/cmdline_test.py | 0
{app => test}/cmdline_test/cmdline_test_data.py| 0
{app => test}/cmdline_test/commands.c | 0
{app => test}/test-acl/Makefile| 0
{app => test}/test-acl/main.c | 0
{app => test}/test-pipeline/Makefile | 0
{app => test}/test-pipeline/config.c | 0
{app => test}/test-pipeline/init.c | 0
{app => test}/test-pipeline/main.c | 0
{app => test}/test-pipeline/main.h | 0
{app => test}/test-pipeline/pipeline_acl.c | 0
{app => test}/test-pipeline/pipeline_hash.c| 0
{app => test}/test-pipeline/pipeline_lpm.c | 0
{app => test}/test-pipeline/pipeline_lpm_ipv6.c| 0
{app => test}/test-pipeline/pipeline_stub.c| 0
{app => test}/test-pipeline/runtime.c | 0
{app => test}/test/Makefile| 0
{app => test}/test/autotest.py | 0
{app => test}/test/autotest_data.py| 0
{app => test}/test/autotest_runner.py | 0
{app => test}/test/autotest_test_funcs.py | 0
{app => test}/test/commands.c | 0
{app => test}/test/packet_burst_generator.c| 0
{app => test}/test/packet_burst_generator.h| 0
{app => test}/test/process.h | 0
{app => test}/test/resource.c | 0
{app => test}/test/resource.h | 0
{app => test}/test/test.c | 0
{app => test}/test/test.h | 0
{app => test}/test/test_acl.c | 0
{app => test}/test/test_acl.h | 0
{app => test}/test/test_alarm.c| 0
{app => test}/test/test_atomic.c | 0
{app => test}/test/test_byteorder.c| 0
{app => test}/test/test_cmdline.c | 0
{app => test}/test/test_cmdline.h | 0
{app => test}/test/test_cmdline_cirbuf.c | 0
{app => test}/test/test_cmdline_etheraddr.c| 0
{app => test}/test/test_cmdline_ipaddr.c | 0
{app => test}/test/test_cmdline_lib.c | 0
{app => test}/test/test_cmdline_num.c | 0
{app => test}/test/test_cmdline_portlist.c | 0
{app => test}/test/test_cmdline_string.c | 0
{app => test}/test/test_common.c | 0
{app => test}/test/test_cpuflags.c | 0
{app => test}/test/test_cryptodev.c| 0
{app => test}/test/test_cryptodev.h| 0
.../test/test_cryptodev_aes_test_vectors.h | 0
{app => test}/test/test_cryptodev_blockcipher.c| 0
{app => test}/test/test_cryptodev_blockcipher.h| 0
.../test/test_cryptodev_des_test_vectors.h | 0
.../test/test_cryptodev_gcm_test_vectors.h | 0
.../test/test_cryptodev_hash_test_vectors.h| 0
.../test/test_cryptodev_hmac_test_vectors.h| 0
.../test/test_cryptodev_kasumi_hash_test_vectors.h | 0
.../test/test_cryptodev_kasumi_test_vectors.h | 0
{app => test}/test/test_cryptodev_perf.c | 0
.../test/test_cryptodev_snow3g_hash_test_vectors.h | 0
.../test/test_cryptodev_snow3g_test_vectors.h | 0
.../test/test_cryptodev_zuc_hash_test_vectors.h| 0
.../test/test_cryptodev_zuc_test_vectors.h | 0
{app => test}/test/test_cycles.c | 0
{app => test}/test/test_debug.c| 0
{app => test}/test/test_devargs.c | 0
{app => test}/test/test_distributor.c | 0
{app => test}/test/test_distributor_perf.c | 0
{app => test}/test/test_eal_flags.c
[dpdk-dev] [PATCH v1] doc: update release notes for 17.02
Fix grammar, spelling and formatting of DPDK 17.02 release notes. Signed-off-by: John McNamara --- Note: The "ABI Changes" section is currently empty. doc/guides/rel_notes/release_17_02.rst | 255 ++--- 1 file changed, 111 insertions(+), 144 deletions(-) diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst index 6420a87..b7c188a 100644 --- a/doc/guides/rel_notes/release_17_02.rst +++ b/doc/guides/rel_notes/release_17_02.rst @@ -40,46 +40,47 @@ New Features * **Added support for representing buses in EAL** - A new structure ``rte_bus`` is introduced in EAL. This allows for devices to - be represented by buses they are connected to. A new bus can be added to - DPDK by extending the ``rte_bus`` structure and implementing the scan and - probe functions. Once a new bus is registered using provided APIs, new - devices can be detected and initialized using bus scan and probe callbacks. + The ``rte_bus`` structure was introduced into the EAL. This allows for + devices to be represented by buses they are connected to. A new bus can be + added to DPDK by extending the ``rte_bus`` structure and implementing the + scan and probe functions. Once a new bus is registered using the provided + APIs, new devices can be detected and initialized using bus scan and probe + callbacks. - With this change, devices other than PCI or VDEV type can also be represented - in DPDK framework. + With this change, devices other than PCI or VDEV type can be represented + in the DPDK framework. * **Added generic EAL API for I/O device memory read/write operations.** - This API introduces 8-bit, 16-bit, 32bit, 64bit I/O device - memory read/write operations along with the relaxed versions. + This API introduces 8 bit, 16 bit, 32 bit and 64 bit I/O device + memory read/write operations along with "relaxed" versions. - The weakly-ordered machine like ARM needs additional I/O barrier for - device memory read/write access over PCI bus. - By introducing the EAL abstraction for I/O device memory read/write access, - The drivers can access I/O device memory in architecture-agnostic manner. - The relaxed version does not have additional I/O memory barrier, useful in - accessing the device registers of integrated controllers which - implicitly strongly ordered with respect to memory access. + Weakly-ordered architectures like ARM need an additional I/O barrier for + device memory read/write access over PCI bus. By introducing the EAL + abstraction for I/O device memory read/write access, the drivers can access + I/O device memory in an architecture-agnostic manner. The relaxed version + does not have an additional I/O memory barrier, which is useful in accessing + the device registers of integrated controllers which is implicitly strongly + ordered with respect to memory access. * **Added generic flow API (rte_flow).** This API provides a generic means to configure hardware to match specific - ingress or egress traffic, alter its fate and query related counters + ingress or egress traffic, alter its behavior and query related counters according to any number of user-defined rules. - It is slightly higher-level than the legacy filtering framework which it - encompasses and supersedes (including all functions and filter types) in - order to expose a single interface with an unambiguous behavior that is - common to all poll-mode drivers (PMDs). + In order to expose a single interface with an unambiguous behavior that is + common to all poll-mode drivers (PMDs) the ``rte_flow`` API is slightly + higher-level than the legacy filtering framework, which it encompasses and + supersedes (including all functions and filter types) . See the :ref:`Generic flow API ` documentation for more information. * **Added firmware version get API.** - Added a new function ``rte_eth_dev_fw_version_get()`` to fetch firmware - version by a given device. + Added a new function ``rte_eth_dev_fw_version_get()`` to fetch the firmware + version for a given device. * **Added APIs for MACsec offload support to the ixgbe PMD.** @@ -90,54 +91,58 @@ New Features Added support for I219 Intel 1GbE NICs. -* **Added VF Daemon (VFD) on i40e. - EXPERIMENTAL** - - This's an EXPERIMENTAL feature to enhance the capability of DPDK PF as many - VF management features are not supported by kernel PF driver. - Some new private APIs are implemented in PMD without abstrction layer. - They can be used directly by some users who have the need. - - The new APIs to control VFs directly from PF include, - 1) set VF MAC anti-spoofing - 2) set VF VLAN anti-spoofing - 3) set TX loopback - 4) set VF unicast promiscuous mode - 5) set VF multicast promiscuous mode - 6) set VF MTU - 7) get/reset VF stats - 8) set VF MAC address - 9) set VF VLAN stripping - 10) VF VLAN insertion - 12) set VF broadcast mode - 13) set VF VLAN t
Re: [dpdk-dev] [PATCH] doc: add thread-safety information about EFD library
> -Original Message- > From: De Lara Guarch, Pablo > Sent: Tuesday, February 14, 2017 3:03 PM > To: Mcnamara, John > Cc: dev@dpdk.org; De Lara Guarch, Pablo > Subject: [PATCH] doc: add thread-safety information about EFD library > > Signed-off-by: Pablo de Lara Acked-by: John McNamara
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
On Tue, Feb 14, 2017 at 03:13:26PM +, Ferruh Yigit wrote: > Tests are part of app folder and compiled with library every time. > Moving tests into a "test" folder which won't be compiled by default. > To compile tests, need to give explicit "make test" command. > > "make test" was previously used to run tests, which renamed to > "make test_run" with this patch. > > This makes default compilation ~30% faster, > [clang, make -j8, old]: real1m04.355s > [clang, make -j8, new]: real0m41.740s > > For new case, test needs to built separately, which takes, > [clang, make -j8 test]: real0m24.293s > > The point is tests are not required always and by every one. > > Signed-off-by: Ferruh Yigit > --- Hi Ferruh, I'm not sure I'm convinced by this, as I think there are advantages to having the test code always compiled. Anything that is not compiled in DPDK by default is more likely to be broken by patch submissions. The speed boost to build is nice, but I'm not sure it's worth it. However, I'm open to being convinced otherwise on this... /Bruce
Re: [dpdk-dev] crypto drivers in the API
2017-02-14 14:46, Doherty, Declan: > On 14/02/2017 11:04 AM, Thomas Monjalon wrote: > > 2017-02-14 10:44, Doherty, Declan: > >> On 13/02/2017 1:25 PM, Thomas Monjalon wrote: > >>> In the crypto API, the drivers are listed. > >>> In my opinion, it is a wrong designed and these lists should be removed. > >>> Do we need a deprecation notice to plan this removal in 17.05, while > >>> working on bus abstraction? > >>> > >> ... > >>> > ... > > > > Yes > > If you were planning to do this, you should have sent a deprecation notice > > few weeks ago. > > Please send it now and we'll see if we have enough supporters shortly. > > > > Thomas, there are a couple of other changes we are looking at in the > cryptodev which would require API changes as well as break ABI including > adding support for a multi-device sessions, and changes to crypto > operation layout and field changes for performance but these but will > require RFCs or at least more discussion of the proposals. Given the > time constrains for the V1 deadline for 17.05 I would prefer to work on > the RFCs and get them out as soon as possible over the next few weeks > and then make all the ABI breaking changes in R17.08 in a single release. > > Otherwise we will end up breaking ABI 2 release in a row which I would > like to avoid if possible. OK, seems good. Thanks
Re: [dpdk-dev] [PATCH v1] doc: update release notes for 17.02
2017-02-14 15:32, John McNamara: > -Resolved Issues > > - > -.. This section should contain bug fixes added to the relevant sections. > Sample format: > - > - * **code/section Fixed issue in the past tense with a full stop.** > - > - Add a short 1-2 sentence description of the resolved issue in the past > tense. > - The title should contain the code/lib section like a commit message. > - Add the entries in alphabetic order in the relevant sections below. > - > - This section is a comment. do not overwrite or remove it. > - Also, make sure to start the actual text at the margin. > - = > - > - > -EAL > -~~~ > - > > Drivers > ~~~ > > * **net/virtio: Fixed multiple process support.** > > - Fixed few regressions introduced in recent releases that break the virtio > + Fixed a few regressions introduced in recent releases that break the virtio >multiple process support. > > > -Libraries > -~ > - > - > Examples > > > * **examples/ethtool: Fixed crash with non-PCI devices.** > > - Querying a non-PCI device was dereferencing non-existent PCI data > - resulting in a segmentation fault. > + Fixed issue where querying a non-PCI device was dereferencing non-existent > + PCI data resulting in a segmentation fault. It looks strange to remove the "Resolved Issues" heading while keeping content. I think we could totally remove them as most of the fixes are not documented here anyway.
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
> On Feb 14, 2017, at 9:41 AM, Bruce Richardson > wrote: > > On Tue, Feb 14, 2017 at 03:13:26PM +, Ferruh Yigit wrote: >> Tests are part of app folder and compiled with library every time. >> Moving tests into a "test" folder which won't be compiled by default. >> To compile tests, need to give explicit "make test" command. >> >> "make test" was previously used to run tests, which renamed to >> "make test_run" with this patch. >> >> This makes default compilation ~30% faster, >> [clang, make -j8, old]: real1m04.355s >> [clang, make -j8, new]: real0m41.740s >> >> For new case, test needs to built separately, which takes, >> [clang, make -j8 test]: real0m24.293s >> >> The point is tests are not required always and by every one. >> >> Signed-off-by: Ferruh Yigit >> --- > > Hi Ferruh, > > I'm not sure I'm convinced by this, as I think there are advantages to > having the test code always compiled. Anything that is not compiled in > DPDK by default is more likely to be broken by patch submissions. The > speed boost to build is nice, but I'm not sure it's worth it. > However, I'm open to being convinced otherwise on this... > > /Bruce I am kind of in the same boat as Bruce on the tests, but I was thinking the other applications in the app directory pretty much use most of the APIs, right? If that is the case then I would agree with Ferruh. Regards, Keith
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Bruce Richardson > Sent: Tuesday, February 14, 2017 3:42 PM > To: Yigit, Ferruh > Cc: Thomas Monjalon ; dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder > > On Tue, Feb 14, 2017 at 03:13:26PM +, Ferruh Yigit wrote: > > Tests are part of app folder and compiled with library every time. > > Moving tests into a "test" folder which won't be compiled by default. > > To compile tests, need to give explicit "make test" command. > > > > "make test" was previously used to run tests, which renamed to "make > > test_run" with this patch. > > > > This makes default compilation ~30% faster, > > [clang, make -j8, old]: real1m04.355s > > [clang, make -j8, new]: real0m41.740s > > > > For new case, test needs to built separately, which takes, > > [clang, make -j8 test]: real0m24.293s > > > > The point is tests are not required always and by every one. > > > > Signed-off-by: Ferruh Yigit > > --- > > Hi Ferruh, > > I'm not sure I'm convinced by this, as I think there are advantages to > having the test code always compiled. Anything that is not compiled in > DPDK by default is more likely to be broken by patch submissions. The > speed boost to build is nice, but I'm not sure it's worth it. > However, I'm open to being convinced otherwise on this... Hi, In general, I am in favour of separating the main compilation from the compilation of the tests. Usually unit test code doesn't get compiled until you run "make test". Also, having the "test" code in the "app" dir is a little odd. > Anything that is not compiled in > DPDK by default is more likely to be broken by patch submissions. It is probably more important that the user runs the tests than just compiles them. :-) As a side effect of running the tests they will also compile them. As a side issue I think that we should improve the ease of running and extending the test suite. YMMV but I have always found the test suite hard to run with 100% passing tests. So I think this is a good first step to separate out unit testing from the rest of the code. John
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit > Sent: Tuesday, February 14, 2017 3:13 PM > To: Thomas Monjalon > Cc: dev@dpdk.org; Yigit, Ferruh > Subject: [dpdk-dev] [RFC 17.05] test: move tests to separate folder > > Tests are part of app folder and compiled with library every time. > Moving tests into a "test" folder which won't be compiled by default. > To compile tests, need to give explicit "make test" command. > > "make test" was previously used to run tests, which renamed to > "make test_run" with this patch. Hi, I would prefer if "make test" still ran the tests, since that is the general assumption in most open source projects. Maybe have a "make test_compile" target to compile but not run the tests. Otherwise +1 for this. John
Re: [dpdk-dev] Further fun with ABI tracking
On Tue, Feb 14, 2017 at 11:52:00AM +0100, Christian Ehrhardt wrote: > Hi, > when moving to DPDK 16.11 Debian/Ubuntu packaging of DPDK has hit a new > twist on the (it seems reoccurring) topic of DPDK ABI tracking. > > I have found, ... well I don't want to call it solution ..., let's say a > crutch to get around it for the moment. But I wanted to use the example I > had to share a few thoughts on it and to kick off a wider discussion. > > > *## In library cross-dependencies plus partial ABI bumps ##* > > Since the day moving away from the combined shared library we had several > improvements on tracking the ABI versions. These days [1] we have LIBABIVER > per library and it gets bumped to reflect it is breaking with former > versions e.g. removing symbols. > > Now in the 16.11 release the ABIs for cryptodev, eal and ethdev got bumped > by [2] and [3]. > > OTOH please remember that in general two versions of a shared library in > the usual sense are meant to be able to stay alongside on a system without > hurting each other. I picked a random one on my system. > Package Library > libisc-export160: /lib/x86_64-linux-gnu/libisc-export.so.160 > libisc-export160: /lib/x86_64-linux-gnu/libisc-export.so.160.0.0 > libisc-export95: /lib/x86_64-linux-gnu/libisc-export.so.95 > libisc-export95: /lib/x86_64-linux-gnu/libisc-export.so.95.5.0 > Some link against the new, some against the old library - all fine. > Usually most programs can just be rebuilt against the new library and after > some time the old one can be dropped. That mechanism gives downstream > distributions a way to handle transitions and consumers of libraries which > might not all be ready for the same version every time. > And since the per lib versioning with LIBABIVER and and the version maps we > are good - in fact we qualify for all common cases on [4]. > > Now in DPDK of those libraries that got an ABI bump eal and ethdev are part > of those which most of us consider "core libraries" and most other libs and > pmds link to them. > And here DPDK continues to be special, due to that inter-dependency with > old and new libraries installed on the same system the following happens on > openvswitch built for an older version of dpdk: > ovs-vswitchd-dpdk > librte_eal.so.2 => /usr/lib/x86_64-linux-gnu/librte_eal.so.2 > librte_pdump.so.1 => /usr/lib/x86_64-linux-gnu/librte_pdump.so.1 > librte_eal.so.3 => /usr/lib/x86_64-linux-gnu/librte_eal.so.3 > > You can see that Openvswitch itself depends on the "old" librte_eal.so.2. > But because librte_pdump.so.1 did not get an ABI bump it got upgraded to > the newer version from DPDK 16.11. > But since the "new" pdump got built with the new DPDK 16.11 it depends on > the "new" librte_eal.so.3. > And having both in the same executable space at the same time causes > segfaults and pain. > > As I said for now I have passed the issue with a crutch that I'm not proud > of and I'd like to avoid in the future. For that I'm reaching out to you > with several suggestions to discuss. > > > *## Thoughts ##* > None of these seems like a perfect solution to me yet, but clearly good to > start discussions on them. > > Options that were in discussion so far and that we might adopt next cycle > (some of these are upstream changes, some downstream, some require both to > change - but any of them should have an ack upstream so that we are > agreeing how to proceed with those cases). > > 1. Downstreams to insert Major version into soname > Distributions could insert the DPDK major version (like 16.11) into the > soname and package names. A common example of this is libboost [5]. > That would perfectly allow 16.07. to coexist with > 16.11. even if for a given library LIBABIVER did not change. > Yet it would mean that anything depending on the old library will have to > be recompiled to pick up the new code, even if it depends on an ABI that is > still present in the new release. > Also - not a technical reason - but it is clearly more work to force update > all dependencies and clean out old packages for every release. > > > 2. ABI Ranges > One could argue that due to the detailed tracking of functions DPDK is > already close to track not ABI levels but actually ABI ranges. DPDK could > track LIBABIVERMIN and LIBABIVER. > Every time functionality is added LIBABIVER would get bumped, but > LIBABIVERMIN only gets moved to the OLDEST still supported ABI when things > are dropped. > So on a given library librte_foo you could have LIBABIVER=5 and > LIBABIVERMIN=3. The make install would then install the shared lib as: > librte_foo.so.5 > and additionally links for all compatible versions: > librte_foo.so.3 -> librte_foo.so.5 > librte_foo.so.4 -> librte_foo.so.5 > Yet, while is has some nice attributes this might make DPDK even more > special and cause ABI level proliferation over time. > Also even with this in place, changes moving LIBABIVERMIN "too fast" (too > fast is different for each downst
Re: [dpdk-dev] [PATCH v1] doc: update release notes for 17.02
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] > Sent: Tuesday, February 14, 2017 3:51 PM > To: Mcnamara, John > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v1] doc: update release notes for 17.02 > > 2017-02-14 15:32, John McNamara: > > -Resolved Issues > > > > - > > -.. This section should contain bug fixes added to the relevant > sections. Sample format: > > - > > - * **code/section Fixed issue in the past tense with a full stop.** > > - > > - Add a short 1-2 sentence description of the resolved issue in the > past tense. > > - The title should contain the code/lib section like a commit > message. > > - Add the entries in alphabetic order in the relevant sections > below. > > - > > - This section is a comment. do not overwrite or remove it. > > - Also, make sure to start the actual text at the margin. > > - = > > - > > - > > -EAL > > -~~~ > > - > > > > Drivers > > ~~~ > > > > * **net/virtio: Fixed multiple process support.** > > > > - Fixed few regressions introduced in recent releases that break the > > virtio > > + Fixed a few regressions introduced in recent releases that break > > + the virtio > >multiple process support. > > > > > > -Libraries > > -~ > > - > > - > > Examples > > > > > > * **examples/ethtool: Fixed crash with non-PCI devices.** > > > > - Querying a non-PCI device was dereferencing non-existent PCI data > > - resulting in a segmentation fault. > > + Fixed issue where querying a non-PCI device was dereferencing > > + non-existent PCI data resulting in a segmentation fault. > > It looks strange to remove the "Resolved Issues" heading while keeping > content. > I think we could totally remove them as most of the fixes are not > documented here anyway. You are right. That was a mistake on my side, I meant to leave the heading. However, now that you mention it, it doesn't look good that we have a "Resolved Issues" section with just 2 fixes. I don't think they capture the range of fixes in this release. Maybe we need to be more disciplined with including information in the Release Notes when non-trivial fixes are submitted. In this case I'll put back in the "Resolved Issues" heading and submit a v2. John
[dpdk-dev] [PATCH v2] doc: update release notes for 17.02
Fix grammar, spelling and formatting of DPDK 17.02 release notes. Signed-off-by: John McNamara --- doc/guides/rel_notes/release_17_02.rst | 241 +++-- 1 file changed, 111 insertions(+), 130 deletions(-) diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst index 6420a87..357965a 100644 --- a/doc/guides/rel_notes/release_17_02.rst +++ b/doc/guides/rel_notes/release_17_02.rst @@ -40,46 +40,47 @@ New Features * **Added support for representing buses in EAL** - A new structure ``rte_bus`` is introduced in EAL. This allows for devices to - be represented by buses they are connected to. A new bus can be added to - DPDK by extending the ``rte_bus`` structure and implementing the scan and - probe functions. Once a new bus is registered using provided APIs, new - devices can be detected and initialized using bus scan and probe callbacks. + The ``rte_bus`` structure was introduced into the EAL. This allows for + devices to be represented by buses they are connected to. A new bus can be + added to DPDK by extending the ``rte_bus`` structure and implementing the + scan and probe functions. Once a new bus is registered using the provided + APIs, new devices can be detected and initialized using bus scan and probe + callbacks. - With this change, devices other than PCI or VDEV type can also be represented - in DPDK framework. + With this change, devices other than PCI or VDEV type can be represented + in the DPDK framework. * **Added generic EAL API for I/O device memory read/write operations.** - This API introduces 8-bit, 16-bit, 32bit, 64bit I/O device - memory read/write operations along with the relaxed versions. + This API introduces 8 bit, 16 bit, 32 bit and 64 bit I/O device + memory read/write operations along with "relaxed" versions. - The weakly-ordered machine like ARM needs additional I/O barrier for - device memory read/write access over PCI bus. - By introducing the EAL abstraction for I/O device memory read/write access, - The drivers can access I/O device memory in architecture-agnostic manner. - The relaxed version does not have additional I/O memory barrier, useful in - accessing the device registers of integrated controllers which - implicitly strongly ordered with respect to memory access. + Weakly-ordered architectures like ARM need an additional I/O barrier for + device memory read/write access over PCI bus. By introducing the EAL + abstraction for I/O device memory read/write access, the drivers can access + I/O device memory in an architecture-agnostic manner. The relaxed version + does not have an additional I/O memory barrier, which is useful in accessing + the device registers of integrated controllers which is implicitly strongly + ordered with respect to memory access. * **Added generic flow API (rte_flow).** This API provides a generic means to configure hardware to match specific - ingress or egress traffic, alter its fate and query related counters + ingress or egress traffic, alter its behavior and query related counters according to any number of user-defined rules. - It is slightly higher-level than the legacy filtering framework which it - encompasses and supersedes (including all functions and filter types) in - order to expose a single interface with an unambiguous behavior that is - common to all poll-mode drivers (PMDs). + In order to expose a single interface with an unambiguous behavior that is + common to all poll-mode drivers (PMDs) the ``rte_flow`` API is slightly + higher-level than the legacy filtering framework, which it encompasses and + supersedes (including all functions and filter types) . See the :ref:`Generic flow API ` documentation for more information. * **Added firmware version get API.** - Added a new function ``rte_eth_dev_fw_version_get()`` to fetch firmware - version by a given device. + Added a new function ``rte_eth_dev_fw_version_get()`` to fetch the firmware + version for a given device. * **Added APIs for MACsec offload support to the ixgbe PMD.** @@ -90,54 +91,58 @@ New Features Added support for I219 Intel 1GbE NICs. -* **Added VF Daemon (VFD) on i40e. - EXPERIMENTAL** - - This's an EXPERIMENTAL feature to enhance the capability of DPDK PF as many - VF management features are not supported by kernel PF driver. - Some new private APIs are implemented in PMD without abstrction layer. - They can be used directly by some users who have the need. - - The new APIs to control VFs directly from PF include, - 1) set VF MAC anti-spoofing - 2) set VF VLAN anti-spoofing - 3) set TX loopback - 4) set VF unicast promiscuous mode - 5) set VF multicast promiscuous mode - 6) set VF MTU - 7) get/reset VF stats - 8) set VF MAC address - 9) set VF VLAN stripping - 10) VF VLAN insertion - 12) set VF broadcast mode - 13) set VF VLAN tag - 14) set VF VLAN filter - VFD also includes VF to
[dpdk-dev] [PATCH v3] mbuf: use pktmbuf helper to create the pool
When possible, replace the uses of rte_mempool_create() with
the helper provided in librte_mbuf: rte_pktmbuf_pool_create().
This is the preferred way to create a mbuf pool.
This also updates the documentation.
Signed-off-by: Olivier Matz
Signed-off-by: Hemant Agrawal
---
v3:
* removing changes from ip_reassembly app
v2:
* removing compilation error from ip_reassmebly
* fix minor patch apply warnings
---
app/test/test_link_bonding_rssconf.c | 11
doc/guides/sample_app_ug/ipv4_multicast.rst| 12
doc/guides/sample_app_ug/l2_forward_job_stats.rst | 33 --
.../sample_app_ug/l2_forward_real_virtual.rst | 26 +++--
doc/guides/sample_app_ug/ptpclient.rst | 11 ++--
doc/guides/sample_app_ug/quota_watermark.rst | 26 ++---
drivers/net/bonding/rte_eth_bond_8023ad.c | 13 -
examples/ip_pipeline/init.c| 18 ++--
examples/multi_process/l2fwd_fork/main.c | 13 +++--
examples/tep_termination/main.c| 16 +--
lib/librte_mbuf/rte_mbuf.c | 7 +++--
lib/librte_mbuf/rte_mbuf.h | 29 +++
12 files changed, 90 insertions(+), 125 deletions(-)
diff --git a/app/test/test_link_bonding_rssconf.c
b/app/test/test_link_bonding_rssconf.c
index 34f1c16..9034f62 100644
--- a/app/test/test_link_bonding_rssconf.c
+++ b/app/test/test_link_bonding_rssconf.c
@@ -67,7 +67,7 @@
#define SLAVE_RXTX_QUEUE_FMT ("rssconf_slave%d_q%d")
#define NUM_MBUFS 8191
-#define MBUF_SIZE (1600 + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
+#define MBUF_SIZE (1600 + RTE_PKTMBUF_HEADROOM)
#define MBUF_CACHE_SIZE 250
#define BURST_SIZE 32
@@ -536,13 +536,12 @@ struct link_bonding_rssconf_unittest_params {
if (test_params.mbuf_pool == NULL) {
- test_params.mbuf_pool = rte_mempool_create("RSS_MBUF_POOL",
NUM_MBUFS *
- SLAVE_COUNT, MBUF_SIZE, MBUF_CACHE_SIZE,
- sizeof(struct rte_pktmbuf_pool_private),
rte_pktmbuf_pool_init,
- NULL, rte_pktmbuf_init, NULL, rte_socket_id(),
0);
+ test_params.mbuf_pool = rte_pktmbuf_pool_create(
+ "RSS_MBUF_POOL", NUM_MBUFS * SLAVE_COUNT,
+ MBUF_CACHE_SIZE, 0, MBUF_SIZE, rte_socket_id());
TEST_ASSERT(test_params.mbuf_pool != NULL,
- "rte_mempool_create failed\n");
+ "rte_pktmbuf_pool_create failed\n");
}
/* Create / initialize ring eth devs. */
diff --git a/doc/guides/sample_app_ug/ipv4_multicast.rst
b/doc/guides/sample_app_ug/ipv4_multicast.rst
index 72da8c4..2c50a4f 100644
--- a/doc/guides/sample_app_ug/ipv4_multicast.rst
+++ b/doc/guides/sample_app_ug/ipv4_multicast.rst
@@ -145,12 +145,12 @@ Memory pools for indirect buffers are initialized
differently from the memory po
.. code-block:: c
-packet_pool = rte_mempool_create("packet_pool", NB_PKT_MBUF,
PKT_MBUF_SIZE, 32, sizeof(struct rte_pktmbuf_pool_private),
- rte_pktmbuf_pool_init, NULL,
rte_pktmbuf_init, NULL, rte_socket_id(), 0);
-
-header_pool = rte_mempool_create("header_pool", NB_HDR_MBUF,
HDR_MBUF_SIZE, 32, 0, NULL, NULL, rte_pktmbuf_init, NULL, rte_socket_id(), 0);
-clone_pool = rte_mempool_create("clone_pool", NB_CLONE_MBUF,
-CLONE_MBUF_SIZE, 32, 0, NULL, NULL, rte_pktmbuf_init, NULL,
rte_socket_id(), 0);
+packet_pool = rte_pktmbuf_pool_create("packet_pool", NB_PKT_MBUF, 32,
+ 0, PKT_MBUF_DATA_SIZE, rte_socket_id());
+header_pool = rte_pktmbuf_pool_create("header_pool", NB_HDR_MBUF, 32,
+ 0, HDR_MBUF_DATA_SIZE, rte_socket_id());
+clone_pool = rte_pktmbuf_pool_create("clone_pool", NB_CLONE_MBUF, 32,
+ 0, 0, rte_socket_id());
The reason for this is because indirect buffers are not supposed to hold any
packet data and
therefore can be initialized with lower amount of reserved memory for each
buffer.
diff --git a/doc/guides/sample_app_ug/l2_forward_job_stats.rst
b/doc/guides/sample_app_ug/l2_forward_job_stats.rst
index 2444e36..15743e6 100644
--- a/doc/guides/sample_app_ug/l2_forward_job_stats.rst
+++ b/doc/guides/sample_app_ug/l2_forward_job_stats.rst
@@ -193,36 +193,25 @@ and the application to store network packet data:
.. code-block:: c
/* create the mbuf pool */
-l2fwd_pktmbuf_pool =
-rte_mempool_create("mbuf_pool", NB_MBUF,
- MBUF_SIZE, 32,
- sizeof(struct rte_pktmbuf_pool_private),
- rte_pktmbuf_pool_init, NULL,
- rte_pktmbuf_init, NULL,
- rte_socket_id(), 0);
+l2fwd_pktmbuf_pool = rte_pktmbuf_pool_create("mbuf_pool", NB_MBUF,
+ MEMPOOL_CACHE_SIZE,
Re: [dpdk-dev] [PATCH] cfgfile: fix uninitialized variable on load error
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Dmitriy Yakovlev
> Sent: Tuesday, February 7, 2017 2:51 AM
> To: dev@dpdk.org
> Cc: Dmitriy Yakovlev
> Subject: [dpdk-dev] [PATCH] cfgfile: fix uninitialized variable on load error
>
> Uninitialized scalar variable. Using uninitialized value cfg-
> >sections[curr_section]->num_entries when calling rte_cfgfile_close.
> And memory in variables cfg->sections[curr_section], sect-
> >entries[curr_entry] maybe not equal NULL. We must decrement counters
> curr_section, curr_entry when failed to realloc.
>
> Fixes: eaafbad419bf ("cfgfile: library to interpret config files")
>
> Signed-off-by: Dmitriy Yakovlev
> ---
> lib/librte_cfgfile/rte_cfgfile.c | 4
> 1 file changed, 4 insertions(+)
>
Acked-by: Cristian Dumitrescu
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
On 2/14/2017 3:41 PM, Bruce Richardson wrote: > On Tue, Feb 14, 2017 at 03:13:26PM +, Ferruh Yigit wrote: >> Tests are part of app folder and compiled with library every time. >> Moving tests into a "test" folder which won't be compiled by default. >> To compile tests, need to give explicit "make test" command. >> >> "make test" was previously used to run tests, which renamed to >> "make test_run" with this patch. >> >> This makes default compilation ~30% faster, >> [clang, make -j8, old]: real1m04.355s >> [clang, make -j8, new]: real0m41.740s >> >> For new case, test needs to built separately, which takes, >> [clang, make -j8 test]: real0m24.293s >> >> The point is tests are not required always and by every one. >> >> Signed-off-by: Ferruh Yigit >> --- > > Hi Ferruh, > > I'm not sure I'm convinced by this, as I think there are advantages to > having the test code always compiled. Anything that is not compiled in > DPDK by default is more likely to be broken by patch submissions. The > speed boost to build is nice, but I'm not sure it's worth it. > However, I'm open to being convinced otherwise on this... Perhaps I should send this as two patches, first separate the unit tests and second disable tests in default compilation. Your concern is about disabling test compilation by default. - If nobody interested in running test, why force them to compile. If people interested in unit tests, test will be compiled and issues will be resolved. Unit tests are for developers more than end users, and if developers are not using unit tests we may have another thing to focus. - This should be automated somehow, unit test should be build and run regularly and automatically. So we can see when it is broken. Thanks, ferruh > > /Bruce >
Re: [dpdk-dev] [PATCH] cfgfile: fix uninitialized variable on load error
> > Uninitialized scalar variable. Using uninitialized value cfg-
> > >sections[curr_section]->num_entries when calling rte_cfgfile_close.
> > And memory in variables cfg->sections[curr_section], sect-
> > >entries[curr_entry] maybe not equal NULL. We must decrement counters
> > curr_section, curr_entry when failed to realloc.
> >
> > Fixes: eaafbad419bf ("cfgfile: library to interpret config files")
> >
> > Signed-off-by: Dmitriy Yakovlev
> > ---
> > lib/librte_cfgfile/rte_cfgfile.c | 4
> > 1 file changed, 4 insertions(+)
> >
>
> Acked-by: Cristian Dumitrescu
Applied, thanks
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
> -Original Message- > From: Yigit, Ferruh > Sent: Tuesday, February 14, 2017 5:07 PM > To: Richardson, Bruce > Cc: Thomas Monjalon ; dev@dpdk.org > Subject: Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder > > On 2/14/2017 3:41 PM, Bruce Richardson wrote: > > On Tue, Feb 14, 2017 at 03:13:26PM +, Ferruh Yigit wrote: > >> Tests are part of app folder and compiled with library every time. > >> Moving tests into a "test" folder which won't be compiled by default. > >> To compile tests, need to give explicit "make test" command. > >> > >> "make test" was previously used to run tests, which renamed to "make > >> test_run" with this patch. > >> > >> This makes default compilation ~30% faster, > >> [clang, make -j8, old]: real1m04.355s > >> [clang, make -j8, new]: real0m41.740s > >> > >> For new case, test needs to built separately, which takes, > >> [clang, make -j8 test]: real0m24.293s > >> > >> The point is tests are not required always and by every one. > >> > >> Signed-off-by: Ferruh Yigit > >> --- > > > > Hi Ferruh, > > > > I'm not sure I'm convinced by this, as I think there are advantages to > > having the test code always compiled. Anything that is not compiled in > > DPDK by default is more likely to be broken by patch submissions. The > > speed boost to build is nice, but I'm not sure it's worth it. > > However, I'm open to being convinced otherwise on this... > > Perhaps I should send this as two patches, first separate the unit tests > and second disable tests in default compilation. > > Your concern is about disabling test compilation by default. > > - If nobody interested in running test, why force them to compile. If > people interested in unit tests, test will be compiled and issues will be > resolved. > Unit tests are for developers more than end users, and if developers are > not using unit tests we may have another thing to focus. > > - This should be automated somehow, unit test should be build and run > regularly and automatically. So we can see when it is broken. > Ok, makes sense. Just to be awkward :-), one last question: Why separate building and running the tests? My suggestion would be to have "make test" both build and run the tests. If there is no work to do in building them, then the time cost of the extra empty build is negligible compared to the time taken to actually run the tests. If we really want to build the tests without running them I'd suggest "build_test" target instead. This keeps "make test" pretty much as it was before. /Bruce
Re: [dpdk-dev] doc: announce TILE-Gx removal
Applied
Re: [dpdk-dev] doc: add ABI change notification for ring library
Applied
Re: [dpdk-dev] [PATCH v2] doc: announce API changes to implement the bus model
Applied
Re: [dpdk-dev] [PATCH] rte_table: ensure prev bucket has a valid pointer
Hi Emmanuel,
> -Original Message-
> From: Emmanuel Roullit [mailto:emmanuel.roul...@gmail.com]
> Sent: Tuesday, January 24, 2017 8:39 PM
> To: Dumitrescu, Cristian
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: [PATCH] rte_table: ensure prev bucket has a valid pointer
>
> Fixes: 43f15e28377f ("table: fix verification on hash bucket header
> alignment")
>
> Signed-off-by: Emmanuel Roullit
> ---
> lib/librte_table/rte_table_hash_key16.c | 7 +--
> lib/librte_table/rte_table_hash_key32.c | 7 +--
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_table/rte_table_hash_key16.c
> b/lib/librte_table/rte_table_hash_key16.c
> index 08d4d77eb..9c04e7f5e 100644
> --- a/lib/librte_table/rte_table_hash_key16.c
> +++ b/lib/librte_table/rte_table_hash_key16.c
> @@ -483,8 +483,11 @@ rte_table_hash_entry_add_key16_ext(
>
> bucket = (struct rte_bucket_4_16 *) &f->memory[(f-
> >n_buckets +
> bucket_index) * f->bucket_size];
> - bucket_prev->next = bucket;
> - bucket_prev->next_valid = 1;
> +
> + if (bucket_prev) {
> + bucket_prev->next = bucket;
> + bucket_prev->next_valid = 1;
> + }
>
> bucket->signature[0] = signature;
> memcpy(bucket->key[0], key, f->key_size);
> diff --git a/lib/librte_table/rte_table_hash_key32.c
> b/lib/librte_table/rte_table_hash_key32.c
> index 161f6b7a7..27e221be9 100644
> --- a/lib/librte_table/rte_table_hash_key32.c
> +++ b/lib/librte_table/rte_table_hash_key32.c
> @@ -471,8 +471,11 @@ rte_table_hash_entry_add_key32_ext(
> bucket = (struct rte_bucket_4_32 *)
> &f->memory[(f->n_buckets + bucket_index) *
> f->bucket_size];
> - bucket_prev->next = bucket;
> - bucket_prev->next_valid = 1;
> +
> + if (bucket_prev) {
> + bucket_prev->next = bucket;
> + bucket_prev->next_valid = 1;
> + }
>
> bucket->signature[0] = signature;
> memcpy(bucket->key[0], key, f->key_size);
> --
> 2.11.0
Each table bucket is initialized with a group of 4 entries which can be further
extended with one (or several) groups of 4 entries (we also call a group of 4
entries as a bucket).
Therefore, there is no way bucket_prev could be NULL. Were you able to hit a
case with bucket_prev == NULL at run-time, or was this produced by a code
analysis tool (in which case this is likely a false positive)?
Regards,
Cristian
Re: [dpdk-dev] [PATCH] doc: deprecation note for renaming vfio symbols for exporting
Applied
Re: [dpdk-dev] [PATCH] doc: postpone API change in ethdev
Applied
Re: [dpdk-dev] [PATCH v2] doc: announce API and ABI change for ethdev
Applied
Re: [dpdk-dev] [PATCH] doc: announce API/ABI changes for vhost
Applied
Re: [dpdk-dev] [PATCH] doc: announce kni_vhost removal
Applied
Re: [dpdk-dev] Further fun with ABI tracking
On Tue, Feb 14, 2017 at 11:52 AM, Christian Ehrhardt wrote: > Hi, > when moving to DPDK 16.11 Debian/Ubuntu packaging of DPDK has hit a new > twist on the (it seems reoccurring) topic of DPDK ABI tracking. > > I have found, ... well I don't want to call it solution ..., let's say a > crutch to get around it for the moment. But I wanted to use the example I > had to share a few thoughts on it and to kick off a wider discussion. > > > *## In library cross-dependencies plus partial ABI bumps ##* > > Since the day moving away from the combined shared library we had several > improvements on tracking the ABI versions. These days [1] we have LIBABIVER > per library and it gets bumped to reflect it is breaking with former > versions e.g. removing symbols. > > Now in the 16.11 release the ABIs for cryptodev, eal and ethdev got bumped > by [2] and [3]. > > OTOH please remember that in general two versions of a shared library in > the usual sense are meant to be able to stay alongside on a system without > hurting each other. I picked a random one on my system. > Package Library > libisc-export160: /lib/x86_64-linux-gnu/libisc-export.so.160 > libisc-export160: /lib/x86_64-linux-gnu/libisc-export.so.160.0.0 > libisc-export95: /lib/x86_64-linux-gnu/libisc-export.so.95 > libisc-export95: /lib/x86_64-linux-gnu/libisc-export.so.95.5.0 > Some link against the new, some against the old library - all fine. > Usually most programs can just be rebuilt against the new library and after > some time the old one can be dropped. That mechanism gives downstream > distributions a way to handle transitions and consumers of libraries which > might not all be ready for the same version every time. > And since the per lib versioning with LIBABIVER and and the version maps we > are good - in fact we qualify for all common cases on [4]. > > Now in DPDK of those libraries that got an ABI bump eal and ethdev are part > of those which most of us consider "core libraries" and most other libs and > pmds link to them. > And here DPDK continues to be special, due to that inter-dependency with > old and new libraries installed on the same system the following happens on > openvswitch built for an older version of dpdk: > ovs-vswitchd-dpdk > librte_eal.so.2 => /usr/lib/x86_64-linux-gnu/librte_eal.so.2 > librte_pdump.so.1 => /usr/lib/x86_64-linux-gnu/librte_pdump.so.1 > librte_eal.so.3 => /usr/lib/x86_64-linux-gnu/librte_eal.so.3 > > You can see that Openvswitch itself depends on the "old" librte_eal.so.2. > But because librte_pdump.so.1 did not get an ABI bump it got upgraded to > the newer version from DPDK 16.11. > But since the "new" pdump got built with the new DPDK 16.11 it depends on > the "new" librte_eal.so.3. > And having both in the same executable space at the same time causes > segfaults and pain. > > As I said for now I have passed the issue with a crutch that I'm not proud > of and I'd like to avoid in the future. For that I'm reaching out to you > with several suggestions to discuss. > > > *## Thoughts ##* > None of these seems like a perfect solution to me yet, but clearly good to > start discussions on them. > > Options that were in discussion so far and that we might adopt next cycle > (some of these are upstream changes, some downstream, some require both to > change - but any of them should have an ack upstream so that we are > agreeing how to proceed with those cases). > > 1. Downstreams to insert Major version into soname > Distributions could insert the DPDK major version (like 16.11) into the > soname and package names. A common example of this is libboost [5]. > That would perfectly allow 16.07. to coexist with > 16.11. even if for a given library LIBABIVER did not change. > Yet it would mean that anything depending on the old library will have to > be recompiled to pick up the new code, even if it depends on an ABI that is > still present in the new release. > Also - not a technical reason - but it is clearly more work to force update > all dependencies and clean out old packages for every release. Actually this isn't exactly what I proposed during the summit. Just keep it simple and fix the ABI version of all libraries at 16.11.0. This is a proven approach and has been used for years with different libraries. You could easily do this independently of us upstream fixing the ABI problems. > 2. ABI Ranges ABI is either backwards compatible (same major) or not. A range doesn't solve the problem. > > 3. A lot of conflicts > This doesn't allow us to have multiple version of the library available at runtime. So in the end it doesn't solve the problem for the distro either. > > 4. ABI bump is infecting > > 5. back to single ABI > This is very similar to approach 1. It just uses up a lot more ABI versions. > 6. More > I'm sure there are more approaches to this, feel free to come up with more. > The problem is that we do not detect and fix the ABI changes that "shine-through" the dependen
Re: [dpdk-dev] [PATCH v2] doc: annouce ABI change for cryptodev ops structure
Applied
Re: [dpdk-dev] [PATCH] doc: add deprecation note to add parameter in rte_cryptodev_info.sym
Applied
Re: [dpdk-dev] [PATCH] doc: add thread-safety information about EFD library
> > Signed-off-by: Pablo de Lara > > Acked-by: John McNamara Applied, thanks
Re: [dpdk-dev] [PATCH v3 00/25] linux/eal: Remove most causes of panic on init
Stephen Hemminger writes: > On Thu, 9 Feb 2017 09:29:28 -0500 > Aaron Conole wrote: > >> In many cases, it's enough to simply let the application know that the >> call to initialize DPDK has failed. A complete halt can then be >> decided by the application based on error returned (and the app could >> even attempt a possible re-attempt after some corrective action by the >> user or application). >> >> ... >> > > I worry that some of these early failure messages may never be visible > because the logging system has not been initialized. Might be safer to > just use fprintf(stderr, ...) or define a new wrapper function. Thanks for the suggestion, Stephen! I've folded it into my series. -Aaron
Re: [dpdk-dev] [PATCH v2] doc/contributing: add description of review tags
2017-02-14 11:50, Harry van Haaren: > This commit details what is meant by the various email > tags that the DPDK community use regularly. The descriptions > state what each tag means, drawing from the kernel's understanding[1], > and the discussion on the DPDK mailing list[2]. > > Signed-off-by: Harry van Haaren > Acked-by: John McNamara Applied, thanks
Re: [dpdk-dev] [PATCH v2] doc: update release notes for 17.02
2017-02-14 16:26, John McNamara: > Fix grammar, spelling and formatting of DPDK 17.02 release notes. > > Signed-off-by: John McNamara Applied, thanks This is the final touch of this release.
Re: [dpdk-dev] [PATCH v3 25/25] rte_eal_init: add info about rte_errno codes
Stephen Hemminger writes: > On Thu, 9 Feb 2017 09:29:53 -0500 > Aaron Conole wrote: > >> + * The error codes returned via rte_errno: >> + * EACCES indicates a permissions issue. >> + * >> + * EAGAIN indicates either a bus or system resource was not available, >> + *try again. >> + * >> + * EALREADY indicates that the rte_eal_init function has already been >> + * called, and cannot be called again. >> + * >> + * EFAULT indicates the tailq configuration name was not found in >> + *memory configuration. >> + * >> + * EINVAL indicates invalid parameters were passed as argv/argc. >> + * >> + * EIO indicates failure to setup the logging handlers. This is usually >> + * caused by an out-of-memory condition. >> + * >> + * ENODEV indicates memory setup issues. >> + * >> + * ENOTSUP indicates that the EAL cannot initialize on this system. >> + * >> + * EUNATCH indicates that the PCI bus is either not present, or is not >> + * readable by the eal. >> */ > > You might want to be less restrictive about wording in the comment. > In future more errors might be returned, and also for out of memory > ENOMEM is better. Sure thing, I'll switch EIO and ENODEV to ENOMEM, does that make sense? Also, which message do you refer to? Is it "The error codes returned via rte_errno" section? I assume that adding new error codes will also bring an update to the eal_init documentation, but perhaps I'm misunderstanding. Thanks for your review, Stephen!
[dpdk-dev] [PATCH] examples: optind should be reset to one not zero
Signed-off-by: Keith Wiles --- app/test-pipeline/config.c | 2 +- examples/distributor/main.c | 2 +- examples/dpdk_qat/main.c| 2 +- examples/ip_fragmentation/main.c| 2 +- examples/ip_pipeline/config_parse.c | 4 ++-- examples/ip_reassembly/main.c | 2 +- examples/ipsec-secgw/ipsec-secgw.c | 2 +- examples/ipv4_multicast/main.c | 2 +- examples/l2fwd-cat/cat.c| 2 +- examples/l2fwd-crypto/main.c| 2 +- examples/l2fwd-jobstats/main.c | 2 +- examples/l2fwd-keepalive/main.c | 2 +- examples/l2fwd/main.c | 2 +- examples/l3fwd-acl/main.c | 2 +- examples/l3fwd-power/main.c | 2 +- examples/l3fwd-vf/main.c| 2 +- examples/l3fwd/main.c | 2 +- examples/link_status_interrupt/main.c | 2 +- examples/load_balancer/config.c | 2 +- examples/multi_process/l2fwd_fork/main.c| 2 +- examples/multi_process/symmetric_mp/main.c | 2 +- examples/packet_ordering/main.c | 2 +- examples/performance-thread/l3fwd-thread/main.c | 2 +- examples/ptpclient/ptpclient.c | 2 +- examples/qos_meter/main.c | 2 +- 25 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/test-pipeline/config.c b/app/test-pipeline/config.c index dd80ed6..1b397c0 100644 --- a/app/test-pipeline/config.c +++ b/app/test-pipeline/config.c @@ -259,6 +259,6 @@ app_parse_args(int argc, char **argv) argv[optind - 1] = prgname; ret = optind - 1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return ret; } diff --git a/examples/distributor/main.c b/examples/distributor/main.c index e7641d2..7b8a759 100644 --- a/examples/distributor/main.c +++ b/examples/distributor/main.c @@ -487,7 +487,7 @@ parse_args(int argc, char **argv) argv[optind-1] = prgname; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return 0; } diff --git a/examples/dpdk_qat/main.c b/examples/dpdk_qat/main.c index aa9b1d5..a96119c 100644 --- a/examples/dpdk_qat/main.c +++ b/examples/dpdk_qat/main.c @@ -582,7 +582,7 @@ parse_args(int argc, char **argv) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return ret; } diff --git a/examples/ip_fragmentation/main.c b/examples/ip_fragmentation/main.c index e1e32c6..9e9ecae 100644 --- a/examples/ip_fragmentation/main.c +++ b/examples/ip_fragmentation/main.c @@ -586,7 +586,7 @@ parse_args(int argc, char **argv) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return ret; } diff --git a/examples/ip_pipeline/config_parse.c b/examples/ip_pipeline/config_parse.c index 8b372e9..3ae7d48 100644 --- a/examples/ip_pipeline/config_parse.c +++ b/examples/ip_pipeline/config_parse.c @@ -1,4 +1,4 @@ -/*- +/*- * BSD LICENSE * * Copyright(c) 2010-2016 Intel Corporation. All rights reserved. @@ -3407,7 +3407,7 @@ app_config_args(struct app_params *app, int argc, char **argv) app_print_usage(argv[0]); } - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ /* Check dependencies between args */ if (preproc_params_present && (preproc_present == 0)) diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c index 50fe422..e62674c 100644 --- a/examples/ip_reassembly/main.c +++ b/examples/ip_reassembly/main.c @@ -718,7 +718,7 @@ parse_args(int argc, char **argv) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return ret; } diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c index 5a4c9b7..685feec 100644 --- a/examples/ipsec-secgw/ipsec-secgw.c +++ b/examples/ipsec-secgw/ipsec-secgw.c @@ -1039,7 +1039,7 @@ parse_args(int32_t argc, char **argv) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return ret; } diff --git a/examples/ipv4_multicast/main.c b/examples/ipv4_multicast/main.c index 708d76e..b681f8e 100644 --- a/examples/ipv4_multicast/main.c +++ b/examples/ipv4_multicast/main.c @@ -575,7 +575,7 @@ parse_args(int argc, char **argv) argv[optind-1] = prgname; ret = optind-1; - optind = 0; /* reset getopt lib */ + optind = 1; /* reset getopt lib */ return re
[dpdk-dev] [dpdk-announce] DPDK 17.02 released
A new major release is available: http://fast.dpdk.org/rel/dpdk-17.02.tar.xz It has been a busy cycle considering the various holidays: 849 patches from 101 authors 655 files changed, 141527 insertions(+), 10539 deletions(-) There are 41 new contributors (including authors, reviewers and testers): Thanks to Alan Dewar, Aleksander Gajewski, Anand B Jyoti, Anders Roxell, Andrew Lee, Andrew Rybchenko, Andy Moreton, Artem Andreev, Aws Ismail, Baruch Siach, Bert van Leeuwen, Chenghu Yao, Christian Maciocco, Emmanuel Roullit, Hanoch Haim, Ilya V. Matveychikov, Ivan Malov, Jacek Piasecki, Jan Wickbom, Karla Saur, Kevin Traynor, Kuba Kozak, Lei Yao, Mark Spender, Matthieu Ternisien d'Ouville, Michał Mirosław, Patrick MacArthur, Robert Stonehouse, Satha Rao, Shahaf Shuler, Steve Shin, Timmons C. Player, Tom Crugnale, Xieming Katty, Yaron Illouz, Yi Zhang, Yong Wang, Yoni Gilad, Yuan Peng, Zbigniew Bodek, Zhaoyan Chen. These new contributors are associated with these domain names: 6wind.com, atendesoftware.pl, brocade.com, caviumnetworks.com, ciena.com, cisco.com, ericsson.com, gmail.com, huawei.com, intel.com, linaro.org, mellanox.com, netronome.com, oktetlabs.ru, patrickmacarthur.net, radcom.com, redhat.com, rere.qmqm.pl, sandvine.com, solarflare.com, spirent.com, tkos.co.il, zte.com.cn. Some highlights: - new ethdev API for hardware filtering - Solarflare networking driver - ARM crypto driver - crypto scheduler driver - crypto performance test application - Elastic Flow Distributor library - virtio-user/kernel-vhost as exception path More details in the release notes: http://dpdk.org/doc/guides/rel_notes/release_17_02.html The new features for the 17.05 cycle must be submitted before the end of February, in order to be reviewed and integrated during March. The next release is expected to happen at the beginning of May. Thanks everyone PS: there is no special name or alias for the DPDK versions though this one could be named Valentine, or Aurelie for love dedication ;)
Re: [dpdk-dev] [PATCH] eventdev: Add rte_errno return values to the enqueue and dequeue functions
> -Original Message-
> From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com]
> Sent: Monday, February 13, 2017 10:10 PM
> To: Eads, Gage
> Cc: dev@dpdk.org; Richardson, Bruce ;
> hemant.agra...@nxp.com; Van Haaren, Harry ;
> nipun.gu...@nxp.com
> Subject: Re: [PATCH] eventdev: Add rte_errno return values to the enqueue and
> dequeue functions
>
> On Fri, Feb 10, 2017 at 03:02:21PM -0600, Gage Eads wrote:
> > This change allows user software to differentiate between an invalid
> > argument (such as an invalid queue_id or sched_type in an enqueued
> > event) and backpressure from the event device.
> >
> > The port and device ID checks are placed in RTE_LIBRTE_EVENTDEV_DEBUG
> > header guards to avoid the performance hit in non-debug execution.
> >
> > Signed-off-by: Gage Eads
> > ---
> > static inline uint16_t
> > @@ -1127,6 +1133,21 @@ rte_event_enqueue_burst(uint8_t dev_id, uint8_t
> > port_id, {
> >struct rte_eventdev *dev = &rte_eventdevs[dev_id];
> >
> > + rte_errno = 0;
>
> I don't think it is required. If at all required, move this under
> RTE_LIBRTE_EVENTDEV_DEBUG to save store to rte_errno cycles on fastpath
Agreed -- if we encounter an error we should set rte_errno, otherwise the
return value will equal nb_events and the user shouldn't check it.
Will fix in v2.
>
> > +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> > + if (rte_eventdevs[dev_id].attached == RTE_EVENTDEV_DETACHED) {
> > + RTE_EDEV_LOG_DEBUG("Invalid dev_id=%d\n", dev_id);
> > + rte_errno = -EINVAL;
> > + return 0;
> > + }
> > +
> > + if (port_id >= dev->data->nb_ports) {
> > + RTE_EDEV_LOG_DEBUG("Invalid port_id=%d\n", port_id);
> > + rte_errno = -EINVAL;
> > + return 0;
> > + }
> > +#endif
> > +
> >/*
> > * Allow zero cost non burst mode routine invocation if application
> > * requests nb_events as const one
> > @@ -1235,6 +1256,21 @@ rte_event_dequeue_burst(uint8_t dev_id, uint8_t
> > port_id, struct rte_event ev[], {
> >struct rte_eventdev *dev = &rte_eventdevs[dev_id];
> >
> > +#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> > + rte_errno = 0;
> > + if (rte_eventdevs[dev_id].attached == RTE_EVENTDEV_DETACHED) {
> > + RTE_EDEV_LOG_DEBUG("Invalid dev_id=%d\n", dev_id);
> > + rte_errno = -EINVAL;
> > + return 0;
> > + }
> > +
> > + if (port_id >= dev->data->nb_ports) {
> > + RTE_EDEV_LOG_DEBUG("Invalid port_id=%d\n", port_id);
> > + rte_errno = -EINVAL;
> > + return 0;
> > + }
> > +#endif
> > +
> >/*
> > * Allow zero cost non burst mode routine invocation if application
> > * requests nb_events as const one
> > --
> > 2.7.4
> >
[dpdk-dev] [PATCH] net/i40e: fix fail to start testpmd
Testpmd failed to start in another hugetlbfs mount point.
Fix the issue by assigning scocket id during hash parameter
defination.
Fixes: 5c53c82c8174 ("net/i40e: store flow director filter")
Fixes: 425c3325f0b0 ("net/i40e: store tunnel filter")
Fixes: 078259773da9 ("net/i40e: store ethertype filter")
Signed-off-by: Beilei Xing
---
drivers/net/i40e/i40e_ethdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 303027b..be2e580 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -899,6 +899,8 @@ i40e_init_ethtype_filter_list(struct rte_eth_dev *dev)
.entries = I40E_MAX_ETHERTYPE_FILTER_NUM,
.key_len = sizeof(struct i40e_ethertype_filter_input),
.hash_func = rte_hash_crc,
+ .hash_func_init_val = 0,
+ .socket_id = rte_socket_id(),
};
/* Initialize ethertype filter rule list and hash */
@@ -942,6 +944,8 @@ i40e_init_tunnel_filter_list(struct rte_eth_dev *dev)
.entries = I40E_MAX_TUNNEL_FILTER_NUM,
.key_len = sizeof(struct i40e_tunnel_filter_input),
.hash_func = rte_hash_crc,
+ .hash_func_init_val = 0,
+ .socket_id = rte_socket_id(),
};
/* Initialize tunnel filter rule list and hash */
@@ -985,6 +989,8 @@ i40e_init_fdir_filter_list(struct rte_eth_dev *dev)
.entries = I40E_MAX_FDIR_FILTER_NUM,
.key_len = sizeof(struct rte_eth_fdir_input),
.hash_func = rte_hash_crc,
+ .hash_func_init_val = 0,
+ .socket_id = rte_socket_id(),
};
/* Initialize flow director filter rule list and hash */
--
2.5.5
Re: [dpdk-dev] [RFC 17.05] test: move tests to separate folder
On Tue, Feb 14, 2017 at 05:30:14PM +, Richardson, Bruce wrote: > Just to be awkward :-), one last question: Why separate building and running > the tests? My suggestion would be to have "make test" both build and run the > tests. If there is no work to do in building them, then the time cost of the > extra empty build is negligible compared to the time taken to actually run > the tests. +1 --yliu > If we really want to build the tests without running them I'd > suggest "build_test" target instead. This keeps "make test" pretty much as > it was before. > > /Bruce
