* [PATCH v2 1/3] mem: fix hugepage mapping error messages
From: Jean Tourrilhes @ 2016-10-04 17:17 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Sergio Gonzalez Monroy, dev
In-Reply-To: <2392264.CCbfKQ6uGK@xps13>
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(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 3fb2188..5df9f6a 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -238,7 +238,8 @@ rte_eal_config_attach(void)
mem_config = (struct rte_mem_config *) mmap(NULL, sizeof(*mem_config),
PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
if (mem_config == MAP_FAILED)
- rte_panic("Cannot mmap memory for rte_config\n");
+ rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
+ errno, strerror(errno));
rte_config.mem_config = mem_config;
}
@@ -263,9 +264,16 @@ rte_eal_config_reattach(void)
mem_config = (struct rte_mem_config *) mmap(rte_mem_cfg_addr,
sizeof(*mem_config), PROT_READ | PROT_WRITE, MAP_SHARED,
mem_cfg_fd, 0);
+ if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr) {
+ if (mem_config != MAP_FAILED)
+ /* errno is stale, don't use */
+ rte_panic("Cannot mmap memory for rte_config at [%p], got [%p] - please use '--base-virtaddr' option\n",
+ rte_mem_cfg_addr, mem_config);
+ else
+ rte_panic("Cannot mmap memory for rte_config! error %i (%s)\n",
+ errno, strerror(errno));
+ }
close(mem_cfg_fd);
- if (mem_config == MAP_FAILED || mem_config != rte_mem_cfg_addr)
- rte_panic("Cannot mmap memory for rte_config\n");
rte_config.mem_config = mem_config;
}
diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c b/lib/librte_eal/linuxapp/eal/eal_memory.c
index 41e0a92..b036ffc 100644
--- a/lib/librte_eal/linuxapp/eal/eal_memory.c
+++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
@@ -1615,10 +1615,18 @@ rte_eal_hugepage_attach(void)
PROT_READ, MAP_PRIVATE, fd_zero, 0);
if (base_addr == MAP_FAILED ||
base_addr != mcfg->memseg[s].addr) {
- RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
- "in /dev/zero to requested address [%p]: '%s'\n",
- (unsigned long long)mcfg->memseg[s].len,
- mcfg->memseg[s].addr, strerror(errno));
+ if (base_addr != MAP_FAILED)
+ /* errno is stale, don't use */
+ RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
+ "in /dev/zero at [%p], got [%p] - "
+ "please use '--base-virtaddr' option\n",
+ (unsigned long long)mcfg->memseg[s].len,
+ mcfg->memseg[s].addr, base_addr);
+ else
+ RTE_LOG(ERR, EAL, "Could not mmap %llu bytes "
+ "in /dev/zero at [%p]: '%s'\n",
+ (unsigned long long)mcfg->memseg[s].len,
+ mcfg->memseg[s].addr, strerror(errno));
if (aslr_enabled() > 0) {
RTE_LOG(ERR, EAL, "It is recommended to "
"disable ASLR in the kernel "
^ permalink raw reply related
* Re: Getting corrupted ESP packet
From: Trahe, Fiona @ 2016-10-04 18:15 UTC (permalink / raw)
To: Pankaj Joshi, qat-linux, dev@dpdk.org; +Cc: Trahe, Fiona, Griffin, John
In-Reply-To: <CAOYs48EWTcFhv8zCvxQkQJFw9-4RRf9dDm3vmZ_Mdt7dFfWWzg@mail.gmail.com>
Hi Pankaj,
I can't think of any way the QAT PMD could return a larger packet than it's been sent, can you provide some more details of your use-case please, e.g. which cipher algorithm, which auth algorithm are you using?
Are you using out-of-place or in-place? i.e. are the m_src and m_dst mbuf pointers in the rte_crypto_sym_op the same or different?
Can you try doing the same operation using the AESNI_MB PMD?
Regards,
Fiona
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pankaj Joshi
> Sent: Tuesday, October 4, 2016 5:04 PM
> To: qat-linux <qat-linux@intel.com>; dev@dpdk.org
> Subject: [dpdk-dev] Getting corrupted ESP packet
>
> Hello All,
>
> I am using QAT library for data encryption ( for coletocreek card).
> I am sending 98 byte ICMP data to the hardware, at successful time it is
> returning 166 byte of data as ESP packet.
> But sometimes it is returning through callback function 180 byte of data ,
> which is corrupted one .
> Can anyone tell, how I can resolve this issue and why it is happening when
> I am sending same data to the hardware.
>
> Regards,
> Pankaj Joshi
^ permalink raw reply
* Re: [PATCH v2 1/3] mem: fix hugepage mapping error messages
From: Sergio Gonzalez Monroy @ 2016-10-04 19:07 UTC (permalink / raw)
To: jean.tourrilhes, Thomas Monjalon; +Cc: dev
In-Reply-To: <20161004171703.GA3053@labs.hpe.com>
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>
^ permalink raw reply
* Re: [PATCH v2]:rte_timer:timer lag issue correction
From: Karmarkar Suyash @ 2016-10-04 19:31 UTC (permalink / raw)
To: dev@dpdk.org, thomas.monjalon@6wind.com, rsanford@akamai.com,
reshma.pattan@intel.com
In-Reply-To: <BN3PR03MB14312FD1C71886E4A3AC4A25B3CE0@BN3PR03MB1431.namprd03.prod.outlook.com>
Hello Robert/Thomas,
Can you please review the changes in V2 of the Patch and suggest next steps? Thanks
Regards
Suyash Karmarkar
-----Original Message-----
From: Karmarkar Suyash
Sent: Thursday, September 29, 2016 10:27 AM
To: dev@dpdk.org; thomas.monjalon@6wind.com; rsanford@akamai.com; reshma.pattan@intel.com
Subject: RE: [PATCH v2]:rte_timer:timer lag issue correction
Hello,
Can you please review the changes and suggest next steps? Thanks
Regards
Suyash Karmarkar
-----Original Message-----
From: Karmarkar Suyash
Sent: Wednesday, September 21, 2016 4:54 PM
To: dev@dpdk.org; thomas.monjalon@6wind.com; rsanford@akamai.com; reshma.pattan@intel.com
Cc: Karmarkar Suyash <skarmarkar@sonusnet.com>
Subject: [PATCH v2]:rte_timer:timer lag issue correction
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>
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index 43da836..18782fa 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -613,7 +613,7 @@ void rte_timer_manage(void)
status.owner = (int16_t)lcore_id;
rte_wmb();
tim->status.u32 = status.u32;
- __rte_timer_reset(tim, cur_time + tim->period,
+ __rte_timer_reset(tim, tim->expire + tim->period,
tim->period, lcore_id, tim->f, tim->arg, 1);
rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
}
--
2.9.3.windows.1
^ permalink raw reply
* Problem Generating Traffic
From: Mauricio Vasquez @ 2016-10-04 20:51 UTC (permalink / raw)
To: dev
Hello,
While performing a series of throughput testing I found a limitation
while generating traffic.
I have a server equipped with two 10G NICs that are connected using a
Ethernet wire. MoonGen is used to generate traffic on these interfaces,
it shows a performance of 22.52 Mpps. Theoretically it should be 29.76
Mpps (14.88x2) while using 64 bytes long packets.
I tried to implemente a silly traffic generator by myself [1], It uses 4
cores, 2 for sending and 2 for receiving, however in this case the
throughput is still 22.52 Mpps.
I tried many different things, change the number of descriptors in the
NIC, use separated mempools, run two separated DPDK processes, change
the burst size, change the mempool parameters, however the maximum
throughput I can get is always 22.52 Mpps.
My question is, what could be the bottleneck in this case?, is the PCI-e
bus an option?
Any other cue?
Just in case, the server's characteristics:
- Intel Xeon E5-2690 v2 @ 3 GHz (ten physical cores plus hyperthreading)
- 64 GB RAM, Ubuntu 15.04, equipped with two 10G Intel 82599ES NICs.
- DPDK 16.07
Thanks in Advance,
Mauricio V.
[1] http://pastebin.com/k565gW6x
^ permalink raw reply
* Re: [PATCH v2]:rte_timer:timer lag issue correction
From: Sanford, Robert @ 2016-10-04 20:51 UTC (permalink / raw)
To: Karmarkar Suyash, dev@dpdk.org, thomas.monjalon@6wind.com,
reshma.pattan@intel.com
In-Reply-To: <BN3PR03MB14318E1B7740E6ADDC2EF85DB3C50@BN3PR03MB1431.namprd03.prod.outlook.com>
Sorry, just saw this. I will take a look and get back shortly.
--
Regards,
Robert
On 10/4/16, 3:31 PM, "Karmarkar Suyash" <skarmarkar@sonusnet.com> wrote:
Hello Robert/Thomas,
Can you please review the changes in V2 of the Patch and suggest next steps? Thanks
Regards
Suyash Karmarkar
-----Original Message-----
From: Karmarkar Suyash
Sent: Thursday, September 29, 2016 10:27 AM
To: dev@dpdk.org; thomas.monjalon@6wind.com; rsanford@akamai.com; reshma.pattan@intel.com
Subject: RE: [PATCH v2]:rte_timer:timer lag issue correction
Hello,
Can you please review the changes and suggest next steps? Thanks
Regards
Suyash Karmarkar
-----Original Message-----
From: Karmarkar Suyash
Sent: Wednesday, September 21, 2016 4:54 PM
To: dev@dpdk.org; thomas.monjalon@6wind.com; rsanford@akamai.com; reshma.pattan@intel.com
Cc: Karmarkar Suyash <skarmarkar@sonusnet.com>
Subject: [PATCH v2]:rte_timer:timer lag issue correction
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>
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index 43da836..18782fa 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -613,7 +613,7 @@ void rte_timer_manage(void)
status.owner = (int16_t)lcore_id;
rte_wmb();
tim->status.u32 = status.u32;
- __rte_timer_reset(tim, cur_time + tim->period,
+ __rte_timer_reset(tim, tim->expire + tim->period,
tim->period, lcore_id, tim->f, tim->arg, 1);
rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
}
--
2.9.3.windows.1
^ permalink raw reply
* Re: [PATCH v2] examples: fix ip_pipeline to load PMD driver correctly
From: Dumitrescu, Cristian @ 2016-10-04 21:16 UTC (permalink / raw)
To: Gowrishankar, dev@dpdk.org
Cc: Chao Zhu, Thomas Monjalon, Christian Ehrhardt, Pradeep
In-Reply-To: <cover.1475577468.git.gowrishankar.m@linux.vnet.ibm.com>
> -----Original Message-----
> From: Gowrishankar [mailto:gowrishankar.m@linux.vnet.ibm.com]
> Sent: Tuesday, October 4, 2016 1:43 PM
> To: dev@dpdk.org
> Cc: Chao Zhu <chaozhu@linux.vnet.ibm.com>; Thomas Monjalon
> <thomas.monjalon@6wind.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; Christian Ehrhardt
> <christian.ehrhardt@canonical.com>; Pradeep <pradeep@us.ibm.com>;
> Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> Subject: [PATCH v2] examples: fix ip_pipeline to load PMD driver correctly
>
> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
>
> v2: minor correction in patch to avoid space between -d option and driver
> path
>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
^ permalink raw reply
* Re: [PATCH v2]:rte_timer:timer lag issue correction
From: Sanford, Robert @ 2016-10-04 21:39 UTC (permalink / raw)
To: Karmarkar Suyash, dev@dpdk.org, thomas.monjalon@6wind.com,
reshma.pattan@intel.com
In-Reply-To: <BN3PR03MB14312FD1C71886E4A3AC4A25B3CE0@BN3PR03MB1431.namprd03.prod.outlook.com>
Yes, this change makes sense. I ran timer tests and they passed.
Acked-by: Robert Sanford <rsanford@akamai.com>
Thanks,
Robert
On 9/29/16, 10:27 AM, "Karmarkar Suyash" <skarmarkar@sonusnet.com> wrote:
Hello,
Can you please review the changes and suggest next steps? Thanks
Regards
Suyash Karmarkar
-----Original Message-----
From: Karmarkar Suyash
Sent: Wednesday, September 21, 2016 4:54 PM
To: dev@dpdk.org; thomas.monjalon@6wind.com; rsanford@akamai.com; reshma.pattan@intel.com
Cc: Karmarkar Suyash <skarmarkar@sonusnet.com>
Subject: [PATCH v2]:rte_timer:timer lag issue correction
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>
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index 43da836..18782fa 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -613,7 +613,7 @@ void rte_timer_manage(void)
status.owner = (int16_t)lcore_id;
rte_wmb();
tim->status.u32 = status.u32;
- __rte_timer_reset(tim, cur_time + tim->period,
+ __rte_timer_reset(tim, tim->expire + tim->period,
tim->period, lcore_id, tim->f, tim->arg, 1);
rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
}
--
2.9.3.windows.1
^ permalink raw reply related
* Re: [RFC] libeventdev: event driven programming model framework for DPDK
From: Vangati, Narender @ 2016-10-04 21:49 UTC (permalink / raw)
To: dev@dpdk.org
Hi 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: [PATCH v2]:rte_timer:timer lag issue correction
From: Karmarkar Suyash @ 2016-10-04 22:36 UTC (permalink / raw)
To: Sanford, Robert, dev@dpdk.org, thomas.monjalon@6wind.com,
reshma.pattan@intel.com
In-Reply-To: <D09D8D35-BE32-48DD-B070-030C590744F4@akamai.com>
Thanks !! So as next steps I will push the patch .
-----Original Message-----
From: Sanford, Robert [mailto:rsanford@akamai.com]
Sent: Tuesday, October 4, 2016 5:40 PM
To: Karmarkar Suyash <skarmarkar@sonusnet.com>; dev@dpdk.org; thomas.monjalon@6wind.com; reshma.pattan@intel.com
Subject: Re: [PATCH v2]:rte_timer:timer lag issue correction
Yes, this change makes sense. I ran timer tests and they passed.
Acked-by: Robert Sanford <rsanford@akamai.com>
Thanks,
Robert
On 9/29/16, 10:27 AM, "Karmarkar Suyash" <skarmarkar@sonusnet.com> wrote:
Hello,
Can you please review the changes and suggest next steps? Thanks
Regards
Suyash Karmarkar
-----Original Message-----
From: Karmarkar Suyash
Sent: Wednesday, September 21, 2016 4:54 PM
To: dev@dpdk.org; thomas.monjalon@6wind.com; rsanford@akamai.com; reshma.pattan@intel.com
Cc: Karmarkar Suyash <skarmarkar@sonusnet.com>
Subject: [PATCH v2]:rte_timer:timer lag issue correction
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>
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
lib/librte_timer/rte_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index 43da836..18782fa 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -613,7 +613,7 @@ void rte_timer_manage(void)
status.owner = (int16_t)lcore_id;
rte_wmb();
tim->status.u32 = status.u32;
- __rte_timer_reset(tim, cur_time + tim->period,
+ __rte_timer_reset(tim, tim->expire + tim->period,
tim->period, lcore_id, tim->f, tim->arg, 1);
rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
}
--
2.9.3.windows.1
^ permalink raw reply
* Re: [PATCH v7] net/virtio: add set_mtu in virtio
From: Dey, Souvik @ 2016-10-04 23:18 UTC (permalink / raw)
To: mark.b.kavanagh@intel.com, yuanhan.liu@linux.intel.com,
stephen@networkplumber.org
Cc: dev@dpdk.org
In-Reply-To: <20160929203130.58712-1-sodey@sonusnet.com>
Hi All,
Is there any further comments or modifications required for this patch, or what next steps do you guys suggest here ?
--
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 acheive
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) */
+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;
+ 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);
+ 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 related
* [PATCH v5 1/4] hash: reorder hash structure
From: Pablo de Lara @ 2016-10-04 23:25 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Pablo de Lara
In-Reply-To: <1475623515-47587-1-git-send-email-pablo.de.lara.guarch@intel.com>
In order to optimize lookup performance, hash structure
is reordered, so all fields used for lookup will be
in the first cache line.
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Sameh Gobriel <sameh.gobriel@intel.com>
---
lib/librte_hash/rte_cuckoo_hash.h | 36 +++++++++++++++++++++---------------
1 file changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h
index e290dab..27a47e5 100644
--- a/lib/librte_hash/rte_cuckoo_hash.h
+++ b/lib/librte_hash/rte_cuckoo_hash.h
@@ -185,30 +185,36 @@ struct rte_hash {
char name[RTE_HASH_NAMESIZE]; /**< Name of the hash. */
uint32_t entries; /**< Total table entries. */
uint32_t num_buckets; /**< Number of buckets in table. */
- uint32_t key_len; /**< Length of hash key. */
+
+ struct rte_ring *free_slots;
+ /**< Ring that stores all indexes of the free slots in the key table */
+ uint8_t hw_trans_mem_support;
+ /**< Hardware transactional memory support */
+ struct lcore_cache *local_free_slots;
+ /**< Local cache per lcore, storing some indexes of the free slots */
+ enum add_key_case add_key; /**< Multi-writer hash add behavior */
+
+ rte_spinlock_t *multiwriter_lock; /**< Multi-writer spinlock for w/o TM */
+
+ /* Fields used in lookup */
+
+ uint32_t key_len __rte_cache_aligned;
+ /**< Length of hash key. */
rte_hash_function hash_func; /**< Function used to calculate hash. */
uint32_t hash_func_init_val; /**< Init value used by hash_func. */
rte_hash_cmp_eq_t rte_hash_custom_cmp_eq;
/**< Custom function used to compare keys. */
enum cmp_jump_table_case cmp_jump_table_idx;
/**< Indicates which compare function to use. */
- uint32_t bucket_bitmask; /**< Bitmask for getting bucket index
- from hash signature. */
+ uint32_t bucket_bitmask;
+ /**< Bitmask for getting bucket index from hash signature. */
uint32_t key_entry_size; /**< Size of each key entry. */
- struct rte_ring *free_slots; /**< Ring that stores all indexes
- of the free slots in the key table */
void *key_store; /**< Table storing all keys and data */
- struct rte_hash_bucket *buckets; /**< Table with buckets storing all the
- hash values and key indexes
- to the key table*/
- uint8_t hw_trans_mem_support; /**< Hardware transactional
- memory support */
- struct lcore_cache *local_free_slots;
- /**< Local cache per lcore, storing some indexes of the free slots */
- enum add_key_case add_key; /**< Multi-writer hash add behavior */
-
- rte_spinlock_t *multiwriter_lock; /**< Multi-writer spinlock for w/o TM */
+ struct rte_hash_bucket *buckets;
+ /**< Table with buckets storing all the hash values and key indexes
+ * to the key table.
+ */
} __rte_cache_aligned;
struct queue_node {
--
2.7.4
^ permalink raw reply related
* [PATCH v5 0/4] Cuckoo hash enhancements
From: Pablo de Lara @ 2016-10-04 23:25 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Pablo de Lara
In-Reply-To: <1475221136-213246-1-git-send-email-pablo.de.lara.guarch@intel.com>
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.
First patch reorganizes the order of the hash structure.
The structure takes more than one 64-byte cache line, but not all
the fields are used in the lookup operation (the most common operation).
Therefore, all these fields have been moved to the first part of the structure,
so they all fit in one cache line, improving slightly the performance in some
scenarios.
Second patch modifies the order of the bucket structure.
Currently, the buckets store all the signatures together (current and alternative).
In order to be able to perform a vectorized signature comparison,
all current signatures have to be together, so the order of the bucket has been changed,
having separated all the current signatures from the alternative signatures.
Third patch introduces x86 vectorized intrinsics.
When performing a lookup bulk operation, all current signatures in a bucket
are compared against the signature of the key being looked up.
Now that they all are together, a vectorized comparison can be performed,
which takes less instructions to be carried out.
In case of having a machine with AVX2, number of entries per bucket are
increased from 4 to 8, as AVX2 allows comparing two 256-bit values, with 8x32-bit integers,
which are the 8 signatures on the bucket.
Fourth (and last) patch modifies the current pipeline of the lookup bulk function.
The new pipeline is based on a loop-and-jump model. The two key improvements are:
- Better prefetching: in this case, first 4 keys to be looked up are prefetched,
and after that, the rest of the keys are prefetched at the time the calculation
of the signatures are being performed. This gives more time for the CPU to
prefetch the data requesting before actually need it, which result in less
cache misses and therefore, higher throughput.
- Lower performance penalty when using fallback: the lookup bulk algorithm
assumes that most times there will not be a collision in a bucket, but it might
happen that two or more signatures are equal, which means that more than one
key comparison might be necessary. In that case, only the key of the first hit is prefetched,
like in the current implementation. The difference now is that if this comparison
results in a miss, the information of the other keys to be compared has been stored,
unlike the current implementation, which needs to perform an entire simple lookup again.
Changes in v5:
- Rebased against current HEAD
- Fix checkpatch warnings
Changes in v4:
- Reordered hash structure, so alt signature is at the start
of the next cache line, and explain in the commit message
why it has been moved
- Reordered hash structure, so name field is on top of the structure,
leaving all the fields used in lookup in the next cache line
(instead of the first cache line)
Changes in v3:
- Corrected the cover letter (wrong number of patches)
Changes in v2:
- Increased entries per bucket from 4 to 8 for all cases,
so it is not architecture dependent any longer.
- Replaced compile-time signature comparison function election
with run-time election, so best optimization available
will be used from a single binary.
- Reordered the hash structure, so all the fields used by lookup
are in the same cache line (first).
Byron Marohn (3):
hash: reorganize bucket structure
hash: add vectorized comparison
hash: modify lookup bulk pipeline
Pablo de Lara (1):
hash: reorder hash structure
lib/librte_hash/rte_cuckoo_hash.c | 455 ++++++++++++++--------------------
lib/librte_hash/rte_cuckoo_hash.h | 68 ++---
lib/librte_hash/rte_cuckoo_hash_x86.h | 20 +-
3 files changed, 235 insertions(+), 308 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v5 2/4] hash: reorganize bucket structure
From: Pablo de Lara @ 2016-10-04 23:25 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Byron Marohn, Saikrishna Edupuganti
In-Reply-To: <1475623515-47587-1-git-send-email-pablo.de.lara.guarch@intel.com>
From: Byron Marohn <byron.marohn@intel.com>
Move current signatures of all entries together in the bucket
and same with all alternative signatures, instead of having
current and alternative signatures together per entry in the bucket.
This will be benefitial in the next commits, where a vectorized
comparison will be performed, achieving better performance.
The alternative signatures have been moved away from
the current signatures, to make the key indices be consecutive
to the current signatures, as these two fields are used by lookup,
so they are in the same cache line.
Signed-off-by: Byron Marohn <byron.marohn@intel.com>
Signed-off-by: Saikrishna Edupuganti <saikrishna.edupuganti@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Sameh Gobriel <sameh.gobriel@intel.com>
---
lib/librte_hash/rte_cuckoo_hash.c | 43 ++++++++++++++++++-----------------
lib/librte_hash/rte_cuckoo_hash.h | 17 ++++----------
lib/librte_hash/rte_cuckoo_hash_x86.h | 20 ++++++++--------
3 files changed, 37 insertions(+), 43 deletions(-)
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 4de4422..a7ee2b9 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -421,7 +421,7 @@ make_space_bucket(const struct rte_hash *h, struct rte_hash_bucket *bkt)
*/
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
/* Search for space in alternative locations */
- next_bucket_idx = bkt->signatures[i].alt & h->bucket_bitmask;
+ next_bucket_idx = bkt->sig_alt[i] & h->bucket_bitmask;
next_bkt[i] = &h->buckets[next_bucket_idx];
for (j = 0; j < RTE_HASH_BUCKET_ENTRIES; j++) {
if (next_bkt[i]->key_idx[j] == EMPTY_SLOT)
@@ -434,8 +434,8 @@ make_space_bucket(const struct rte_hash *h, struct rte_hash_bucket *bkt)
/* Alternative location has spare room (end of recursive function) */
if (i != RTE_HASH_BUCKET_ENTRIES) {
- next_bkt[i]->signatures[j].alt = bkt->signatures[i].current;
- next_bkt[i]->signatures[j].current = bkt->signatures[i].alt;
+ next_bkt[i]->sig_alt[j] = bkt->sig_current[i];
+ next_bkt[i]->sig_current[j] = bkt->sig_alt[i];
next_bkt[i]->key_idx[j] = bkt->key_idx[i];
return i;
}
@@ -461,8 +461,8 @@ make_space_bucket(const struct rte_hash *h, struct rte_hash_bucket *bkt)
*/
bkt->flag[i] = 0;
if (ret >= 0) {
- next_bkt[i]->signatures[ret].alt = bkt->signatures[i].current;
- next_bkt[i]->signatures[ret].current = bkt->signatures[i].alt;
+ next_bkt[i]->sig_alt[ret] = bkt->sig_current[i];
+ next_bkt[i]->sig_current[ret] = bkt->sig_alt[i];
next_bkt[i]->key_idx[ret] = bkt->key_idx[i];
return i;
} else
@@ -544,8 +544,8 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
/* Check if key is already inserted in primary location */
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- if (prim_bkt->signatures[i].current == sig &&
- prim_bkt->signatures[i].alt == alt_hash) {
+ if (prim_bkt->sig_current[i] == sig &&
+ prim_bkt->sig_alt[i] == alt_hash) {
k = (struct rte_hash_key *) ((char *)keys +
prim_bkt->key_idx[i] * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
@@ -564,8 +564,8 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
/* Check if key is already inserted in secondary location */
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- if (sec_bkt->signatures[i].alt == sig &&
- sec_bkt->signatures[i].current == alt_hash) {
+ if (sec_bkt->sig_alt[i] == sig &&
+ sec_bkt->sig_current[i] == alt_hash) {
k = (struct rte_hash_key *) ((char *)keys +
sec_bkt->key_idx[i] * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
@@ -611,8 +611,8 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
/* Check if slot is available */
if (likely(prim_bkt->key_idx[i] == EMPTY_SLOT)) {
- prim_bkt->signatures[i].current = sig;
- prim_bkt->signatures[i].alt = alt_hash;
+ prim_bkt->sig_current[i] = sig;
+ prim_bkt->sig_alt[i] = alt_hash;
prim_bkt->key_idx[i] = new_idx;
break;
}
@@ -632,8 +632,8 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
*/
ret = make_space_bucket(h, prim_bkt);
if (ret >= 0) {
- prim_bkt->signatures[ret].current = sig;
- prim_bkt->signatures[ret].alt = alt_hash;
+ prim_bkt->sig_current[ret] = sig;
+ prim_bkt->sig_alt[ret] = alt_hash;
prim_bkt->key_idx[ret] = new_idx;
if (h->add_key == ADD_KEY_MULTIWRITER)
rte_spinlock_unlock(h->multiwriter_lock);
@@ -707,7 +707,7 @@ __rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key,
/* Check if key is in primary location */
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- if (bkt->signatures[i].current == sig &&
+ if (bkt->sig_current[i] == sig &&
bkt->key_idx[i] != EMPTY_SLOT) {
k = (struct rte_hash_key *) ((char *)keys +
bkt->key_idx[i] * h->key_entry_size);
@@ -730,8 +730,8 @@ __rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key,
/* Check if key is in secondary location */
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- if (bkt->signatures[i].current == alt_hash &&
- bkt->signatures[i].alt == sig) {
+ if (bkt->sig_current[i] == alt_hash &&
+ bkt->sig_alt[i] == sig) {
k = (struct rte_hash_key *) ((char *)keys +
bkt->key_idx[i] * h->key_entry_size);
if (rte_hash_cmp_eq(key, k->key, h) == 0) {
@@ -785,7 +785,8 @@ remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt, unsigned i)
unsigned lcore_id, n_slots;
struct lcore_cache *cached_free_slots;
- bkt->signatures[i].sig = NULL_SIGNATURE;
+ bkt->sig_current[i] = NULL_SIGNATURE;
+ bkt->sig_alt[i] = NULL_SIGNATURE;
if (h->hw_trans_mem_support) {
lcore_id = rte_lcore_id();
cached_free_slots = &h->local_free_slots[lcore_id];
@@ -823,7 +824,7 @@ __rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key,
/* Check if key is in primary location */
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- if (bkt->signatures[i].current == sig &&
+ if (bkt->sig_current[i] == sig &&
bkt->key_idx[i] != EMPTY_SLOT) {
k = (struct rte_hash_key *) ((char *)keys +
bkt->key_idx[i] * h->key_entry_size);
@@ -848,7 +849,7 @@ __rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key,
/* Check if key is in secondary location */
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- if (bkt->signatures[i].current == alt_hash &&
+ if (bkt->sig_current[i] == alt_hash &&
bkt->key_idx[i] != EMPTY_SLOT) {
k = (struct rte_hash_key *) ((char *)keys +
bkt->key_idx[i] * h->key_entry_size);
@@ -957,8 +958,8 @@ lookup_stage2(unsigned idx, hash_sig_t prim_hash, hash_sig_t sec_hash,
prim_hash_matches = 1 << RTE_HASH_BUCKET_ENTRIES;
sec_hash_matches = 1 << RTE_HASH_BUCKET_ENTRIES;
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- prim_hash_matches |= ((prim_hash == prim_bkt->signatures[i].current) << i);
- sec_hash_matches |= ((sec_hash == sec_bkt->signatures[i].current) << i);
+ prim_hash_matches |= ((prim_hash == prim_bkt->sig_current[i]) << i);
+ sec_hash_matches |= ((sec_hash == sec_bkt->sig_current[i]) << i);
}
key_idx = prim_bkt->key_idx[__builtin_ctzl(prim_hash_matches)];
diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h
index 27a47e5..6549731 100644
--- a/lib/librte_hash/rte_cuckoo_hash.h
+++ b/lib/librte_hash/rte_cuckoo_hash.h
@@ -151,17 +151,6 @@ struct lcore_cache {
void *objs[LCORE_CACHE_SIZE]; /**< Cache objects */
} __rte_cache_aligned;
-/* Structure storing both primary and secondary hashes */
-struct rte_hash_signatures {
- union {
- struct {
- hash_sig_t current;
- hash_sig_t alt;
- };
- uint64_t sig;
- };
-};
-
/* Structure that stores key-value pair */
struct rte_hash_key {
union {
@@ -174,9 +163,13 @@ struct rte_hash_key {
/** Bucket structure */
struct rte_hash_bucket {
- struct rte_hash_signatures signatures[RTE_HASH_BUCKET_ENTRIES];
+ hash_sig_t sig_current[RTE_HASH_BUCKET_ENTRIES];
+
/* Includes dummy key index that always contains index 0 */
uint32_t key_idx[RTE_HASH_BUCKET_ENTRIES + 1];
+
+ hash_sig_t sig_alt[RTE_HASH_BUCKET_ENTRIES];
+
uint8_t flag[RTE_HASH_BUCKET_ENTRIES];
} __rte_cache_aligned;
diff --git a/lib/librte_hash/rte_cuckoo_hash_x86.h b/lib/librte_hash/rte_cuckoo_hash_x86.h
index 7ffa56f..47aec6d 100644
--- a/lib/librte_hash/rte_cuckoo_hash_x86.h
+++ b/lib/librte_hash/rte_cuckoo_hash_x86.h
@@ -54,8 +54,8 @@ rte_hash_cuckoo_insert_mw_tm(struct rte_hash_bucket *prim_bkt,
for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
/* Check if slot is available */
if (likely(prim_bkt->key_idx[i] == EMPTY_SLOT)) {
- prim_bkt->signatures[i].current = sig;
- prim_bkt->signatures[i].alt = alt_hash;
+ prim_bkt->sig_current[i] = sig;
+ prim_bkt->sig_alt[i] = alt_hash;
prim_bkt->key_idx[i] = new_idx;
break;
}
@@ -101,7 +101,7 @@ rte_hash_cuckoo_move_insert_mw_tm(const struct rte_hash *h,
prev_slot = curr_node->prev_slot;
prev_alt_bkt_idx
- = prev_bkt->signatures[prev_slot].alt
+ = prev_bkt->sig_alt[prev_slot]
& h->bucket_bitmask;
if (unlikely(&h->buckets[prev_alt_bkt_idx]
@@ -113,10 +113,10 @@ rte_hash_cuckoo_move_insert_mw_tm(const struct rte_hash *h,
* Cuckoo insert to move elements back to its
* primary bucket if available
*/
- curr_bkt->signatures[curr_slot].alt =
- prev_bkt->signatures[prev_slot].current;
- curr_bkt->signatures[curr_slot].current =
- prev_bkt->signatures[prev_slot].alt;
+ curr_bkt->sig_alt[curr_slot] =
+ prev_bkt->sig_current[prev_slot];
+ curr_bkt->sig_current[curr_slot] =
+ prev_bkt->sig_alt[prev_slot];
curr_bkt->key_idx[curr_slot]
= prev_bkt->key_idx[prev_slot];
@@ -125,8 +125,8 @@ rte_hash_cuckoo_move_insert_mw_tm(const struct rte_hash *h,
curr_bkt = curr_node->bkt;
}
- curr_bkt->signatures[curr_slot].current = sig;
- curr_bkt->signatures[curr_slot].alt = alt_hash;
+ curr_bkt->sig_current[curr_slot] = sig;
+ curr_bkt->sig_alt[curr_slot] = alt_hash;
curr_bkt->key_idx[curr_slot] = new_idx;
rte_xend();
@@ -178,7 +178,7 @@ rte_hash_cuckoo_make_space_mw_tm(const struct rte_hash *h,
}
/* Enqueue new node and keep prev node info */
- alt_bkt = &(h->buckets[curr_bkt->signatures[i].alt
+ alt_bkt = &(h->buckets[curr_bkt->sig_alt[i]
& h->bucket_bitmask]);
head->bkt = alt_bkt;
head->prev = tail;
--
2.7.4
^ permalink raw reply related
* [PATCH v5 3/4] hash: add vectorized comparison
From: Pablo de Lara @ 2016-10-04 23:25 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Byron Marohn, Saikrishna Edupuganti,
Pablo de Lara
In-Reply-To: <1475623515-47587-1-git-send-email-pablo.de.lara.guarch@intel.com>
From: Byron Marohn <byron.marohn@intel.com>
In lookup bulk function, the signatures of all entries
are compared against the signature of the key that is being looked up.
Now that all the signatures are together, they can be compared
with vector instructions (SSE, AVX2), achieving higher lookup performance.
Also, entries per bucket are increased to 8 when using processors
with AVX2, as 256 bits can be compared at once, which is the size of
8x32-bit signatures.
Signed-off-by: Byron Marohn <byron.marohn@intel.com>
Signed-off-by: Saikrishna Edupuganti <saikrishna.edupuganti@intel.com>
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Sameh Gobriel <sameh.gobriel@intel.com>
---
lib/librte_hash/rte_cuckoo_hash.c | 76 +++++++++++++++++++++++++++++++++++----
lib/librte_hash/rte_cuckoo_hash.h | 12 ++++++-
2 files changed, 81 insertions(+), 7 deletions(-)
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index a7ee2b9..d762f36 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -284,6 +284,15 @@ rte_hash_create(const struct rte_hash_parameters *params)
h->free_slots = r;
h->hw_trans_mem_support = hw_trans_mem_support;
+#if defined(RTE_ARCH_X86)
+ if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
+ h->sig_cmp_fn = RTE_HASH_COMPARE_AVX2;
+ else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE2))
+ h->sig_cmp_fn = RTE_HASH_COMPARE_SSE;
+ else
+#endif
+ h->sig_cmp_fn = RTE_HASH_COMPARE_SCALAR;
+
/* Turn on multi-writer only with explicit flat from user and TM
* support.
*/
@@ -940,6 +949,62 @@ lookup_stage1(unsigned idx, hash_sig_t *prim_hash, hash_sig_t *sec_hash,
rte_prefetch0(*secondary_bkt);
}
+static inline void
+compare_signatures(unsigned int *prim_hash_matches,
+ unsigned int *sec_hash_matches,
+ const struct rte_hash_bucket *prim_bkt,
+ const struct rte_hash_bucket *sec_bkt,
+ hash_sig_t prim_hash, hash_sig_t sec_hash,
+ enum rte_hash_sig_compare_function sig_cmp_fn)
+{
+ unsigned int i;
+
+ switch (sig_cmp_fn) {
+#ifdef RTE_MACHINE_CPUFLAG_AVX2
+ case RTE_HASH_COMPARE_AVX2:
+ *prim_hash_matches |= _mm256_movemask_ps((__m256)_mm256_cmpeq_epi32(
+ _mm256_load_si256(
+ (__m256i const *)prim_bkt->sig_current),
+ _mm256_set1_epi32(prim_hash)));
+ *sec_hash_matches |= _mm256_movemask_ps((__m256)_mm256_cmpeq_epi32(
+ _mm256_load_si256(
+ (__m256i const *)sec_bkt->sig_current),
+ _mm256_set1_epi32(sec_hash)));
+ break;
+#endif
+#ifdef RTE_MACHINE_CPUFLAG_SSE2
+ case RTE_HASH_COMPARE_SSE:
+ /* Compare the first 4 signatures in the bucket */
+ *prim_hash_matches |= _mm_movemask_ps((__m128)_mm_cmpeq_epi16(
+ _mm_load_si128(
+ (__m128i const *)prim_bkt->sig_current),
+ _mm_set1_epi32(prim_hash)));
+ *prim_hash_matches |= (_mm_movemask_ps((__m128)_mm_cmpeq_epi16(
+ _mm_load_si128(
+ (__m128i const *)&prim_bkt->sig_current[4]),
+ _mm_set1_epi32(prim_hash)))) << 4;
+ /* Compare the first 4 signatures in the bucket */
+ *sec_hash_matches |= _mm_movemask_ps((__m128)_mm_cmpeq_epi16(
+ _mm_load_si128(
+ (__m128i const *)sec_bkt->sig_current),
+ _mm_set1_epi32(sec_hash)));
+ *sec_hash_matches |= (_mm_movemask_ps((__m128)_mm_cmpeq_epi16(
+ _mm_load_si128(
+ (__m128i const *)&sec_bkt->sig_current[4]),
+ _mm_set1_epi32(sec_hash)))) << 4;
+ break;
+#endif
+ default:
+ for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
+ *prim_hash_matches |=
+ ((prim_hash == prim_bkt->sig_current[i]) << i);
+ *sec_hash_matches |=
+ ((sec_hash == sec_bkt->sig_current[i]) << i);
+ }
+ }
+
+}
+
/*
* Lookup bulk stage 2: Search for match hashes in primary/secondary locations
* and prefetch first key slot
@@ -952,15 +1017,14 @@ lookup_stage2(unsigned idx, hash_sig_t prim_hash, hash_sig_t sec_hash,
uint64_t *extra_hits_mask, const void *keys,
const struct rte_hash *h)
{
- unsigned prim_hash_matches, sec_hash_matches, key_idx, i;
- unsigned total_hash_matches;
+ unsigned int prim_hash_matches, sec_hash_matches, key_idx;
+ unsigned int total_hash_matches;
prim_hash_matches = 1 << RTE_HASH_BUCKET_ENTRIES;
sec_hash_matches = 1 << RTE_HASH_BUCKET_ENTRIES;
- for (i = 0; i < RTE_HASH_BUCKET_ENTRIES; i++) {
- prim_hash_matches |= ((prim_hash == prim_bkt->sig_current[i]) << i);
- sec_hash_matches |= ((sec_hash == sec_bkt->sig_current[i]) << i);
- }
+
+ compare_signatures(&prim_hash_matches, &sec_hash_matches, prim_bkt,
+ sec_bkt, prim_hash, sec_hash, h->sig_cmp_fn);
key_idx = prim_bkt->key_idx[__builtin_ctzl(prim_hash_matches)];
if (key_idx == 0)
diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h
index 6549731..504661d 100644
--- a/lib/librte_hash/rte_cuckoo_hash.h
+++ b/lib/librte_hash/rte_cuckoo_hash.h
@@ -130,7 +130,7 @@ enum add_key_case {
};
/** Number of items per bucket. */
-#define RTE_HASH_BUCKET_ENTRIES 4
+#define RTE_HASH_BUCKET_ENTRIES 8
#define NULL_SIGNATURE 0
@@ -161,6 +161,14 @@ struct rte_hash_key {
char key[0];
} __attribute__((aligned(KEY_ALIGNMENT)));
+/* All different signature compare functions */
+enum rte_hash_sig_compare_function {
+ RTE_HASH_COMPARE_SCALAR = 0,
+ RTE_HASH_COMPARE_SSE,
+ RTE_HASH_COMPARE_AVX2,
+ RTE_HASH_COMPARE_NUM
+};
+
/** Bucket structure */
struct rte_hash_bucket {
hash_sig_t sig_current[RTE_HASH_BUCKET_ENTRIES];
@@ -199,6 +207,8 @@ struct rte_hash {
/**< Custom function used to compare keys. */
enum cmp_jump_table_case cmp_jump_table_idx;
/**< Indicates which compare function to use. */
+ enum rte_hash_sig_compare_function sig_cmp_fn;
+ /**< Indicates which signature compare function to use. */
uint32_t bucket_bitmask;
/**< Bitmask for getting bucket index from hash signature. */
uint32_t key_entry_size; /**< Size of each key entry. */
--
2.7.4
^ permalink raw reply related
* [PATCH v5 4/4] hash: modify lookup bulk pipeline
From: Pablo de Lara @ 2016-10-04 23:25 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, Byron Marohn, Saikrishna Edupuganti,
Pablo de Lara
In-Reply-To: <1475623515-47587-1-git-send-email-pablo.de.lara.guarch@intel.com>
From: Byron Marohn <byron.marohn@intel.com>
This patch replaces the pipelined rte_hash lookup mechanism with a
loop-and-jump model, which performs significantly better,
especially for smaller table sizes and smaller table occupancies.
Signed-off-by: Byron Marohn <byron.marohn@intel.com>
Signed-off-by: Saikrishna Edupuganti <saikrishna.edupuganti@intel.com>
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Sameh Gobriel <sameh.gobriel@intel.com>
---
lib/librte_hash/rte_cuckoo_hash.c | 378 ++++++++++++--------------------------
lib/librte_hash/rte_cuckoo_hash.h | 3 +-
2 files changed, 117 insertions(+), 264 deletions(-)
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index d762f36..3324b17 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -914,44 +914,8 @@ rte_hash_get_key_with_position(const struct rte_hash *h, const int32_t position,
return 0;
}
-/* Lookup bulk stage 0: Prefetch input key */
static inline void
-lookup_stage0(unsigned *idx, uint64_t *lookup_mask,
- const void * const *keys)
-{
- *idx = __builtin_ctzl(*lookup_mask);
- if (*lookup_mask == 0)
- *idx = 0;
-
- rte_prefetch0(keys[*idx]);
- *lookup_mask &= ~(1llu << *idx);
-}
-
-/*
- * Lookup bulk stage 1: Calculate primary/secondary hashes
- * and prefetch primary/secondary buckets
- */
-static inline void
-lookup_stage1(unsigned idx, hash_sig_t *prim_hash, hash_sig_t *sec_hash,
- const struct rte_hash_bucket **primary_bkt,
- const struct rte_hash_bucket **secondary_bkt,
- hash_sig_t *hash_vals, const void * const *keys,
- const struct rte_hash *h)
-{
- *prim_hash = rte_hash_hash(h, keys[idx]);
- hash_vals[idx] = *prim_hash;
- *sec_hash = rte_hash_secondary_hash(*prim_hash);
-
- *primary_bkt = &h->buckets[*prim_hash & h->bucket_bitmask];
- *secondary_bkt = &h->buckets[*sec_hash & h->bucket_bitmask];
-
- rte_prefetch0(*primary_bkt);
- rte_prefetch0(*secondary_bkt);
-}
-
-static inline void
-compare_signatures(unsigned int *prim_hash_matches,
- unsigned int *sec_hash_matches,
+compare_signatures(uint32_t *prim_hash_matches, uint32_t *sec_hash_matches,
const struct rte_hash_bucket *prim_bkt,
const struct rte_hash_bucket *sec_bkt,
hash_sig_t prim_hash, hash_sig_t sec_hash,
@@ -962,11 +926,11 @@ compare_signatures(unsigned int *prim_hash_matches,
switch (sig_cmp_fn) {
#ifdef RTE_MACHINE_CPUFLAG_AVX2
case RTE_HASH_COMPARE_AVX2:
- *prim_hash_matches |= _mm256_movemask_ps((__m256)_mm256_cmpeq_epi32(
+ *prim_hash_matches = _mm256_movemask_ps((__m256)_mm256_cmpeq_epi32(
_mm256_load_si256(
(__m256i const *)prim_bkt->sig_current),
_mm256_set1_epi32(prim_hash)));
- *sec_hash_matches |= _mm256_movemask_ps((__m256)_mm256_cmpeq_epi32(
+ *sec_hash_matches = _mm256_movemask_ps((__m256)_mm256_cmpeq_epi32(
_mm256_load_si256(
(__m256i const *)sec_bkt->sig_current),
_mm256_set1_epi32(sec_hash)));
@@ -975,7 +939,7 @@ compare_signatures(unsigned int *prim_hash_matches,
#ifdef RTE_MACHINE_CPUFLAG_SSE2
case RTE_HASH_COMPARE_SSE:
/* Compare the first 4 signatures in the bucket */
- *prim_hash_matches |= _mm_movemask_ps((__m128)_mm_cmpeq_epi16(
+ *prim_hash_matches = _mm_movemask_ps((__m128)_mm_cmpeq_epi16(
_mm_load_si128(
(__m128i const *)prim_bkt->sig_current),
_mm_set1_epi32(prim_hash)));
@@ -984,7 +948,7 @@ compare_signatures(unsigned int *prim_hash_matches,
(__m128i const *)&prim_bkt->sig_current[4]),
_mm_set1_epi32(prim_hash)))) << 4;
/* Compare the first 4 signatures in the bucket */
- *sec_hash_matches |= _mm_movemask_ps((__m128)_mm_cmpeq_epi16(
+ *sec_hash_matches = _mm_movemask_ps((__m128)_mm_cmpeq_epi16(
_mm_load_si128(
(__m128i const *)sec_bkt->sig_current),
_mm_set1_epi32(sec_hash)));
@@ -1005,244 +969,134 @@ compare_signatures(unsigned int *prim_hash_matches,
}
-/*
- * Lookup bulk stage 2: Search for match hashes in primary/secondary locations
- * and prefetch first key slot
- */
+#define PREFETCH_OFFSET 4
static inline void
-lookup_stage2(unsigned idx, hash_sig_t prim_hash, hash_sig_t sec_hash,
- const struct rte_hash_bucket *prim_bkt,
- const struct rte_hash_bucket *sec_bkt,
- const struct rte_hash_key **key_slot, int32_t *positions,
- uint64_t *extra_hits_mask, const void *keys,
- const struct rte_hash *h)
+__rte_hash_lookup_bulk(const struct rte_hash *h, const void **keys,
+ int32_t num_keys, int32_t *positions,
+ uint64_t *hit_mask, void *data[])
{
- unsigned int prim_hash_matches, sec_hash_matches, key_idx;
- unsigned int total_hash_matches;
+ uint64_t hits = 0;
+ int32_t i;
+ uint32_t prim_hash[RTE_HASH_LOOKUP_BULK_MAX];
+ uint32_t sec_hash[RTE_HASH_LOOKUP_BULK_MAX];
+ const struct rte_hash_bucket *primary_bkt[RTE_HASH_LOOKUP_BULK_MAX];
+ const struct rte_hash_bucket *secondary_bkt[RTE_HASH_LOOKUP_BULK_MAX];
+ uint32_t prim_hitmask[RTE_HASH_LOOKUP_BULK_MAX] = {0};
+ uint32_t sec_hitmask[RTE_HASH_LOOKUP_BULK_MAX] = {0};
+
+ /* Prefetch first keys */
+ for (i = 0; i < PREFETCH_OFFSET && i < num_keys; i++)
+ rte_prefetch0(keys[i]);
- prim_hash_matches = 1 << RTE_HASH_BUCKET_ENTRIES;
- sec_hash_matches = 1 << RTE_HASH_BUCKET_ENTRIES;
+ /*
+ * Prefetch rest of the keys, calculate primary and
+ * secondary bucket and prefetch them
+ */
+ for (i = 0; i < (num_keys - PREFETCH_OFFSET); i++) {
+ rte_prefetch0(keys[i + PREFETCH_OFFSET]);
- compare_signatures(&prim_hash_matches, &sec_hash_matches, prim_bkt,
- sec_bkt, prim_hash, sec_hash, h->sig_cmp_fn);
+ prim_hash[i] = rte_hash_hash(h, keys[i]);
+ sec_hash[i] = rte_hash_secondary_hash(prim_hash[i]);
- key_idx = prim_bkt->key_idx[__builtin_ctzl(prim_hash_matches)];
- if (key_idx == 0)
- key_idx = sec_bkt->key_idx[__builtin_ctzl(sec_hash_matches)];
+ primary_bkt[i] = &h->buckets[prim_hash[i] & h->bucket_bitmask];
+ secondary_bkt[i] = &h->buckets[sec_hash[i] & h->bucket_bitmask];
- total_hash_matches = (prim_hash_matches |
- (sec_hash_matches << (RTE_HASH_BUCKET_ENTRIES + 1)));
- *key_slot = (const struct rte_hash_key *) ((const char *)keys +
- key_idx * h->key_entry_size);
+ rte_prefetch0(primary_bkt[i]);
+ rte_prefetch0(secondary_bkt[i]);
+ }
- rte_prefetch0(*key_slot);
- /*
- * Return index where key is stored,
- * substracting the first dummy index
- */
- positions[idx] = (key_idx - 1);
+ /* Calculate and prefetch rest of the buckets */
+ for (; i < num_keys; i++) {
+ prim_hash[i] = rte_hash_hash(h, keys[i]);
+ sec_hash[i] = rte_hash_secondary_hash(prim_hash[i]);
- *extra_hits_mask |= (uint64_t)(__builtin_popcount(total_hash_matches) > 3) << idx;
+ primary_bkt[i] = &h->buckets[prim_hash[i] & h->bucket_bitmask];
+ secondary_bkt[i] = &h->buckets[sec_hash[i] & h->bucket_bitmask];
-}
+ rte_prefetch0(primary_bkt[i]);
+ rte_prefetch0(secondary_bkt[i]);
+ }
+ /* Compare signatures and prefetch key slot of first hit */
+ for (i = 0; i < num_keys; i++) {
+ compare_signatures(&prim_hitmask[i], &sec_hitmask[i],
+ primary_bkt[i], secondary_bkt[i],
+ prim_hash[i], sec_hash[i], h->sig_cmp_fn);
+
+ if (prim_hitmask[i]) {
+ uint32_t first_hit = __builtin_ctzl(prim_hitmask[i]);
+ uint32_t key_idx = primary_bkt[i]->key_idx[first_hit];
+ const struct rte_hash_key *key_slot =
+ (const struct rte_hash_key *)(
+ (const char *)h->key_store +
+ key_idx * h->key_entry_size);
+ rte_prefetch0(key_slot);
+ continue;
+ }
-/* Lookup bulk stage 3: Check if key matches, update hit mask and return data */
-static inline void
-lookup_stage3(unsigned idx, const struct rte_hash_key *key_slot, const void * const *keys,
- const int32_t *positions, void *data[], uint64_t *hits,
- const struct rte_hash *h)
-{
- unsigned hit;
- unsigned key_idx;
+ if (sec_hitmask[i]) {
+ uint32_t first_hit = __builtin_ctzl(sec_hitmask[i]);
+ uint32_t key_idx = secondary_bkt[i]->key_idx[first_hit];
+ const struct rte_hash_key *key_slot =
+ (const struct rte_hash_key *)(
+ (const char *)h->key_store +
+ key_idx * h->key_entry_size);
+ rte_prefetch0(key_slot);
+ }
+ }
- hit = !rte_hash_cmp_eq(key_slot->key, keys[idx], h);
- if (data != NULL)
- data[idx] = key_slot->pdata;
+ /* Compare keys, first hits in primary first */
+ for (i = 0; i < num_keys; i++) {
+ positions[i] = -ENOENT;
+ while (prim_hitmask[i]) {
+ uint32_t hit_index = __builtin_ctzl(prim_hitmask[i]);
+
+ uint32_t key_idx = primary_bkt[i]->key_idx[hit_index];
+ const struct rte_hash_key *key_slot =
+ (const struct rte_hash_key *)(
+ (const char *)h->key_store +
+ key_idx * h->key_entry_size);
+ /*
+ * If key index is 0, do not compare key,
+ * as it is checking the dummy slot
+ */
+ if (!!key_idx & !rte_hash_cmp_eq(key_slot->key, keys[i], h)) {
+ if (data != NULL)
+ data[i] = key_slot->pdata;
- key_idx = positions[idx] + 1;
- /*
- * If key index is 0, force hit to be 0, in case key to be looked up
- * is all zero (as in the dummy slot), which would result in a wrong hit
- */
- *hits |= (uint64_t)(hit && !!key_idx) << idx;
-}
+ hits |= 1ULL << i;
+ positions[i] = key_idx - 1;
+ goto next_key;
+ }
+ prim_hitmask[i] &= ~(1 << (hit_index));
+ }
-static inline void
-__rte_hash_lookup_bulk(const struct rte_hash *h, const void **keys,
- uint32_t num_keys, int32_t *positions,
- uint64_t *hit_mask, void *data[])
-{
- uint64_t hits = 0;
- uint64_t extra_hits_mask = 0;
- uint64_t lookup_mask, miss_mask;
- unsigned idx;
- const void *key_store = h->key_store;
- int ret;
- hash_sig_t hash_vals[RTE_HASH_LOOKUP_BULK_MAX];
-
- unsigned idx00, idx01, idx10, idx11, idx20, idx21, idx30, idx31;
- const struct rte_hash_bucket *primary_bkt10, *primary_bkt11;
- const struct rte_hash_bucket *secondary_bkt10, *secondary_bkt11;
- const struct rte_hash_bucket *primary_bkt20, *primary_bkt21;
- const struct rte_hash_bucket *secondary_bkt20, *secondary_bkt21;
- const struct rte_hash_key *k_slot20, *k_slot21, *k_slot30, *k_slot31;
- hash_sig_t primary_hash10, primary_hash11;
- hash_sig_t secondary_hash10, secondary_hash11;
- hash_sig_t primary_hash20, primary_hash21;
- hash_sig_t secondary_hash20, secondary_hash21;
-
- lookup_mask = (uint64_t) -1 >> (64 - num_keys);
- miss_mask = lookup_mask;
-
- lookup_stage0(&idx00, &lookup_mask, keys);
- lookup_stage0(&idx01, &lookup_mask, keys);
-
- idx10 = idx00, idx11 = idx01;
-
- lookup_stage0(&idx00, &lookup_mask, keys);
- lookup_stage0(&idx01, &lookup_mask, keys);
- lookup_stage1(idx10, &primary_hash10, &secondary_hash10,
- &primary_bkt10, &secondary_bkt10, hash_vals, keys, h);
- lookup_stage1(idx11, &primary_hash11, &secondary_hash11,
- &primary_bkt11, &secondary_bkt11, hash_vals, keys, h);
-
- primary_bkt20 = primary_bkt10;
- primary_bkt21 = primary_bkt11;
- secondary_bkt20 = secondary_bkt10;
- secondary_bkt21 = secondary_bkt11;
- primary_hash20 = primary_hash10;
- primary_hash21 = primary_hash11;
- secondary_hash20 = secondary_hash10;
- secondary_hash21 = secondary_hash11;
- idx20 = idx10, idx21 = idx11;
- idx10 = idx00, idx11 = idx01;
-
- lookup_stage0(&idx00, &lookup_mask, keys);
- lookup_stage0(&idx01, &lookup_mask, keys);
- lookup_stage1(idx10, &primary_hash10, &secondary_hash10,
- &primary_bkt10, &secondary_bkt10, hash_vals, keys, h);
- lookup_stage1(idx11, &primary_hash11, &secondary_hash11,
- &primary_bkt11, &secondary_bkt11, hash_vals, keys, h);
- lookup_stage2(idx20, primary_hash20, secondary_hash20, primary_bkt20,
- secondary_bkt20, &k_slot20, positions, &extra_hits_mask,
- key_store, h);
- lookup_stage2(idx21, primary_hash21, secondary_hash21, primary_bkt21,
- secondary_bkt21, &k_slot21, positions, &extra_hits_mask,
- key_store, h);
-
- while (lookup_mask) {
- k_slot30 = k_slot20, k_slot31 = k_slot21;
- idx30 = idx20, idx31 = idx21;
- primary_bkt20 = primary_bkt10;
- primary_bkt21 = primary_bkt11;
- secondary_bkt20 = secondary_bkt10;
- secondary_bkt21 = secondary_bkt11;
- primary_hash20 = primary_hash10;
- primary_hash21 = primary_hash11;
- secondary_hash20 = secondary_hash10;
- secondary_hash21 = secondary_hash11;
- idx20 = idx10, idx21 = idx11;
- idx10 = idx00, idx11 = idx01;
-
- lookup_stage0(&idx00, &lookup_mask, keys);
- lookup_stage0(&idx01, &lookup_mask, keys);
- lookup_stage1(idx10, &primary_hash10, &secondary_hash10,
- &primary_bkt10, &secondary_bkt10, hash_vals, keys, h);
- lookup_stage1(idx11, &primary_hash11, &secondary_hash11,
- &primary_bkt11, &secondary_bkt11, hash_vals, keys, h);
- lookup_stage2(idx20, primary_hash20, secondary_hash20,
- primary_bkt20, secondary_bkt20, &k_slot20, positions,
- &extra_hits_mask, key_store, h);
- lookup_stage2(idx21, primary_hash21, secondary_hash21,
- primary_bkt21, secondary_bkt21, &k_slot21, positions,
- &extra_hits_mask, key_store, h);
- lookup_stage3(idx30, k_slot30, keys, positions, data, &hits, h);
- lookup_stage3(idx31, k_slot31, keys, positions, data, &hits, h);
- }
+ while (sec_hitmask[i]) {
+ uint32_t hit_index = __builtin_ctzl(sec_hitmask[i]);
+
+ uint32_t key_idx = secondary_bkt[i]->key_idx[hit_index];
+ const struct rte_hash_key *key_slot =
+ (const struct rte_hash_key *)(
+ (const char *)h->key_store +
+ key_idx * h->key_entry_size);
+ /*
+ * If key index is 0, do not compare key,
+ * as it is checking the dummy slot
+ */
+
+ if (!!key_idx & !rte_hash_cmp_eq(key_slot->key, keys[i], h)) {
+ if (data != NULL)
+ data[i] = key_slot->pdata;
- k_slot30 = k_slot20, k_slot31 = k_slot21;
- idx30 = idx20, idx31 = idx21;
- primary_bkt20 = primary_bkt10;
- primary_bkt21 = primary_bkt11;
- secondary_bkt20 = secondary_bkt10;
- secondary_bkt21 = secondary_bkt11;
- primary_hash20 = primary_hash10;
- primary_hash21 = primary_hash11;
- secondary_hash20 = secondary_hash10;
- secondary_hash21 = secondary_hash11;
- idx20 = idx10, idx21 = idx11;
- idx10 = idx00, idx11 = idx01;
-
- lookup_stage1(idx10, &primary_hash10, &secondary_hash10,
- &primary_bkt10, &secondary_bkt10, hash_vals, keys, h);
- lookup_stage1(idx11, &primary_hash11, &secondary_hash11,
- &primary_bkt11, &secondary_bkt11, hash_vals, keys, h);
- lookup_stage2(idx20, primary_hash20, secondary_hash20, primary_bkt20,
- secondary_bkt20, &k_slot20, positions, &extra_hits_mask,
- key_store, h);
- lookup_stage2(idx21, primary_hash21, secondary_hash21, primary_bkt21,
- secondary_bkt21, &k_slot21, positions, &extra_hits_mask,
- key_store, h);
- lookup_stage3(idx30, k_slot30, keys, positions, data, &hits, h);
- lookup_stage3(idx31, k_slot31, keys, positions, data, &hits, h);
-
- k_slot30 = k_slot20, k_slot31 = k_slot21;
- idx30 = idx20, idx31 = idx21;
- primary_bkt20 = primary_bkt10;
- primary_bkt21 = primary_bkt11;
- secondary_bkt20 = secondary_bkt10;
- secondary_bkt21 = secondary_bkt11;
- primary_hash20 = primary_hash10;
- primary_hash21 = primary_hash11;
- secondary_hash20 = secondary_hash10;
- secondary_hash21 = secondary_hash11;
- idx20 = idx10, idx21 = idx11;
-
- lookup_stage2(idx20, primary_hash20, secondary_hash20, primary_bkt20,
- secondary_bkt20, &k_slot20, positions, &extra_hits_mask,
- key_store, h);
- lookup_stage2(idx21, primary_hash21, secondary_hash21, primary_bkt21,
- secondary_bkt21, &k_slot21, positions, &extra_hits_mask,
- key_store, h);
- lookup_stage3(idx30, k_slot30, keys, positions, data, &hits, h);
- lookup_stage3(idx31, k_slot31, keys, positions, data, &hits, h);
-
- k_slot30 = k_slot20, k_slot31 = k_slot21;
- idx30 = idx20, idx31 = idx21;
-
- lookup_stage3(idx30, k_slot30, keys, positions, data, &hits, h);
- lookup_stage3(idx31, k_slot31, keys, positions, data, &hits, h);
-
- /* ignore any items we have already found */
- extra_hits_mask &= ~hits;
-
- if (unlikely(extra_hits_mask)) {
- /* run a single search for each remaining item */
- do {
- idx = __builtin_ctzl(extra_hits_mask);
- if (data != NULL) {
- ret = rte_hash_lookup_with_hash_data(h,
- keys[idx], hash_vals[idx], &data[idx]);
- if (ret >= 0)
- hits |= 1ULL << idx;
- } else {
- positions[idx] = rte_hash_lookup_with_hash(h,
- keys[idx], hash_vals[idx]);
- if (positions[idx] >= 0)
- hits |= 1llu << idx;
+ hits |= 1ULL << i;
+ positions[i] = key_idx - 1;
+ goto next_key;
}
- extra_hits_mask &= ~(1llu << idx);
- } while (extra_hits_mask);
- }
+ sec_hitmask[i] &= ~(1 << (hit_index));
+ }
- miss_mask &= ~hits;
- if (unlikely(miss_mask)) {
- do {
- idx = __builtin_ctzl(miss_mask);
- positions[idx] = -ENOENT;
- miss_mask &= ~(1llu << idx);
- } while (miss_mask);
+next_key:
+ continue;
}
if (hit_mask != NULL)
diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h
index 504661d..c00aafa 100644
--- a/lib/librte_hash/rte_cuckoo_hash.h
+++ b/lib/librte_hash/rte_cuckoo_hash.h
@@ -173,8 +173,7 @@ enum rte_hash_sig_compare_function {
struct rte_hash_bucket {
hash_sig_t sig_current[RTE_HASH_BUCKET_ENTRIES];
- /* Includes dummy key index that always contains index 0 */
- uint32_t key_idx[RTE_HASH_BUCKET_ENTRIES + 1];
+ uint32_t key_idx[RTE_HASH_BUCKET_ENTRIES];
hash_sig_t sig_alt[RTE_HASH_BUCKET_ENTRIES];
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6 0/4] new crypto software based device
From: De Lara Guarch, Pablo @ 2016-10-04 23:36 UTC (permalink / raw)
To: Mrozowicz, SlawomirX, dev@dpdk.org; +Cc: Mrozowicz, SlawomirX
In-Reply-To: <1475593882-32364-1-git-send-email-slawomirx.mrozowicz@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Slawomir Mrozowicz
> Sent: Tuesday, October 04, 2016 8:11 AM
> To: dev@dpdk.org
> Cc: Mrozowicz, SlawomirX
> Subject: [dpdk-dev] [PATCH v6 0/4] new crypto software based device
>
> This code provides the initial implementation of the libcrypto poll mode
> driver.
> All cryptography operations are using Openssl library crypto API.
> Each algorithm uses EVP_ interface from openssl API - which is recommended
> by
> Openssl maintainers.
>
> For more information about how to use this driver, go to:
> doc/guides/cryptodevs/libcrypto.rst
>
> Changes in V6:
> - fix checkpatch warnings
>
> Changes in V5:
> - reduce source of big data test
>
> Changes in V4:
> - move aes test rework to another patch
> - move big data test to another patch
> - checking if libcrypto pmd is available
>
> Changes in V3:
> - add nagative verification tests
> - add big data test
> - fix pmd according to negative verification tests
> - change gmac aad max size
> - update documentation and commits comments
>
> Changes in V2:
> - add gcm/gmac algorithm correction
> - unit test rework
>
> Slawomir Mrozowicz (1):
> libcrypto_pmd: initial implementation of SW crypto device
>
> Piotr Azarewicz (2)
> app/test: cryptodev AES tests rework
> app/test: added tests for libcrypto PMD
>
> Daniel Mrzyglod (1)
> examples/l2fwd-crypto: updated example for libcrypto PMD
>
> MAINTAINERS | 4 +
> app/test/Makefile | 2 +-
> app/test/test_cryptodev.c | 1584 ++++++++++++++++++--
> app/test/test_cryptodev.h | 1 +
> app/test/test_cryptodev_aes.c | 687 ---------
> app/test/test_cryptodev_aes.h | 1124 --------------
> app/test/test_cryptodev_aes_test_vectors.h | 1097 ++++++++++++++
> app/test/test_cryptodev_blockcipher.c | 538 +++++++
> app/test/test_cryptodev_blockcipher.h | 125 ++
> app/test/test_cryptodev_des_test_vectors.h | 955 ++++++++++++
> app/test/test_cryptodev_gcm_test_vectors.h | 36 +-
> app/test/test_cryptodev_hash_test_vectors.h | 491 ++++++
> app/test/test_cryptodev_perf.c | 712 ++++++++-
> config/common_base | 6 +
> doc/guides/cryptodevs/index.rst | 1 +
> doc/guides/cryptodevs/libcrypto.rst | 116 ++
> doc/guides/rel_notes/release_16_11.rst | 23 +-
> drivers/crypto/Makefile | 1 +
> drivers/crypto/libcrypto/Makefile | 60 +
> drivers/crypto/libcrypto/rte_libcrypto_pmd.c | 1062 +++++++++++++
> drivers/crypto/libcrypto/rte_libcrypto_pmd_ops.c | 708 +++++++++
> .../crypto/libcrypto/rte_libcrypto_pmd_private.h | 174 +++
> .../crypto/libcrypto/rte_pmd_libcrypto_version.map | 3 +
> examples/l2fwd-crypto/main.c | 9 +
> lib/librte_cryptodev/rte_cryptodev.h | 5 +-
> mk/rte.app.mk | 23 +-
> 26 files changed, 7621 insertions(+), 1926 deletions(-)
> delete mode 100644 app/test/test_cryptodev_aes.c
> delete mode 100644 app/test/test_cryptodev_aes.h
> create mode 100644 app/test/test_cryptodev_aes_test_vectors.h
> create mode 100644 app/test/test_cryptodev_blockcipher.c
> create mode 100644 app/test/test_cryptodev_blockcipher.h
> create mode 100644 app/test/test_cryptodev_des_test_vectors.h
> create mode 100644 app/test/test_cryptodev_hash_test_vectors.h
> create mode 100644 doc/guides/cryptodevs/libcrypto.rst
> create mode 100644 drivers/crypto/libcrypto/Makefile
> create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd.c
> create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd_ops.c
> create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd_private.h
> create mode 100644
> drivers/crypto/libcrypto/rte_pmd_libcrypto_version.map
>
> --
> 2.5.0
Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Thanks for all the rework!
^ permalink raw reply
* Re: [PATCH v6 0/4] new crypto software based device
From: De Lara Guarch, Pablo @ 2016-10-05 0:05 UTC (permalink / raw)
To: De Lara Guarch, Pablo, Mrozowicz, SlawomirX, dev@dpdk.org
Cc: Mrozowicz, SlawomirX
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA0457E@IRSMSX108.ger.corp.intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Tuesday, October 04, 2016 4:37 PM
> To: Mrozowicz, SlawomirX; dev@dpdk.org
> Cc: Mrozowicz, SlawomirX
> Subject: Re: [dpdk-dev] [PATCH v6 0/4] new crypto software based device
>
>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Slawomir
> Mrozowicz
> > Sent: Tuesday, October 04, 2016 8:11 AM
> > To: dev@dpdk.org
> > Cc: Mrozowicz, SlawomirX
> > Subject: [dpdk-dev] [PATCH v6 0/4] new crypto software based device
> >
> > This code provides the initial implementation of the libcrypto poll mode
> > driver.
> > All cryptography operations are using Openssl library crypto API.
> > Each algorithm uses EVP_ interface from openssl API - which is
> recommended
> > by
> > Openssl maintainers.
> >
> > For more information about how to use this driver, go to:
> > doc/guides/cryptodevs/libcrypto.rst
> >
> > Changes in V6:
> > - fix checkpatch warnings
> >
> > Changes in V5:
> > - reduce source of big data test
> >
> > Changes in V4:
> > - move aes test rework to another patch
> > - move big data test to another patch
> > - checking if libcrypto pmd is available
> >
> > Changes in V3:
> > - add nagative verification tests
> > - add big data test
> > - fix pmd according to negative verification tests
> > - change gmac aad max size
> > - update documentation and commits comments
> >
> > Changes in V2:
> > - add gcm/gmac algorithm correction
> > - unit test rework
> >
> > Slawomir Mrozowicz (1):
> > libcrypto_pmd: initial implementation of SW crypto device
> >
> > Piotr Azarewicz (2)
> > app/test: cryptodev AES tests rework
> > app/test: added tests for libcrypto PMD
> >
> > Daniel Mrzyglod (1)
> > examples/l2fwd-crypto: updated example for libcrypto PMD
> >
> > MAINTAINERS | 4 +
> > app/test/Makefile | 2 +-
> > app/test/test_cryptodev.c | 1584 ++++++++++++++++++--
> > app/test/test_cryptodev.h | 1 +
> > app/test/test_cryptodev_aes.c | 687 ---------
> > app/test/test_cryptodev_aes.h | 1124 --------------
> > app/test/test_cryptodev_aes_test_vectors.h | 1097 ++++++++++++++
> > app/test/test_cryptodev_blockcipher.c | 538 +++++++
> > app/test/test_cryptodev_blockcipher.h | 125 ++
> > app/test/test_cryptodev_des_test_vectors.h | 955 ++++++++++++
> > app/test/test_cryptodev_gcm_test_vectors.h | 36 +-
> > app/test/test_cryptodev_hash_test_vectors.h | 491 ++++++
> > app/test/test_cryptodev_perf.c | 712 ++++++++-
> > config/common_base | 6 +
> > doc/guides/cryptodevs/index.rst | 1 +
> > doc/guides/cryptodevs/libcrypto.rst | 116 ++
> > doc/guides/rel_notes/release_16_11.rst | 23 +-
> > drivers/crypto/Makefile | 1 +
> > drivers/crypto/libcrypto/Makefile | 60 +
> > drivers/crypto/libcrypto/rte_libcrypto_pmd.c | 1062 +++++++++++++
> > drivers/crypto/libcrypto/rte_libcrypto_pmd_ops.c | 708 +++++++++
> > .../crypto/libcrypto/rte_libcrypto_pmd_private.h | 174 +++
> > .../crypto/libcrypto/rte_pmd_libcrypto_version.map | 3 +
> > examples/l2fwd-crypto/main.c | 9 +
> > lib/librte_cryptodev/rte_cryptodev.h | 5 +-
> > mk/rte.app.mk | 23 +-
> > 26 files changed, 7621 insertions(+), 1926 deletions(-)
> > delete mode 100644 app/test/test_cryptodev_aes.c
> > delete mode 100644 app/test/test_cryptodev_aes.h
> > create mode 100644 app/test/test_cryptodev_aes_test_vectors.h
> > create mode 100644 app/test/test_cryptodev_blockcipher.c
> > create mode 100644 app/test/test_cryptodev_blockcipher.h
> > create mode 100644 app/test/test_cryptodev_des_test_vectors.h
> > create mode 100644 app/test/test_cryptodev_hash_test_vectors.h
> > create mode 100644 doc/guides/cryptodevs/libcrypto.rst
> > create mode 100644 drivers/crypto/libcrypto/Makefile
> > create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd.c
> > create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd_ops.c
> > create mode 100644 drivers/crypto/libcrypto/rte_libcrypto_pmd_private.h
> > create mode 100644
> > drivers/crypto/libcrypto/rte_pmd_libcrypto_version.map
> >
> > --
> > 2.5.0
>
> Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>
> Thanks for all the rework!
Applied to dpdk-next-crypto.
I made a small change in the first patch, due to the changes introduced in
http://dpdk.org/ml/archives/dev/2016-September/047087.html,
which affects driver registration.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH] examples/ipsec-secgw: Update checksum while decrementing ttl
From: De Lara Guarch, Pablo @ 2016-10-05 0:34 UTC (permalink / raw)
To: Gonzalez Monroy, Sergio, akhil.goyal@nxp.com, dev@dpdk.org
In-Reply-To: <df1b09f9-b7f7-8f76-b1b4-6098257d9f8b@intel.com>
> -----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).
>
> 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.
>
> Also, in the inbound path you have to consider a possible ECN value update.
Any further comments here, Akhil?
Thanks,
Pablo
>
> 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 v3 0/9] IPsec Enhancements
From: De Lara Guarch, Pablo @ 2016-10-05 0:43 UTC (permalink / raw)
To: De Lara Guarch, Pablo, Gonzalez Monroy, Sergio, dev@dpdk.org
In-Reply-To: <E115CCD9D858EF4F90C690B0DCB4D8973CA024D3@IRSMSX108.ger.corp.intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of De Lara Guarch,
> Pablo
> Sent: Thursday, September 29, 2016 5:28 PM
> To: Gonzalez Monroy, Sergio; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/9] IPsec Enhancements
>
>
>
> > -----Original Message-----
> > From: Gonzalez Monroy, Sergio
> > Sent: Thursday, September 29, 2016 8:44 AM
> > To: dev@dpdk.org
> > Cc: De Lara Guarch, Pablo
> > Subject: [PATCH v3 0/9] IPsec Enhancements
> >
> > This patch set mainly adds support for AES-GCM and AES-CTR.
> >
> > It also updates the IV generation method for AES-CBC mode using
> > the forward function instead of randomly generating the IV.
> >
> > v3:
> > - update sample app guide
> > - remove unused function
> > - improve commit messages
> >
> > v2:
> > - Update releas notes.
> > - Initialize salt values, GCM/CTR key length now is 20B,
> > 16B key and 4 LSB salt.
> > - Do not check SP/ACL if we have no rules.
> > - Add macro for cryptodev queue size
> >
> > Dependencies:
> > examples/ipsec-secgw: add configuration file support
> > http://dpdk.org/dev/patchwork/patch/16004/
> >
> > examples/ipsec-secgw: add sample configuration files
> > http://dpdk.org/dev/patchwork/patch/16003/
> >
> > Sergio Gonzalez Monroy (9):
> > examples/ipsec-secgw: change CBC IV generation
> > examples/ipsec-secgw: reset crypto operation status
> > examples/ipsec-secgw: add AES-GCM support
> > examples/ipsec-secgw: enable AES-CTR mode
> > examples/ipsec-secgw: check sp only when setup
> > examples/ipsec-secgw: add cryptodev queue size macro
> > examples/ipsec-secgw: initialize sa salt
> > examples/ipsec-secgw: update release notes
> > examples/ipsec-secgw: update ipsec-secgw guide
> >
> > doc/guides/rel_notes/release_16_11.rst | 9 ++
> > doc/guides/sample_app_ug/ipsec_secgw.rst | 15 ++--
> > examples/ipsec-secgw/esp.c | 144 ++++++++++++++++++++++---------
> > examples/ipsec-secgw/ipsec-secgw.c | 7 +-
> > examples/ipsec-secgw/ipsec.c | 1 +
> > examples/ipsec-secgw/ipsec.h | 35 +++++++-
> > examples/ipsec-secgw/sa.c | 54 ++++++++++--
> > 7 files changed, 207 insertions(+), 58 deletions(-)
> >
> > --
> > 2.5.5
>
> Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Applied to dpdk-next-crypto.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH v2] app/test: remove hard-coding of crypto num qps
From: De Lara Guarch, Pablo @ 2016-10-05 0:49 UTC (permalink / raw)
To: Trahe, Fiona, dev@dpdk.org; +Cc: Trahe, Fiona, Akhil Goyal
In-Reply-To: <1475169509-19247-1-git-send-email-fiona.trahe@intel.com>
Hi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fiona Trahe
> Sent: Thursday, September 29, 2016 10:18 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Trahe, Fiona; Akhil Goyal
> Subject: [dpdk-dev] [PATCH v2] app/test: remove hard-coding of crypto num
> qps
>
> ts_params->conf.nb_queue_pairs should not be hard coded with device
> specific number. It should be retrieved from the device info.
> Any test which changes it should restore it to orig value.
>
> Also related cleanup of test code setting number and size of
> queue-pairs on a device, e.g.
> * Removed irrelevant “for” loop – was hardcoded to only loop once.
> * Removed obsolete comment re inability to free and re-allocate queu
> memory
> and obsolete workaround for it which used to create maximum size queues.
>
> And added freeing of ring memory on queue-pair release in aesni_mb PMD,
> else releasing and setting up queue-pair of a different size fails.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> Signed-off-by: Fiona Trahe <fiona.trahe@intel.com>
> ---
>
> v2:
> Fix for broken QAT PMD unit tests exposed by v1
> i.e. In test_device_configure_invalid_queue_pair_ids() after running tests
> for invalid values restore original nb_queue_pairs.
> Also cleanup of test code setting number and size of queue-pairs on a device
> Also fix for aesni_mb PMD not freeing ring memory on qp release
Sorry, I missed this patch. Could you split this patch into different patches?
It looks like you are making (three?) changes in different places.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH 1/2] i40e: Add packet_type metadata in the i40e vPMD
From: Chen, Jing D @ 2016-10-05 1:29 UTC (permalink / raw)
To: Shaw, Jeffrey B, dev@dpdk.org, Zhang, Helin, Wu, Jingjing,
damarion@cisco.com
In-Reply-To: <1468515542-39207-2-git-send-email-jeffrey.b.shaw@intel.com>
Hi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Shaw
> Sent: Thursday, July 14, 2016 9:59 AM
> To: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; damarion@cisco.com
> Subject: [dpdk-dev] [PATCH 1/2] i40e: Add packet_type metadata in the i40e
> vPMD
>
> From: Damjan Marion <damarion@cisco.com>
>
> The ptype is decoded from the rx descriptor and stored in the packet type
> field in the mbuf using the same function as the non-vector driver.
>
> Signed-off-by: Damjan Marion <damarion@cisco.com>
> Signed-off-by: Jeff Shaw <jeffrey.b.shaw@intel.com>
> ---
> drivers/net/i40e/i40e_rxtx.c | 566 +--------------------------------------
> drivers/net/i40e/i40e_rxtx.h | 563
> ++++++++++++++++++++++++++++++++++++++
> drivers/net/i40e/i40e_rxtx_vec.c | 16 ++
> 3 files changed, 581 insertions(+), 564 deletions(-)
>
> -
> #define I40E_RX_DESC_EXT_STATUS_FLEXBH_MASK 0x03
> #define I40E_RX_DESC_EXT_STATUS_FLEXBH_FD_ID 0x01
> #define I40E_RX_DESC_EXT_STATUS_FLEXBH_FLEX 0x02
> @@ -2136,7 +1573,8 @@ i40e_dev_supported_ptypes_get(struct rte_eth_dev
> *dev) #ifdef RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC
> dev->rx_pkt_burst == i40e_recv_pkts_bulk_alloc || #endif
> - dev->rx_pkt_burst == i40e_recv_scattered_pkts)
> + dev->rx_pkt_burst == i40e_recv_scattered_pkts ||
> + dev->rx_pkt_burst == i40e_recv_pkts_vec)
Missed i40e_recv_scattered_pkts_vec()?
^ permalink raw reply
* Re: [PATCH] cryptodev: fix compilation error in SUSE 11 SP2
From: De Lara Guarch, Pablo @ 2016-10-05 2:31 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev@dpdk.org, Doherty, Declan
In-Reply-To: <20160930083359.GU17252@6wind.com>
Hi Adrien,
> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Friday, September 30, 2016 1:34 AM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org; Doherty, Declan
> Subject: Re: [PATCH] cryptodev: fix compilation error in SUSE 11 SP2
>
> On Thu, Sep 29, 2016 at 07:30:31PM +0000, De Lara Guarch, Pablo wrote:
> > > -----Original Message-----
> > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > Sent: Tuesday, September 27, 2016 12:45 AM
> > > To: De Lara Guarch, Pablo
> > > Cc: dev@dpdk.org; Doherty, Declan
> > > Subject: Re: [PATCH] cryptodev: fix compilation error in SUSE 11 SP2
> > >
> > > On Mon, Sep 26, 2016 at 10:50:35PM +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:71:0:
> > > > lib/librte_cryptodev/rte_cryptodev_pmd.h:76: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>
> > >
> > > Hmm, this error message does not seem related to your patch. Assuming a
> > > similar error is caused by the original code, I think there is a more
> > > important issue as the struct should not be empty. Can you check the
> > > error?
> >
> > Well, I don't really understand what is the difference between array[] and
> array[0],
> > I thought both were the same, but some compilers only accept the latter.
>
> Before array[] got standardized by C99, a common trick was to use array[0],
> in a sense they are similar except for this one case: a struct with a single
> array[] field is explicitly not allowed in C99 since it causes the structure
> to be empty (this syntax only provides an alignment constraint for what
> follows in case padding is required), no such problem with array[0] which
> although nonstandard, is an accepted behavior, sizeof(struct foo) may yield
> 0 without complaint.
>
> > In any case, the struct will not be empty, as there are other fields, that are
> not variable sized.
> >
> > I saw that in your patch you made these two changes (among others):
> >
> > diff --git a/lib/librte_cryptodev/rte_cryptodev.h
> b/lib/librte_cryptodev/rte_cryptodev.h
> > index affbdec..1e30a19 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev.h
> > +++ b/lib/librte_cryptodev/rte_cryptodev.h
> > @@ -759,7 +759,7 @@ struct rte_cryptodev_sym_session {
> > } __rte_aligned(8);
> > /**< Public symmetric session details */
> >
> > - char _private[0];
> > + char _private[];
> > /**< Private session material */
> > };
> >
> > diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> > index 7d049ea..42e7b79 100644
> > --- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
> > +++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
> > @@ -71,7 +71,7 @@ struct rte_cryptodev_session {
> > struct rte_mempool *mp;
> > } __rte_aligned(8);
> >
> > - char _private[0];
> > + __extension__ char _private[0];
> > };
> >
> > So I would expect the same change in both, as they are almost identical,
> > but you took different approaches (do you know why? I would like to know
> :))
>
> Yes, this was done to address the exact same error (probably with the same
> old GCC version (4.4.7 perhaps?)), hence my surprise to see it fixed once
> again according to your commit log, I think your only mistake was to paste
> the error message for the wrong header in there (rte_cryptodev_pmd.h
> instead
> of rte_cryptodev.h), nothing wrong with your patch besides this.
Ohhh, all right! I understand now. Will send a v2 with the commit message fixed.
Thanks!
Pablo
>
> > Basically, I noticed that gcc 4.5 doesn't complain when using your second
> approach,
> > that's why I changed it.
>
> For the record GCC wrongly thinks the structure is empty because a unnamed
> struct field is declared inside. Before C11 such declarations only created a
> new type that did not occupy any space and not an actual field, hence why it
> complains when faced with [] instead of the well-behaved [0].
>
> In this particular case it's a parsing error fixed in subsequent GCC
> versions, the unnamed struct actually uses some space otherwise it would
> have crashed during non-regression testing (right?)
>
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply
* [PATCH v2] cryptodev: fix compilation error in SUSE 11 SP2
From: Pablo de Lara @ 2016-10-05 2:45 UTC (permalink / raw)
To: dev; +Cc: adrien.mazarguil, declan.doherty, Pablo de Lara
In-Reply-To: <1474926635-13290-1-git-send-email-pablo.de.lara.guarch@intel.com>
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
^ permalink raw reply related
* 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