* Re: [PATCH v2 09/12] virtio: add Rx checksum offload support
From: Olivier Matz @ 2016-10-05 11:56 UTC (permalink / raw)
To: Maxime Coquelin, dev, yuanhan.liu
Cc: konstantin.ananyev, sugesh.chandran, bruce.richardson,
jianfeng.tan, helin.zhang, adrien.mazarguil, stephen, dprovan,
xiao.w.wang
In-Reply-To: <1da0b2c2-931b-3164-d211-4ceee7fd6864@redhat.com>
Hi Maxime,
On 10/03/2016 02:51 PM, Maxime Coquelin wrote:
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -50,6 +50,7 @@
>> #include <rte_string_fns.h>
>> #include <rte_errno.h>
>> #include <rte_byteorder.h>
>> +#include <rte_net.h>
>>
>> #include "virtio_logs.h"
>> #include "virtio_ethdev.h"
>> @@ -627,6 +628,56 @@ virtio_update_packet_stats(struct virtnet_stats
>> *stats, struct rte_mbuf *mbuf)
>> }
>> }
>>
>> +/* Optionally fill offload information in structure */
>> +static int
>> +virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr)
>> +{
>> + struct rte_net_hdr_lens hdr_lens;
>> + uint32_t hdrlen, ptype;
>> + int l4_supported = 0;
>> +
>> + /* nothing to do */
>> + if (hdr->flags == 0 && hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE)
>> + return 0;
> Maybe we could first check whether offload features were negotiated?
> Doing this, we could return before accessing the header and so avoid a
> cache miss.
Yes, doing this would avoid reading the virtio header when the rx
function is virtio_recv_pkts(). When using virtio_recv_mergeable_pkts(),
it won't have a big impact since we already need to read hdr->num_buffers.
I plan to do something like this in both recv functions:
@@ -854,6 +854,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts, uint16_t nb_pkts)
int error;
uint32_t i, nb_enqueued;
uint32_t hdr_size;
+ uint64_t features;
struct virtio_net_hdr *hdr;
nb_used = VIRTQUEUE_NUSED(vq);
@@ -872,6 +873,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts, uint16_t nb_pkts)
nb_rx = 0;
nb_enqueued = 0;
hdr_size = hw->vtnet_hdr_size;
+ features = hw->guest_features;
for (i = 0; i < num ; i++) {
rxm = rcv_pkts[i];
@@ -903,7 +905,8 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf
**rx_pkts, uint16_t nb_pkts)
rte_vlan_strip(rxm);
/* Update offload features */
- if (virtio_rx_offload(rxm, hdr) < 0) {
+ if ((features & VIRTIO_NET_F_GUEST_CSUM) &&
+ virtio_rx_offload(rxm, hdr) < 0) {
virtio_discard_rxbuf(vq, rxm);
rxvq->stats.errors++;
continue;
Thank you for the feedback.
Olivier
^ permalink raw reply
* Re: [PATCH] examples/l3fwd: em path hash offload to machine
From: Jerin Jacob @ 2016-10-05 11:52 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev
In-Reply-To: <1471964080-29958-1-git-send-email-hemant.agrawal@nxp.com>
On Tue, Aug 23, 2016 at 08:24:39PM +0530, Hemant Agrawal wrote:
Maybe you can change the subject line to:
examples/l3fwd: em: use hw accelerated crc hash function for arm64
instead of:
examples/l3fwd: em path hash offload to machine
> if machine level CRC extension are available, offload the
> hash to machine provided functions e.g. armv8-a CRC extensions
> support it
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> examples/l3fwd/l3fwd_em.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/examples/l3fwd/l3fwd_em.c b/examples/l3fwd/l3fwd_em.c
> index def5a02..a889c67 100644
> --- a/examples/l3fwd/l3fwd_em.c
> +++ b/examples/l3fwd/l3fwd_em.c
> @@ -58,13 +58,13 @@
>
> #include "l3fwd.h"
>
> -#ifdef RTE_MACHINE_CPUFLAG_SSE4_2
> +#if defined(RTE_MACHINE_CPUFLAG_SSE4_2) || defined(RTE_MACHINE_CPUFLAG_CRC32)
Rather than adding new compilation flag everywhere, Maybe you can add
#if defined(RTE_MACHINE_CPUFLAG_SSE4_2) &&
defined(RTE_MACHINE_CPUFLAG_CRC32)
#define EM_HASH_CRC 1
#endif
something like above to reduce the change for future platforms with crc
support.
Other than that, you can add:
Reviewed-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> #include <rte_hash_crc.h>
> #define DEFAULT_HASH_FUNC rte_hash_crc
> #else
> #include <rte_jhash.h>
> #define DEFAULT_HASH_FUNC rte_jhash
> -#endif /* RTE_MACHINE_CPUFLAG_SSE4_2 */
> +#endif
>
> #define IPV6_ADDR_LEN 16
>
> @@ -169,17 +169,17 @@ ipv4_hash_crc(const void *data, __rte_unused uint32_t data_len,
> t = k->proto;
> p = (const uint32_t *)&k->port_src;
>
> -#ifdef RTE_MACHINE_CPUFLAG_SSE4_2
> +#if defined(RTE_MACHINE_CPUFLAG_SSE4_2) || defined(RTE_MACHINE_CPUFLAG_CRC32)
> init_val = rte_hash_crc_4byte(t, init_val);
> init_val = rte_hash_crc_4byte(k->ip_src, init_val);
> init_val = rte_hash_crc_4byte(k->ip_dst, init_val);
> init_val = rte_hash_crc_4byte(*p, init_val);
> -#else /* RTE_MACHINE_CPUFLAG_SSE4_2 */
> +#else
> init_val = rte_jhash_1word(t, init_val);
> init_val = rte_jhash_1word(k->ip_src, init_val);
> init_val = rte_jhash_1word(k->ip_dst, init_val);
> init_val = rte_jhash_1word(*p, init_val);
> -#endif /* RTE_MACHINE_CPUFLAG_SSE4_2 */
> +#endif
>
> return init_val;
> }
> @@ -191,16 +191,16 @@ ipv6_hash_crc(const void *data, __rte_unused uint32_t data_len,
> const union ipv6_5tuple_host *k;
> uint32_t t;
> const uint32_t *p;
> -#ifdef RTE_MACHINE_CPUFLAG_SSE4_2
> +#if defined(RTE_MACHINE_CPUFLAG_SSE4_2) || defined(RTE_MACHINE_CPUFLAG_CRC32)
> const uint32_t *ip_src0, *ip_src1, *ip_src2, *ip_src3;
> const uint32_t *ip_dst0, *ip_dst1, *ip_dst2, *ip_dst3;
> -#endif /* RTE_MACHINE_CPUFLAG_SSE4_2 */
> +#endif
>
> k = data;
> t = k->proto;
> p = (const uint32_t *)&k->port_src;
>
> -#ifdef RTE_MACHINE_CPUFLAG_SSE4_2
> +#if defined(RTE_MACHINE_CPUFLAG_SSE4_2) || defined(RTE_MACHINE_CPUFLAG_CRC32)
> ip_src0 = (const uint32_t *) k->ip_src;
> ip_src1 = (const uint32_t *)(k->ip_src+4);
> ip_src2 = (const uint32_t *)(k->ip_src+8);
> @@ -219,14 +219,14 @@ ipv6_hash_crc(const void *data, __rte_unused uint32_t data_len,
> init_val = rte_hash_crc_4byte(*ip_dst2, init_val);
> init_val = rte_hash_crc_4byte(*ip_dst3, init_val);
> init_val = rte_hash_crc_4byte(*p, init_val);
> -#else /* RTE_MACHINE_CPUFLAG_SSE4_2 */
> +#else
> init_val = rte_jhash_1word(t, init_val);
> init_val = rte_jhash(k->ip_src,
> sizeof(uint8_t) * IPV6_ADDR_LEN, init_val);
> init_val = rte_jhash(k->ip_dst,
> sizeof(uint8_t) * IPV6_ADDR_LEN, init_val);
> init_val = rte_jhash_1word(*p, init_val);
> -#endif /* RTE_MACHINE_CPUFLAG_SSE4_2 */
> +#endif
> return init_val;
> }
>
> --
> 1.9.1
>
^ permalink raw reply
* Re: [RFC 0/7] changing mbuf pool handler
From: Hemant Agrawal @ 2016-10-05 11:49 UTC (permalink / raw)
To: Hunt, David, Olivier Matz, dev@dpdk.org; +Cc: jerin.jacob@caviumnetworks.com
In-Reply-To: <cde42672-b7f4-069c-4260-2ac7342ef71e@intel.com>
Hi Olivier,
> -----Original Message-----
> From: Hunt, David [mailto:david.hunt@intel.com]
> Hi Olivier,
>
>
> On 3/10/2016 4:49 PM, Olivier Matz wrote:
> > Hi Hemant,
> >
> > Thank you for your feedback.
> >
> > On 09/22/2016 01:52 PM, Hemant Agrawal wrote:
> >> Hi Olivier
> >>
> >> On 9/19/2016 7:12 PM, Olivier Matz wrote:
> >>> Hello,
> >>>
> >>> Following discussion from [1] ("usages issue with external mempool").
> >>>
> >>> This is a tentative to make the mempool_ops feature introduced by
> >>> David Hunt [2] more widely used by applications.
> >>>
> >>> It applies on top of a minor fix in mbuf lib [3].
> >>>
> >>> To sumarize the needs (please comment if I did not got it properly):
> >>>
> >>> - new hw-assisted mempool handlers will soon be introduced
> >>> - to make use of it, the new mempool API [4]
> (rte_mempool_create_empty,
> >>> rte_mempool_populate, ...) has to be used
> >>> - the legacy mempool API (rte_mempool_create) does not allow to
> change
> >>> the mempool ops. The default is "ring_<s|m>p_<s|m>c" depending on
> >>> flags.
> >>> - the mbuf helper (rte_pktmbuf_pool_create) does not allow to change
> >>> them either, and the default is RTE_MBUF_DEFAULT_MEMPOOL_OPS
> >>> ("ring_mp_mc")
> >>> - today, most (if not all) applications and examples use either
> >>> rte_pktmbuf_pool_create or rte_mempool_create to create the mbuf
> >>> pool, making it difficult to take advantage of this feature with
> >>> existing apps.
> >>>
> >>> My initial idea was to deprecate both rte_pktmbuf_pool_create() and
> >>> rte_mempool_create(), forcing the applications to use the new API,
> >>> which is more flexible. But after digging a bit, it appeared that
> >>> rte_mempool_create() is widely used, and not only for mbufs.
> >>> Deprecating it would have a big impact on applications, and
> >>> replacing it with the new API would be overkill in many use-cases.
> >> I agree with the proposal.
> >>
> >>> So I finally tried the following approach (inspired from a
> >>> suggestion Jerin [5]):
> >>>
> >>> - add a new mempool_ops parameter to rte_pktmbuf_pool_create().
> This
> >>> unfortunatelly breaks the API, but I implemented an ABI compat layer.
> >>> If the patch is accepted, we could discuss how to announce/schedule
> >>> the API change.
> >>> - update the applications and documentation to prefer
> >>> rte_pktmbuf_pool_create() as much as possible
> >>> - update most used examples (testpmd, l2fwd, l3fwd) to add a new
> command
> >>> line argument to select the mempool handler
> >>>
> >>> I hope the external applications would then switch to
> >>> rte_pktmbuf_pool_create(), since it supports most of the use-cases
> >>> (even priv_size != 0, since we can call rte_mempool_obj_iter() after) .
> >>>
> >> I will still prefer if you can add the "rte_mempool_obj_cb_t *obj_cb,
> >> void *obj_cb_arg" into "rte_pktmbuf_pool_create". This single
> >> consolidated wrapper will almost make it certain that applications
> >> will not try to use rte_mempool_create for packet buffers.
> > The patch changes the example applications. I'm not sure I understand
> > why adding these arguments would force application to not use
> > rte_mempool_create() for packet buffers. Do you have a application in
> mind?
> >
> > For the mempool_ops parameter, we must pass it at init because we need
> > to know the mempool handler before populating the pool. For object
> > initialization, it can be done after, so I thought it was better to
> > reduce the number of arguments to avoid to fall in the
> > mempool_create() syndrom :)
>
> I also agree with the proposal. Looks cleaner.
>
> I would lean to the side of keeping the parameters to the minimum, i.e.
> not adding *obj_cb and *obj_cb_arg into rte_pktmbuf_pool_create.
> Developers always have the option of going with rte_mempool_create if they
> need more fine-grained control.
[Hemant] The implementations with hw offloaded mempools don't want developer using *rte_mempool_create* for packet buffer pools.
This API does not work for hw offloaded mempool.
Also, *rte_mempool_create_empty* - may not be convenient for many application, as it requires calling 4+ APIs.
Olivier is not in favor of deprecating the *rte_mempool_create*. I agree with concerns raised by him.
Essentially, I was suggesting to upgrade * rte_pktmbuf_pool_create* to be like *rte_mempool_create* for packet buffers exclusively.
This will provide a clear segregation for API usages w.r.t the packet buffer pool vs all other type of mempools.
Regards,
Hemant
>
> Regards,
> Dave.
>
> > Any other opinions?
> >
> > Regards,
> > Olivier
^ permalink raw reply
* Re: [PATCH 00/19] KNI checkpatch cleanup
From: Pattan, Reshma @ 2016-10-05 10:38 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org; +Cc: Stephen Hemminger, Ferruh Yigit
In-Reply-To: <1473954405-7150-1-git-send-email-ferruh.yigit@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Thursday, September 15, 2016 4:46 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Ferruh Yigit
> <ferruh.yigit@itnel.com>
> Subject: [dpdk-dev] [PATCH 00/19] KNI checkpatch cleanup
>
> KNI checkpatch cleanup, mostly non-functional but cosmetic modifications.
> Only functional change is related logging, switched to kernel dynamic logging
> and compile time KNI debug options removed, some log message levels
> updated.
>
> Ferruh Yigit (19):
> kni: move externs to the header file
> kni: uninitialize global variables
> kni: make static struct const
> kni: whitespace, indentation, long line corrections
> kni: prefer unsigned int to unsigned
> kni: remove useless return
> kni: comparisons should place the constant on the right
> kni: trailing statements should be on next line
> kni: do not use assignment in if condition
> kni: macros with complex values should be enclosed in parentheses
> kni: prefer min_t to min
> kni: prefer ether_addr_copy to memcpy
> kni: update kernel logging
> kni: remove unnecessary 'out of memory' message
> kni: move functions to eliminate function declarations
> kni: remove compile time debug configuration
> kni: updated log messages
> kni: prefer uint32_t to unsigned int
> kni: move kernel version ifdefs to compat header
>
Patches 4,5,11 and 13-19 are failed to apply.
Thanks,
Reshma
^ permalink raw reply
* Re: [PATCH] eal: fix c++ compilation issue with rte_delay_us()
From: Ananyev, Konstantin @ 2016-10-05 10:17 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org
In-Reply-To: <1492896.7EzNbMBhFc@xps13>
Hi Thomas,
>
> 2016-10-03 18:27, Konstantin Ananyev:
> > When compiling with C++, it treats
> > void (*rte_delay_us)(unsigned int us);
> > as definition of the global variable.
> > So further linking with librte_eal fails.
> >
> > Fixes: b4d63fb62240 ("eal: customize delay function")
>
> Applied, thanks
>
> I don't understand why it was not failing with C compilation?
Don't know off hand.
Yes, I would expect gcc to fail with same symptoms too.
But by some reason it puts it makes it a 'common' symbol:
$ cat rttm1.c
#include <stdio.h>
#include <rte_eal.h>
#include <rte_cycles.h>
int main(int argc, char *argv[])
{
int ret = rte_eal_init(argc, argv);
rte_delay_us(1);
printf("return code: %d\n", ret);
return ret;
}
$ gcc -m64 -pthread -o rttm1 rttm1.o -ldl -L/${RTE_SDK}/${RTE_TARGET}/lib -Wl,-lrte_eal
$ nm rttm1.o | grep rte_delay_us
0000000000000008 C rte_delay_us
Konstantin
^ permalink raw reply
* Re: [PATCH v5 0/4] Cuckoo hash enhancements
From: Thomas Monjalon @ 2016-10-05 10:12 UTC (permalink / raw)
To: Pablo de Lara; +Cc: dev, bruce.richardson
In-Reply-To: <1475623515-47587-1-git-send-email-pablo.de.lara.guarch@intel.com>
2016-10-05 00:25, Pablo de Lara:
> This patchset improves lookup performance on the current hash library
> by changing the existing lookup bulk pipeline, with an improved pipeline,
> based on a loop-and-jump model, instead of the current 4-stage 2-entry pipeline.
> Also, x86 vectorized intrinsics are used to improve performance when comparing signatures.
Applied, thanks
^ permalink raw reply
* Re: [PATCH v2]:rte_timer:timer lag issue correction
From: Thomas Monjalon @ 2016-10-05 10:06 UTC (permalink / raw)
To: Karmarkar Suyash; +Cc: Sanford, Robert, dev, reshma.pattan@intel.com
In-Reply-To: <D09D8D35-BE32-48DD-B070-030C590744F4@akamai.com>
> > For Periodic timers ,if the lag gets introduced, the current code
> > added additional delay when the next peridoc timer was initialized
> > by not taking into account the delay added, with this fix the code
> > would start the next occurrence of timer keeping in account the
> > lag added.Corrected the behavior.
> >
> > Fixes: 9b15ba89 ("timer: use a skip list")
> >
> > Karmarkar Suyash (1):
> > Signed-off-by: Karmarkar Suyash <skarmarkar@sonusnet.com>
>
> Yes, this change makes sense. I ran timer tests and they passed.
>
> Acked-by: Robert Sanford <rsanford@akamai.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v2] app/testpmd: fix DCB config issue
From: Iremonger, Bernard @ 2016-10-05 10:02 UTC (permalink / raw)
To: Lu, Wenzhuo, dev@dpdk.org; +Cc: De Lara Guarch, Pablo
In-Reply-To: <1474852271-11309-1-git-send-email-wenzhuo.lu@intel.com>
> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Monday, September 26, 2016 2:11 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Iremonger,
> Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: [PATCH v2] app/testpmd: fix DCB config issue
>
> An issue is found that DCB cannot be configured on ixgbe NICs. It's said the
> TX queue number is not right.
> On ixgbe the max TX queue number is not fixed, it depends on the multi-
> queue mode.
>
> This patch adds the device configuration before getting info in the DCB
> configuration process. So the right info can be got depending on the
> configuration.
>
> Fixes: 1a572499beb6 (app/testpmd: setup DCB forwarding based on traffic
> class)
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Acked-by: Bernard Iremonger <Bernard.iremonger@intel.com>
^ permalink raw reply
* Re: [PATCH v2 1/3] mem: fix hugepage mapping error messages
From: Thomas Monjalon @ 2016-10-05 9:51 UTC (permalink / raw)
To: jean.tourrilhes; +Cc: Sergio Gonzalez Monroy, dev
In-Reply-To: <fc86e7d0-7b91-8c78-f100-37f22b72df16@intel.com>
2016-10-04 20:07, Sergio Gonzalez Monroy:
> On 04/10/2016 18:17, Jean Tourrilhes wrote:
> > Running secondary is tricky due to the need to map the memory region
> > at the right place in VM, which is whatever primary has chosen. If the
> > base address for primary happens to by already mapped in the
> > secondary, we will hit precisely these error messages (depending if we
> > fail on the config region or the hugepages). This is why there is
> > already a comment about ASLR.
> >
> > The issue is that in most cases, remapping does not happen and "errno"
> > is not changed and therefore stale. In our case, we got a "permission
> > denied", which sent us down the wrong track. It's such a common error
> > for secondary that I feel this error message should be unambiguous and
> > helpful.
> > The call to close was also moved because close() may override errno.
> >
> > Signed-off-by: Jean Tourrilhes <jt@labs.hpe.com>
> > ---
> > lib/librte_eal/linuxapp/eal/eal.c | 14 +++++++++++---
> > lib/librte_eal/linuxapp/eal/eal_memory.c | 16 ++++++++++++----
> > 2 files changed, 23 insertions(+), 7 deletions(-)
>
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Applied, thanks
A rebase was necessary because of this patch: http://dpdk.org/commit/c00ae961
Please check everything is OK.
^ permalink raw reply
* Re: [RFC 0/7] changing mbuf pool handler
From: Hunt, David @ 2016-10-05 9:41 UTC (permalink / raw)
To: Olivier Matz, Hemant Agrawal, dev; +Cc: jerin.jacob
In-Reply-To: <dc89a863-3f9e-c0e9-a851-04de2f6e08fe@6wind.com>
Hi Olivier,
On 3/10/2016 4:49 PM, Olivier Matz wrote:
> Hi Hemant,
>
> Thank you for your feedback.
>
> On 09/22/2016 01:52 PM, Hemant Agrawal wrote:
>> Hi Olivier
>>
>> On 9/19/2016 7:12 PM, Olivier Matz wrote:
>>> Hello,
>>>
>>> Following discussion from [1] ("usages issue with external mempool").
>>>
>>> This is a tentative to make the mempool_ops feature introduced
>>> by David Hunt [2] more widely used by applications.
>>>
>>> It applies on top of a minor fix in mbuf lib [3].
>>>
>>> To sumarize the needs (please comment if I did not got it properly):
>>>
>>> - new hw-assisted mempool handlers will soon be introduced
>>> - to make use of it, the new mempool API [4] (rte_mempool_create_empty,
>>> rte_mempool_populate, ...) has to be used
>>> - the legacy mempool API (rte_mempool_create) does not allow to change
>>> the mempool ops. The default is "ring_<s|m>p_<s|m>c" depending on
>>> flags.
>>> - the mbuf helper (rte_pktmbuf_pool_create) does not allow to change
>>> them either, and the default is RTE_MBUF_DEFAULT_MEMPOOL_OPS
>>> ("ring_mp_mc")
>>> - today, most (if not all) applications and examples use either
>>> rte_pktmbuf_pool_create or rte_mempool_create to create the mbuf
>>> pool, making it difficult to take advantage of this feature with
>>> existing apps.
>>>
>>> My initial idea was to deprecate both rte_pktmbuf_pool_create() and
>>> rte_mempool_create(), forcing the applications to use the new API, which
>>> is more flexible. But after digging a bit, it appeared that
>>> rte_mempool_create() is widely used, and not only for mbufs. Deprecating
>>> it would have a big impact on applications, and replacing it with the
>>> new API would be overkill in many use-cases.
>> I agree with the proposal.
>>
>>> So I finally tried the following approach (inspired from a suggestion
>>> Jerin [5]):
>>>
>>> - add a new mempool_ops parameter to rte_pktmbuf_pool_create(). This
>>> unfortunatelly breaks the API, but I implemented an ABI compat layer.
>>> If the patch is accepted, we could discuss how to announce/schedule
>>> the API change.
>>> - update the applications and documentation to prefer
>>> rte_pktmbuf_pool_create() as much as possible
>>> - update most used examples (testpmd, l2fwd, l3fwd) to add a new command
>>> line argument to select the mempool handler
>>>
>>> I hope the external applications would then switch to
>>> rte_pktmbuf_pool_create(), since it supports most of the use-cases (even
>>> priv_size != 0, since we can call rte_mempool_obj_iter() after) .
>>>
>> I will still prefer if you can add the "rte_mempool_obj_cb_t *obj_cb,
>> void *obj_cb_arg" into "rte_pktmbuf_pool_create". This single
>> consolidated wrapper will almost make it certain that applications will
>> not try to use rte_mempool_create for packet buffers.
> The patch changes the example applications. I'm not sure I understand
> why adding these arguments would force application to not use
> rte_mempool_create() for packet buffers. Do you have a application in mind?
>
> For the mempool_ops parameter, we must pass it at init because we need
> to know the mempool handler before populating the pool. For object
> initialization, it can be done after, so I thought it was better to
> reduce the number of arguments to avoid to fall in the mempool_create()
> syndrom :)
I also agree with the proposal. Looks cleaner.
I would lean to the side of keeping the parameters to the minimum, i.e.
not adding *obj_cb and *obj_cb_arg into rte_pktmbuf_pool_create.
Developers always have the option of going with rte_mempool_create if
they need more fine-grained control.
Regards,
Dave.
> Any other opinions?
>
> Regards,
> Olivier
^ permalink raw reply
* Re: [PATCH v2]:rte_timer:timer lag issue correction
From: Pattan, Reshma @ 2016-10-05 9:34 UTC (permalink / raw)
To: Karmarkar Suyash; +Cc: Sanford, Robert, dev@dpdk.org, thomas.monjalon@6wind.com
In-Reply-To: <BN3PR03MB1431F25DD93D4148C1493320B3C50@BN3PR03MB1431.namprd03.prod.outlook.com>
Hi Suyash,
> -----Original Message-----
> From: Karmarkar Suyash [mailto:skarmarkar@sonusnet.com]
> Sent: Tuesday, October 4, 2016 11:36 PM
> To: Sanford, Robert <rsanford@akamai.com>; dev@dpdk.org;
> thomas.monjalon@6wind.com; Pattan, Reshma <reshma.pattan@intel.com>
> Subject: RE: [PATCH v2]:rte_timer:timer lag issue correction
>
> Thanks !! So as next steps I will push the patch .
>
Thomas will apply the patch, you no need to do anything.
Thanks,
Reshma
^ permalink raw reply
* Re: [PATCH] test_cryptodev_perf: IV and digest should be stored at a DMAeble address
From: Kusztal, ArkadiuszX @ 2016-10-05 9:26 UTC (permalink / raw)
To: Akhil Goyal, dev@dpdk.org, Doherty, Declan
Cc: Jain, Deepak K, Trahe, Fiona, Griffin, John
In-Reply-To: <17b5c942-4b83-2ef0-a0f7-6755eaf73383@nxp.com>
Hi Akhil,
Could you rebase it against newest next-crypto subtree, there were changes with function names in the meantime.
And to make it really consistent across all hw tests could you add this change to qat_snow3g too,
for snow3g I assume aad need to obtain correct physical address too.
Regards,
Arek
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
> Sent: Wednesday, October 05, 2016 7:40 AM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] test_cryptodev_perf: IV and digest should
> be stored at a DMAeble address
>
> On 9/26/2016 10:03 PM, akhil.goyal@nxp.com wrote:
> > From: Akhil Goyal <akhil.goyal@nxp.com>
> >
> > For physical crypto devices, IV and digest are processed by the crypto
> > device which need the contents to be written on some DMA able address.
> >
> > So in order to do that, IV and digest are accomodated in the packet.
> >
> > Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> > ---
> > app/test/test_cryptodev_perf.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test/test_cryptodev_perf.c
> > b/app/test/test_cryptodev_perf.c index 0ea7ec1..930d5b8 100644
> > --- a/app/test/test_cryptodev_perf.c
> > +++ b/app/test/test_cryptodev_perf.c
> > @@ -2366,9 +2366,13 @@ test_perf_set_crypto_op(struct rte_crypto_op
> *op, struct rte_mbuf *m,
> > op->sym->auth.aad.length = AES_CBC_CIPHER_IV_LENGTH;
> >
> > /* Cipher Parameters */
> > - op->sym->cipher.iv.data = aes_cbc_iv;
> > + op->sym->cipher.iv.data = (uint8_t *)m->buf_addr + m->data_off;
> > + op->sym->cipher.iv.phys_addr = rte_pktmbuf_mtophys(m);
> > op->sym->cipher.iv.length = AES_CBC_CIPHER_IV_LENGTH;
> >
> > + rte_memcpy(op->sym->cipher.iv.data, aes_cbc_iv,
> > + AES_CBC_CIPHER_IV_LENGTH);
> > +
> > /* Data lengths/offsets Parameters */
> > op->sym->auth.data.offset = 0;
> > op->sym->auth.data.length = data_len; @@ -2468,7 +2472,9 @@
> > test_perf_aes_sha(uint8_t dev_id, uint16_t queue_id,
> > rte_pktmbuf_free(mbufs[k]);
> > return -1;
> > }
> > -
> > + /* Make room for Digest and IV in mbuf */
> > + rte_pktmbuf_append(mbufs[i], digest_length);
> > + rte_pktmbuf_prepend(mbufs[i],
> AES_CBC_CIPHER_IV_LENGTH);
> > }
> >
> >
> >
> Hi Declan,
>
> Sorry I missed out copy your name in the TO list. Do we have some
> comments on this patch.
>
> Regards,
> Akhil
^ permalink raw reply
* Re: [PATCH] eal: fix c++ compilation issue with rte_delay_us()
From: Thomas Monjalon @ 2016-10-05 9:23 UTC (permalink / raw)
To: Konstantin Ananyev; +Cc: dev
In-Reply-To: <1475515645-24667-1-git-send-email-konstantin.ananyev@intel.com>
2016-10-03 18:27, Konstantin Ananyev:
> When compiling with C++, it treats
> void (*rte_delay_us)(unsigned int us);
> as definition of the global variable.
> So further linking with librte_eal fails.
>
> Fixes: b4d63fb62240 ("eal: customize delay function")
Applied, thanks
I don't understand why it was not failing with C compilation?
^ permalink raw reply
* Re: [PATCH 2/2] kni: remove unnecessary ethtool files
From: Remy Horton @ 2016-10-05 8:58 UTC (permalink / raw)
To: Ferruh Yigit, dev
In-Reply-To: <20160930101030.1178-2-ferruh.yigit@intel.com>
On 30/09/2016 11:10, Ferruh Yigit wrote:
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> lib/librte_eal/linuxapp/kni/Makefile | 3 -
> .../linuxapp/kni/ethtool/igb/igb_debugfs.c | 28 --
> .../linuxapp/kni/ethtool/igb/igb_hwmon.c | 260 ---------------
> .../linuxapp/kni/ethtool/igb/igb_procfs.c | 363 ---------------------
> 4 files changed, 654 deletions(-)
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_debugfs.c
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_hwmon.c
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_procfs.c
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply
* Re: [PATCH 1/2] kni: remove unused ethtool files
From: Remy Horton @ 2016-10-05 8:58 UTC (permalink / raw)
To: Ferruh Yigit, dev
In-Reply-To: <20160930101030.1178-1-ferruh.yigit@intel.com>
On 30/09/2016 11:10, Ferruh Yigit wrote:
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> lib/librte_eal/linuxapp/kni/Makefile | 2 -
> lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ptp.c | 944 -------------
> lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.c | 1482 --------------------
> .../linuxapp/kni/ethtool/igb/kcompat_ethtool.c | 1171 ----------------
> .../linuxapp/kni/ethtool/ixgbe/ixgbe_sriov.h | 73 -
> 5 files changed, 3672 deletions(-)
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ptp.c
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.c
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat_ethtool.c
> delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_sriov.h
Acked-by: Remy Horton <remy.horton@intel.com>
^ permalink raw reply
* Re: [PATCH v2] cryptodev: fix compilation error in SUSE 11 SP2
From: Adrien Mazarguil @ 2016-10-05 8:51 UTC (permalink / raw)
To: Pablo de Lara; +Cc: dev, declan.doherty
In-Reply-To: <1475635551-121997-1-git-send-email-pablo.de.lara.guarch@intel.com>
On Wed, Oct 05, 2016 at 03:45:51AM +0100, Pablo de Lara wrote:
> This commit fixes following build error, which happens in SUSE 11 SP2,
> with gcc 4.5.1:
>
> In file included from lib/librte_cryptodev/rte_cryptodev.c:70:0:
> lib/librte_cryptodev/rte_cryptodev.h:772:7:
> error: flexible array member in otherwise empty struct
>
> Fixes: 347a1e037fd3 ("lib: use C99 syntax for zero-size arrays")
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>
> Changes in v2:
> - Fixed commit message
>
> lib/librte_cryptodev/rte_cryptodev.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> index d565f39..6ad5e91 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -773,7 +773,7 @@ struct rte_cryptodev_sym_session {
> } __rte_aligned(8);
> /**< Public symmetric session details */
>
> - char _private[];
> + __extension__ char _private[0];
> /**< Private session material */
> };
>
> --
> 2.7.4
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* [PATCH v2] doc: arm64: document DPDK application profiling methods
From: Jerin Jacob @ 2016-10-05 8:43 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, jianbo.liu, viktorin, Jerin Jacob
In-Reply-To: <1475577630-14318-1-git-send-email-jerin.jacob@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
v2:
-Addressed ARM64 specific review comments(Suggested by Thomas)
http://dpdk.org/dev/patchwork/patch/16362/
---
doc/guides/prog_guide/profile_app.rst | 58 +++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/doc/guides/prog_guide/profile_app.rst b/doc/guides/prog_guide/profile_app.rst
index 3226187..9f1b7ee 100644
--- a/doc/guides/prog_guide/profile_app.rst
+++ b/doc/guides/prog_guide/profile_app.rst
@@ -31,6 +31,14 @@
Profile Your Application
========================
+Introduction
+------------
+
+The following sections describe the methods to profile DPDK applications on
+different architectures.
+
+x86
+~~~
Intel processors provide performance counters to monitor events.
Some tools provided by Intel can be used to profile and benchmark an application.
See the *VTune Performance Analyzer Essentials* publication from Intel Press for more information.
@@ -50,3 +58,53 @@ The main situations that should be monitored through event counters are:
Refer to the
`Intel Performance Analysis Guide <http://software.intel.com/sites/products/collateral/hpc/vtune/performance_analysis_guide.pdf>`_
for details about application profiling.
+
+ARM64
+~~~~~
+
+Perf
+^^^^
+ARM64 architecture provide performance counters to monitor events.
+The Linux perf tool can be used to profile and benchmark an application.
+In addition to the standard events, perf can be used to profile arm64 specific
+PMU events through raw events(-e -rXX)
+
+Refer to the
+`ARM64 specific PMU events enumeration <http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_0002_04_en/way1382543438508.html>`_
+
+High-resolution cycle counter
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+The default cntvct_el0 based rte_rdtsc() provides portable means to get wall
+clock counter at user space. Typically it runs at <= 100MHz.
+
+The alternative method to enable rte_rdtsc() for high resolution
+wall clock counter is through armv8 PMU subsystem.
+The PMU cycle counter runs at CPU frequency, However, access to PMU cycle
+counter from user space is not enabled by default in the arm64 linux kernel.
+It is possible to enable cycle counter at user space access
+by configuring the PMU from the privileged mode (kernel space).
+
+by default rte_rdtsc() implementation uses portable cntvct_el0 scheme.
+Application can choose the PMU based implementation with
+CONFIG_RTE_ARM_EAL_RDTSC_USE_PMU
+
+Find below the example steps to configure the PMU based cycle counter on an
+armv8 machine.
+
+.. code-block:: console
+
+ git clone https://github.com/jerinjacobk/armv8_pmu_cycle_counter_el0
+ cd armv8_pmu_cycle_counter_el0
+ make
+ sudo insmod pmu_el0_cycle_counter.ko
+ cd $DPDK_DIR
+ make config T=arm64-armv8a-linuxapp-gcc
+ echo "CONFIG_RTE_ARM_EAL_RDTSC_USE_PMU=y" >> build/.config
+ make
+
+.. warning::
+
+ The PMU based scheme is useful for high accuracy performance profiling with
+ rte_rdtsc(). However, This method can not be used in conjunction with Linux
+ userspace profiling tools like perf as this scheme alters the PMU registers
+ state.
--
2.5.5
^ permalink raw reply related
* Re: [PATCH v7] net/virtio: add set_mtu in virtio
From: Kavanagh, Mark B @ 2016-10-05 8:15 UTC (permalink / raw)
To: Dey, Souvik, yuanhan.liu@linux.intel.com,
stephen@networkplumber.org
Cc: dev@dpdk.org
In-Reply-To: <BN3PR03MB1494CD67AB36557559F2CCCADAC50@BN3PR03MB1494.namprd03.prod.outlook.com>
>Hi All,
> Is there any further comments or modifications required for this patch, or what next
>steps do you guys suggest here ?
Hi Souvik,
Some minor comments inline.
Thanks,
Mark
>
>--
>Regards,
>Souvik
>
>-----Original Message-----
>From: Dey, Souvik
>Sent: Saturday, October 1, 2016 10:09 AM
>To: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; stephen@networkplumber.org;
>dev@dpdk.org
>Subject: RE: [PATCH v7] net/virtio: add set_mtu in virtio
>
>Hi Liu/Stephen/Mark,
>
> I have submitted Version 7 of this patch. Do let me know if this looks proper.
>
>--
>Regards,
>Souvik
>
>-----Original Message-----
>From: Dey, Souvik
>Sent: Thursday, September 29, 2016 4:32 PM
>To: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; stephen@networkplumber.org;
>dev@dpdk.org
>Cc: Dey, Souvik <sodey@sonusnet.com>
>Subject: [PATCH v7] net/virtio: add set_mtu in virtio
>
>
>Virtio interfaces do not currently allow the user to specify a particular
>Maximum Transmission Unit (MTU).Consequently, the MTU of Virtio interfaces
>is typically set to the Ethernet default value of 1500.
>This is problematic in the case of cloud deployments, in which a specific
>(and potentially non-standard) MTU needs to be set by a DHCP server, which
>needs to be honored by all interfaces across the traffic path.To achieve
>this Virtio interfaces should support setting of MTU.
>In case when GRE/VXLAN tunneling is used for internal communication, there
>will be an overhead added by the infrastructure in the packet over and
>above the ETHER MTU of 1518. So to take care of this overhead in these
>cases the DHCP server corrects the L3 MTU to 1454. But since virtio
>interfaces was not having the MTU set functionality that MTU sent by the
>DHCP server was ignored and the instance will still send packets with 1500
>MTU which after encapsulation will become more than 1518 and eventually
>gets dropped in the infrastructure.
>By adding an additional 'set_mtu' function to the Virtio driver, we can
>honor the MTU sent by the DHCP server. The dhcp server/controller can
>then leverage this 'set_mtu' functionality to resolve the above
>mentioned issue of packets getting dropped due to incorrect size.
>
>
>Signed-off-by: Souvik Dey <sodey@sonusnet.com>
>
>---
>Changes in v7:
>- Replaced the CRC_LEN with the merge rx buf header length.
>- Changed the frame_len max validation to VIRTIO_MAX_RX_PKTLEN.
>Changes in v6:
>- Description of change corrected
>- Corrected the identations
>- Corrected the subject line too
>- The From line was also not correct
>- Re-submitting as the below patch was not proper
>Changes in v5:
>- Fix log message for out-of-bounds MTU parameter in virtio_mtu_set
>- Calculate frame size, based on 'mtu' parameter
>- Corrected the upper bound and lower bound checks in virtio_mtu_set
>Changes in v4: Incorporated review comments.
>Changes in v3: Corrected few style errors as reported by sys-stv.
>Changes in v2: Incorporated review comments.
>
> drivers/net/virtio/virtio_ethdev.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>index 423c597..1dbfea6 100644
>--- a/drivers/net/virtio/virtio_ethdev.c
>+++ b/drivers/net/virtio/virtio_ethdev.c
>@@ -653,12 +653,20 @@ virtio_dev_allmulticast_disable(struct rte_eth_dev *dev)
> PMD_INIT_LOG(ERR, "Failed to disable allmulticast");
> }
>
>+#define VLAN_TAG_LEN 4 /* 802.3ac tag (not DMA'd) */
There should be a blank line between the #define and the function prototype beneath.
>+static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
>+{
>+ struct virtio_hw *hw = dev->data->dev_private;
>+ uint32_t ether_hdr_len = ETHER_HDR_LEN + VLAN_TAG_LEN +
>+ hw->vtnet_hdr_size;
I'll rely on Stephen and Yuanhan's judgment for this.
>+ uint32_t frame_size = mtu + ether_hdr_len;
>+
>+ if (mtu < ETHER_MIN_MTU || frame_size > VIRTIO_MAX_RX_PKTLEN) {
>+ PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n",
>+ ETHER_MIN_MTU, VIRTIO_MAX_RX_PKTLEN);
Shouldn't last format print parameter should be (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len)?
i.e PMD_INIT_LOG(ERR, "MTU should........%d\n",
ETHER_MIN_MTU, (VIRTIO_MAX_RX_PKTLEN - ether_hdr_len));
>+ return -EINVAL;
>+ }
>+ return 0;
>+}
>
> /*
> * dev_ops for virtio, bare necessities for basic operation
> */
>@@ -677,7 +685,6 @@ static const struct eth_dev_ops virtio_eth_dev_ops = {
> .allmulticast_enable = virtio_dev_allmulticast_enable,
> .allmulticast_disable = virtio_dev_allmulticast_disable,
>+ .mtu_set = virtio_mtu_set,
> .dev_infos_get = virtio_dev_info_get,
> .stats_get = virtio_dev_stats_get,
> .xstats_get = virtio_dev_xstats_get,
>--
>2.9.3.windows.1
^ permalink raw reply
* Re: [PATCH 1/1] eal: Don't fail secondary if primary is missing tailqs
From: David Marchand @ 2016-10-05 7:58 UTC (permalink / raw)
To: jean.tourrilhes; +Cc: Sergio Gonzalez Monroy, dev@dpdk.org
In-Reply-To: <20161004165930.GA2012@labs.hpe.com>
Hello,
On Tue, Oct 4, 2016 at 6:59 PM, Jean Tourrilhes <jt@labs.hpe.com> wrote:
> On Tue, Oct 04, 2016 at 02:11:39PM +0100, Sergio Gonzalez Monroy wrote:
>> The case you are trying to fix is, as an example, when your secondary app is
>> using LPM but your primary is not.
>> So basically with this patch, you are removing the tailq for LPM on
>> secondary and continuing as normal, is that the case?
>
> The secondary can't use tailq types that the primary does not
> have, because they are shared across the shared memory.
I am not a "multi process" user but afaik the primary process is
responsible for filling the shared memory.
The secondary processes look at it.
So having unaligned processes can't work.
> What happens is that the primary and secondary did not compile
> in the same list of tailq. See previous e-mail :
> http://dpdk.org/ml/archives/dev/2016-September/047329.html
> The reason it's happening is that the secondary was not
> compiled with the DPDK build system, but with the build system of the
> application (in this case, Snort). Oubviously, porting the application
> to the DPDK build system is not practical, so we need to live with
> this case.
> The build system of the application does not have all the
> subtelties of the DPDK build system, and ends up including *all* the
> constructors, wether they are used or not in the code. Moreover, they
> are included in a different order. Actually, by default the builds
> include no constructors at all (which is a big fail), so the library
> needs to be included with --whole-archive (see Snort DPDK
> instructions).
I thought you had unaligned binaries.
You are compiling only one binary ?
>> I am not convinced about this approach.
>
> I agree that the whole constructor approach is flaky and my
> patch is only a band aid. Constructors should be entirely removed
> IMHO, and a more deterministic init method should be used instead of
> depending on linker magic.
> Note that the other constructors happen to work right in my
> case, but that's probably pure luck. The list of mempool constructors
> happen to be the same and in the same order (order matters for mempool
> constructors). The app is not using spinlock, hash, crc and acl, so
> I did not look if the lists did match.
I am not sure Sergio is talking about the constructor approach.
Anyway, the constructors invocation order should not matter.
Primary and secondary processes build their local tailq entries list
in constructors (so far, I can't see how this is wrong).
"Later", each process updates this list with the actual pointer to the
lists by looking at the shared memory in rte_eal_init (calling
rte_eal_tailqs_init).
What matters is that secondary tailqs are a subset of the primary tailqs.
I still have some trouble understanding what you are trying to do.
As Sergio asked, can you come up with a simplified example/use case ?
Thanks.
--
David Marchand
^ permalink raw reply
* Re: [RFC] libeventdev: event driven programming model framework for DPDK
From: Jerin Jacob @ 2016-10-05 7:24 UTC (permalink / raw)
To: Vangati, Narender; +Cc: dev@dpdk.org
In-Reply-To: <3B5C8B5D2B969C4CBE78502867F1AD9A0134149F@FMSMSX108.amr.corp.intel.com>
On Tue, Oct 04, 2016 at 09:49:52PM +0000, Vangati, Narender wrote:
> Hi Jerin,
Hi Narender,
Thanks for the comments.I agree with proposed changes; I will address these comments in v2.
/Jerin
>
>
>
> Here are some comments on the libeventdev RFC.
>
> These are collated thoughts after discussions with you & others to understand the concepts and rationale for the current proposal.
>
>
>
> 1. Concept of flow queues. This is better abstracted as flow ids and not as flow queues which implies there is a queueing structure per flow. A s/w implementation can do atomic load balancing on multiple flow ids more efficiently than maintaining each event in a specific flow queue.
>
>
>
> 2. Scheduling group. A scheduling group is more a steam of events, so an event queue might be a better abstraction.
>
>
>
> 3. An event queue should support the concept of max active atomic flows (maximum number of active flows this queue can track at any given time) and max active ordered sequences (maximum number of outstanding events waiting to be egress reordered by this queue). This allows a scheduler implementation to dimension/partition its resources among event queues.
>
>
>
> 4. An event queue should support concept of a single consumer. In an application, a stream of events may need to be brought together to a single core for some stages of processing, e.g. for TX at the end of the pipeline to avoid NIC reordering of the packets. Having a 'single consumer' event queue for that stage allows the intensive scheduling logic to be short circuited and can improve throughput for s/w implementations.
>
>
>
> 5. Instead of tying eventdev access to an lcore, a higher level of abstraction called event port is needed which is the application i/f to the eventdev. Event ports are connected to event queues and is the object the application uses to dequeue and enqueue events. There can be more than one event port per lcore allowing multiple lightweight threads to have their own i/f into eventdev, if the implementation supports it. An event port abstraction also encapsulates dequeue depth and enqueue depth for a scheduler implementations which can schedule multiple events at a time and output events that can be buffered.
>
>
>
> 6. An event should support priority. Per event priority is useful for segregating high priority (control messages) traffic from low priority within the same flow. This needs to be part of the event definition for implementations which support it.
>
>
>
> 7. Event port to event queue servicing priority. This allows two event ports to connect to the same event queue with different priorities. For implementations which support it, this allows a worker core to participate in two different workflows with different priorities (workflow 1 needing 3.5 cores, workflow 2 needing 2.5 cores, and so on).
>
>
>
> 8. Define the workflow as schedule/dequeue/enqueue. An implementation is free to define schedule as NOOP. A distributed s/w scheduler can use this to schedule events; also a centralized s/w scheduler can make this a NOOP on non-scheduler cores.
>
>
>
> 9. The schedule_from_group API does not fit the workflow.
>
>
>
> 10. The ctxt_update/ctxt_wait breaks the normal workflow. If the normal workflow is a dequeue -> do work based on event type -> enqueue, a pin_event argument to enqueue (where the pinned event is returned through the normal dequeue) allows application workflow to remain the same whether or not an implementation supports it.
>
>
>
> 11. Burst dequeue/enqueue needed.
>
>
>
> 12. Definition of a closed/open system - where open system is memory backed and closed system eventdev has limited capacity. In such systems, it is also useful to denote per event port how many packets can be active in the system. This can serve as a threshold for ethdev like devices so they don't overwhelm core to core events.
>
>
>
> 13. There should be sort of device capabilities definition to address different implementations.
>
>
>
>
> vnr
> ---
>
^ permalink raw reply
* Re: qos: traffic shaping at queue level
From: Nikhil Jagtap @ 2016-10-05 7:09 UTC (permalink / raw)
To: Dumitrescu, Cristian; +Cc: dev@dpdk.org, users@dpdk.org
In-Reply-To: <3EB4FA525960D640B5BDFFD6A3D8912647A8D693@IRSMSX108.ger.corp.intel.com>
Hi Cristian,
Thanks for the info. A few more comments/questions inline.
On 3 October 2016 at 23:42, Dumitrescu, Cristian <
cristian.dumitrescu@intel.com> wrote:
>
>
>
>
> *From:* Nikhil Jagtap [mailto:nikhil.jagtap@gmail.com]
> *Sent:* Friday, September 30, 2016 7:12 AM
> *To:* dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> users@dpdk.org
> *Subject:* Re: qos: traffic shaping at queue level
>
>
>
> Hi,
>
> Can someone please answer my queries?
>
> I tried using queue weights to distribute traffic-class bandwidth among
> the child queues, but did not get the desired results.
>
> [Cristian] Can you please describe what issues you see?
>
[Nikhil] At the end of a 20 minute test, the total number of packets
dequeued from the respective queues were not in the ratio 1:5.
In one other test where 4 equal-rate traffic-streams were hitting 4
different queues of the same TC configured with weights 1:2:4:8, I observed
that the queue with highest weight had the least number of dequeued packets
when in theory it should have been the one with highest packet count.
Regards,
>
> Nikhil
>
>
>
> On 27 September 2016 at 15:34, Nikhil Jagtap <nikhil.jagtap@gmail.com>
> wrote:
>
> Hi,
>
>
>
> I have a few questions about the hierarchical scheduler. I am taking a
> simple example here to get a better understanding.
>
>
>
> Reference example:
>
> pipe rate = 30 mbps
>
> tc 0 rate = 30 mbps
>
> traffic-type 0 being queued to queue 0, tc 0.
>
> traffic-type 1 being queued to queue 1, tc 0.
>
> Assume traffic-type 0 is being received at the rate of 25 mbps.
>
> Assume traffic-type 1 is also being received at the rate of 25 mbps.
>
>
>
> Requirement:
>
> To limit traffic-type 0 to (CIR = 5 mbps, PIR = 30 mbps), AND
>
> limit traffic-type 1 to (CIR = 25 mbps, PIR = 30 mbps).
>
>
>
> The questions:
>
> 1) I understand that with the scheduler, it is possible to do rate
> limiting only at the sub-port and pipe levels and not at the individual
> queue level.
>
> [Cristian] Yes, correct, only subports and pipes own token buckets, with
> all the pipe traffic classes and queues sharing their pipe token bucket.
>
>
>
> Is it possible to achieve rate limiting using the notion of queue weights?
> For the above example, will assigning weights in 1:5 ratio to the two
> queues help achieve shaping the two traffic-types at the two different
> rates?
>
> [Cristian] Yes. However, getting the weight observed accurately relies on
> all the queues being backlogged (always having packets to dequeue). When a
> pipe and certain TC is examined for dequeuing, the relative weights are
> enforced between the queues that have packets at that precise moment in
> time, with the empty queues being ignored. The fully backlogged scenario is
> not taking place in practice, and the set of non-empty queues changes over
> time. As said it the past, having big relative weight ratios between queues
> helps (1:5 should be good).
>
> [Nikhil] I see. So I guess not having fully backlogged queues could be one
of the reasons for the observations I mentioned above where the
weights-ratio does not directly translate into rate-ratio. I think I should
also mention that there was no pipelining i.e. packet-processing, queueing,
dequeing was all being done inline in a run-to-completion model.
a) Would having some kind of pipelining help achieve better rate-ratio? May
be say atleast splitting the enqueue and dequeue operations?
b) If pipelining is not an option, what would be the recommended values for
enqueue and dequeue packet count in the run-to-completion model? You have
mentioned in one of your presentations to use different values for these
two. If I go with (enqueue# > dequeue#), don't I run the risk of filling up
the scheduler queues and failed enqueues even at rates lower than the
scheduler pipe rates? In the other case where (dequeue# > enqueue#), we
would end up dequeing all packets that were enqueued every time.
>
>
> 2) In continuation to previous question: if queue weights don't help,
> would it be possible to use metering to achieve rate limiting? Assume we
> meter individual traffic-types (using CIR-PIR config mentioned above)
> before queuing it to the scheduler queues. So to achieve the respective
> queue rates, the dequeuer would be expected to prioritise green packets
> over yellow.
>
> Looking into the code, the packet color is used as an input to the dropper
> block, but does not seem to be used anywhere in the scheduler. So I guess
> it is not possible to prioritise green packets when dequeing?
>
> [Cristian] Packet color is used by Weighted RED (WRED) congestion
> management scheme on the enqueue side, not on the dequeue side. Once the
> packet has been enqueued, it cannot be dropped (i.e. every enqueued packet
> will eventually be dequeued), so rate limiting cannot be enforced on the
> dequeue side.
>
>
>
> Regards,
>
> Nikhil
>
>
>
>
>
Thanks.
Nikhil
^ permalink raw reply
* Re: [PATCH v2 1/8] mbuf: add function to dump ol flag list
From: De Lara Guarch, Pablo @ 2016-10-05 6:45 UTC (permalink / raw)
To: Olivier Matz, dev@dpdk.org
In-Reply-To: <1473407734-11253-2-git-send-email-olivier.matz@6wind.com>
Hi Olivier,
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Friday, September 09, 2016 12:55 AM
> To: dev@dpdk.org; De Lara Guarch, Pablo
> Subject: [PATCH v2 1/8] mbuf: add function to dump ol flag list
>
> The functions rte_get_rx_ol_flag_name() and rte_get_tx_ol_flag_name()
> can dump one flag, or set of flag that are part of the same mask (ex:
> PKT_TX_UDP_CKSUM, part of PKT_TX_L4_MASK). But they are not designed
> to
> dump the list of flags contained in mbuf->ol_flags.
>
> This commit introduce new functions to do that. Similarly to the packet
> type dump functions, the goal is to factorize the code that could be
> used in several applications and reduce the risk of desynchronization
> between the flags and the dump functions.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> doc/guides/rel_notes/release_16_11.rst | 5 ++
> lib/librte_mbuf/rte_mbuf.c | 91
> ++++++++++++++++++++++++++++++++++
> lib/librte_mbuf/rte_mbuf.h | 28 +++++++++++
> lib/librte_mbuf/rte_mbuf_version.map | 2 +
> 4 files changed, 126 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_16_11.rst
> b/doc/guides/rel_notes/release_16_11.rst
> index 36111f3..a877e58 100644
> --- a/doc/guides/rel_notes/release_16_11.rst
> +++ b/doc/guides/rel_notes/release_16_11.rst
> @@ -50,6 +50,11 @@ New Features
>
> Added new functions ``rte_get_ptype_*()`` to dump a packet type as a
> string.
>
> +* **Added functions to dump the offload flags as a string.**
> +
> + Added two new functions ``rte_get_rx_ol_flag_list()`` and
> + ``rte_get_tx_ol_flag_list()`` to dump offload flags as a string.
> +
> Resolved Issues
> ---------------
>
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index fb2b962..56f37e6 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -319,6 +319,53 @@ const char *rte_get_rx_ol_flag_name(uint64_t mask)
> }
> }
>
> +struct flag_mask {
> + uint64_t flag;
> + uint64_t mask;
> + const char *default_name;
> +};
> +
> +/* write the list of rx ol flags in buffer buf */
> +int rte_get_rx_ol_flag_list(uint64_t mask, char *buf, size_t buflen)
Return type should go in a separate line (same case in the other function).
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH] test_cryptodev_perf: IV and digest should be stored at a DMAeble address
From: Akhil Goyal @ 2016-10-05 6:40 UTC (permalink / raw)
To: dev, Declan Doherty
In-Reply-To: <20160926163300.22990-3-akhil.goyal@nxp.com>
On 9/26/2016 10:03 PM, akhil.goyal@nxp.com wrote:
> From: Akhil Goyal <akhil.goyal@nxp.com>
>
> For physical crypto devices, IV and digest are processed by the crypto
> device which need the contents to be written on some DMA able address.
>
> So in order to do that, IV and digest are accomodated in the packet.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> ---
> app/test/test_cryptodev_perf.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c
> index 0ea7ec1..930d5b8 100644
> --- a/app/test/test_cryptodev_perf.c
> +++ b/app/test/test_cryptodev_perf.c
> @@ -2366,9 +2366,13 @@ test_perf_set_crypto_op(struct rte_crypto_op *op, struct rte_mbuf *m,
> op->sym->auth.aad.length = AES_CBC_CIPHER_IV_LENGTH;
>
> /* Cipher Parameters */
> - op->sym->cipher.iv.data = aes_cbc_iv;
> + op->sym->cipher.iv.data = (uint8_t *)m->buf_addr + m->data_off;
> + op->sym->cipher.iv.phys_addr = rte_pktmbuf_mtophys(m);
> op->sym->cipher.iv.length = AES_CBC_CIPHER_IV_LENGTH;
>
> + rte_memcpy(op->sym->cipher.iv.data, aes_cbc_iv,
> + AES_CBC_CIPHER_IV_LENGTH);
> +
> /* Data lengths/offsets Parameters */
> op->sym->auth.data.offset = 0;
> op->sym->auth.data.length = data_len;
> @@ -2468,7 +2472,9 @@ test_perf_aes_sha(uint8_t dev_id, uint16_t queue_id,
> rte_pktmbuf_free(mbufs[k]);
> return -1;
> }
> -
> + /* Make room for Digest and IV in mbuf */
> + rte_pktmbuf_append(mbufs[i], digest_length);
> + rte_pktmbuf_prepend(mbufs[i], AES_CBC_CIPHER_IV_LENGTH);
> }
>
>
>
Hi Declan,
Sorry I missed out copy your name in the TO list. Do we have some
comments on this patch.
Regards,
Akhil
^ permalink raw reply
* Re: [PATCH] examples/ipsec-secgw: Update checksum while decrementing ttl
From: Akhil Goyal @ 2016-10-05 6:32 UTC (permalink / raw)
To: De Lara Guarch, Pablo, Gonzalez Monroy, Sergio, dev@dpdk.org
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA045FF@IRSMSX108.ger.corp.intel.com>
On 10/5/2016 6:04 AM, De Lara Guarch, Pablo wrote:
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez
>> Monroy
>> Sent: Monday, September 26, 2016 6:28 AM
>> To: akhil.goyal@nxp.com; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] examples/ipsec-secgw: Update checksum
>> while decrementing ttl
>>
>> Hi Akhil,
>>
>> This application relies on checksum offload in both outbound and inbound
>> paths (PKT_TX_IP_CKSUM flag).
[Akhil]Agreed that the application relies on checksum offload, but here
we are talking about the inner ip header. Inner IP checksum will be
updated on the next end point after decryption. This would expect that
the next end point must have checksum offload capability. What if we are
capturing the encrypted packets on wireshark or say send it to some
other machine which does not run DPDK and do not know about checksum
offload, then wireshark/other machine will not be able to get the
correct the checksum and will show error.
>>
>> Because we assume that we always forward the packet in both paths, we
>> decrement the ttl in both inbound and outbound.
>> You seem to only increment (recalculate) the checksum of the inner IP
>> header in the outbound path but not the inbound path.
[Akhil]Correct I missed out the inbound path.
>>
>> Also, in the inbound path you have to consider a possible ECN value update.
[Akhil]If I take care of the ECN then it would mean I need to calculate
the checksum completely, incremental checksum wont give correct results.
This would surely impact performance. Any suggestion on how should we
take care of ECN update. Should I recalculate the checksum and send the
patch for ECN update? Or do we have a better solution.
>
> Any further comments here, Akhil?
>
> Thanks,
> Pablo
>
[Akhil] Sorry I missed out the previous reply from Sergio.
Thanks,
Akhil
>>
>> Sergio
>>
>>
>> On 26/09/2016 17:32, akhil.goyal@nxp.com wrote:
>>> From: Akhil Goyal <akhil.goyal@nxp.com>
>>>
>>> In IPsec-secgw application when TTL is decremented in IP header
>>> before forwarding the packet, checksum needs to be updated.
>>>
>>> In this patch an incremental checksum is added.
>>> Other applications(like l3fwd) are also doing so.
>>>
>>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>>> ---
>>> examples/ipsec-secgw/ipip.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/examples/ipsec-secgw/ipip.h b/examples/ipsec-secgw/ipip.h
>>> index ff1dccd..ef059a9 100644
>>> --- a/examples/ipsec-secgw/ipip.h
>>> +++ b/examples/ipsec-secgw/ipip.h
>>> @@ -56,6 +56,7 @@ ipip_outbound(struct rte_mbuf *m, uint32_t offset,
>> uint32_t is_ipv6,
>>> if (inip4->ip_v == IPVERSION) {
>>> /* XXX This should be done by the forwarding engine instead
>> */
>>> inip4->ip_ttl -= 1;
>>> + inip4->ip_sum += 1;
>>> ds_ecn = inip4->ip_tos;
>>> } else {
>>> inip6 = (struct ip6_hdr *)inip4;
>>
>>
>
>
^ permalink raw reply
* Re: [PATCH v2 4/8] app/testpmd: add option to enable lro
From: De Lara Guarch, Pablo @ 2016-10-05 6:26 UTC (permalink / raw)
To: Olivier Matz, dev@dpdk.org
In-Reply-To: <1473407734-11253-5-git-send-email-olivier.matz@6wind.com>
Hi Olivier,
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Friday, September 09, 2016 12:56 AM
> To: dev@dpdk.org; De Lara Guarch, Pablo
> Subject: [PATCH v2 4/8] app/testpmd: add option to enable lro
>
> Introduce a new argument '--enable-lro' to ask testpmd to enable the LRO
> feature on enabled ports, like it's done for '--enable-rx-cksum' for
> instance.
>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> app/test-pmd/parameters.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 8792c2c..612ad37 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -150,6 +150,7 @@ usage(char* progname)
> "If the drop-queue doesn't exist, the packet is dropped. "
> "By default drop-queue=127.\n");
> printf(" --crc-strip: enable CRC stripping by hardware.\n");
> + printf(" --enable-lro: enable large receive offload.\n");
> printf(" --enable-rx-cksum: enable rx hardware checksum
> offload.\n");
> printf(" --disable-hw-vlan: disable hardware vlan.\n");
> printf(" --disable-hw-vlan-filter: disable hardware vlan filter.\n");
> @@ -525,6 +526,7 @@ launch_args_parse(int argc, char** argv)
> { "pkt-filter-size", 1, 0, 0 },
> { "pkt-filter-drop-queue", 1, 0, 0 },
> { "crc-strip", 0, 0, 0 },
> + { "enable-lro", 0, 0, 0 },
> { "enable-rx-cksum", 0, 0, 0 },
> { "enable-scatter", 0, 0, 0 },
> { "disable-hw-vlan", 0, 0, 0 },
> @@ -765,6 +767,8 @@ launch_args_parse(int argc, char** argv)
> }
> if (!strcmp(lgopts[opt_idx].name, "crc-strip"))
> rx_mode.hw_strip_crc = 1;
> + if (!strcmp(lgopts[opt_idx].name, "enable-lro"))
> + rx_mode.enable_lro = 1;
> if (!strcmp(lgopts[opt_idx].name, "enable-scatter"))
> rx_mode.enable_scatter = 1;
> if (!strcmp(lgopts[opt_idx].name, "enable-rx-cksum"))
> --
> 2.8.1
Could you add this new parameter in the testpmd documentation?
Thanks,
Pablo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox