[dpdk-dev] [PATCH] scripts: check commit formatting

2016-03-30 Thread Yuanhan Liu
On Wed, Mar 30, 2016 at 03:27:40PM +0100, Bruce Richardson wrote:
> On Wed, Mar 30, 2016 at 09:46:34AM +0800, Yuanhan Liu wrote:
> > On Tue, Mar 29, 2016 at 11:29:46PM +0200, Thomas Monjalon wrote:
> > > The git messages have three parts:
> > > 1/ the headline
> > > 2/ the explanations
> > > 3/ the footer tags
> > > 
> > > The headline helps to quickly browse an history or catch instantly the
> > > purpose of a commit. Making it short with some consistent wording
> > > allows to easily parse it or match some patterns.
> > > 
> > > The explanations must give some keys like the reason of the change.
> > > Nothing can be automatically checked for this part.
> > 
> > Actually, I think we might be able to do 2 tests here:
> > 
> > - space line between paragraphs.
> > 
> > - lines over 80 chars.
> 
> 75 chars for commit messages, and 50 for commit titles :-)

I'd agree that 75 char is better. My personal preference is actually
68 :) 

--yliu


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Harish Patil
>
>On Tue, 29 Mar 2016 22:28:20 -0700
>Rasesh Mody  wrote:
>
>> +static void
>> +qede_mac_addr_remove(__rte_unused struct rte_eth_dev *eth_dev,
>> + __rte_unused uint32_t index)
>> +{
>> +struct qede_dev *qdev = eth_dev->data->dev_private;
>> +struct ecore_dev *edev = >edev;
>> +
>> +/* TBD: Not implemented currently because DPDK does not provide
>> + * macaddr and instead just passes the index. So pmd needs to
>> + * maintain index mapping to macaddr.
>> + */
>> +DP_NOTICE(edev, false, "%s: Unsupported operation\n", __func__);
>> +}
>> +
>
>Rather than have a stub and break software, why not just
>not define the operation and let base DPDK handle the error?
>
Okay. I shall have it implemented and not leave it as stubbed one.



[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Harish Patil
>
>On Tue, 29 Mar 2016 22:28:20 -0700
>Rasesh Mody  wrote:
>
>> +
>> +static void qede_print_adapter_info(struct qede_dev *qdev)
>> +{
>> +struct ecore_dev *edev = >edev;
>> +struct qed_dev_info *info = >dev_info.common;
>> +char ver_str[QED_DRV_VER_STR_SIZE] = { 0 };
>> +
>> +RTE_LOG(INFO, PMD,
>> +  " Chip details : %s%d\n",
>> +  ECORE_IS_BB(edev) ? "BB" : "AH",
>> +  CHIP_REV_IS_A0(edev) ? 0 : 1);
>> +
>> +sprintf(ver_str, "%s %s_%d.%d.%d.%d", QEDE_PMD_VER_PREFIX,
>> +edev->ver_str, QEDE_PMD_VERSION_MAJOR, QEDE_PMD_VERSION_MINOR,
>> +QEDE_PMD_VERSION_REVISION, QEDE_PMD_VERSION_PATCH);
>> +strcpy(qdev->drv_ver, ver_str);
>> +RTE_LOG(INFO, PMD, " Driver version : %s\n", ver_str);
>> +
>> +ver_str[0] = '\0';
>> +sprintf(ver_str, "%d.%d.%d.%d", info->fw_major, info->fw_minor,
>> +info->fw_rev, info->fw_eng);
>> +RTE_LOG(INFO, PMD, " Firmware version : %s\n", ver_str);
>> +
>> +ver_str[0] = '\0';
>> +sprintf(ver_str, "%d.%d.%d.%d",
>> +(info->mfw_rev >> 24) & 0xff,
>> +(info->mfw_rev >> 16) & 0xff,
>> +(info->mfw_rev >> 8) & 0xff, (info->mfw_rev) & 0xff);
>> +RTE_LOG(INFO, PMD, " Management firmware version : %s\n", ver_str);
>> +
>> +RTE_LOG(INFO, PMD, " Firmware file : %s\n", QEDE_FW_FILE_NAME);
>
>This means the driver is far too chatty in the logs.
>Can't this be made DEBUG level?
>
Not clear what is the issue here?
RTE_LOG is used here to display basic adapter info like firmware/driver
versions etc without the need to enable any debug flags.
The driver debug logging is under the control of appropriate debug flags.



[dpdk-dev] [PATCH] enic: expose RX missed packets counter

2016-03-30 Thread Thomas Monjalon
2016-03-30 11:07, John Daley:
> Update the 'imissed' counter with the number of packets dropped
> by the NIC.
> 
> Fixes: fefed3d1e62c ("enic: new driver")
> 
> Signed-off-by: John Daley 
> Reviewed-by: Nelson Escobar 

Applied, thanks


[dpdk-dev] [PATCH] examples/ip_pipeline: fix SSE4.2 optimization branch

2016-03-30 Thread Thomas Monjalon
The branch was disabled because of a typo in the SSE4.2 flag.
Change also the x86_64 flag to use a DPDK one.

Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64 without 
SSE4.2")

Signed-off-by: Thomas Monjalon 
Acked-by: Cristian Dumitrescu 
---
 examples/ip_pipeline/pipeline/hash_func.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ip_pipeline/pipeline/hash_func.h 
b/examples/ip_pipeline/pipeline/hash_func.h
index 1953ad4..9db7173 100644
--- a/examples/ip_pipeline/pipeline/hash_func.h
+++ b/examples/ip_pipeline/pipeline/hash_func.h
@@ -152,7 +152,7 @@ hash_xor_key64(void *key, __rte_unused uint32_t key_size, 
uint64_t seed)
return (xor0 >> 32) ^ xor0;
 }

-#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
+#if defined(RTE_ARCH_X86_64) && defined(RTE_MACHINE_CPUFLAG_SSE4_2)

 #include 

-- 
2.7.0



[dpdk-dev] [PATCH] ixgbe: return err for too many interrupt queues

2016-03-30 Thread Thomas Monjalon
> > The lower 16 bits of EICR register are used for queue interrupts, dpdk 
> > framework
> > take over the first bit for other interrupts like LSC, so there're only 15 
> > bits left for
> > queue interrupts mapping.
> > This patch adds a check for the num of interrupt queues at dev_start.
> > 
> > Signed-off-by: Wang Xiao W 
> Acked-by: Wenzhuo Lu 

Applied, thanks


[dpdk-dev] [PATCH] ixgbe/base: fix VF multi-queue failure

2016-03-30 Thread Thomas Monjalon
2016-03-30 10:46, Wenzhuo Lu:
> When starting testpmd with multiple queues on a ixgbe VF
> port, it failed with this printing, "nb_rxq(4) is greater
> than max_rx_queues(1)".
> 
> The root cause is the VF doesn't get the right max rx queue
> number from PF and it uses the default value 1.
> VF max rx queue number is set by PF through mailbox messages.
> The message for this setting only supports version 1.1. As
> message version is updated to 1.2, VF cannot parse the rx queue
> number setting message correctly.
> 
> This patch raise a specific base code update for this issue.
> 
> Fixes: 72dec9e37a84 ("ixgbe: support multicast promiscuous mode on VF")
> Signed-off-by: Wenzhuo Lu 

Applied, thanks


[dpdk-dev] [PATCH] enic: state change from link-down to link-up not recognized

2016-03-30 Thread Thomas Monjalon
2016-03-25 17:45, John Daley:
> When the enic was disabled, link notification was correctly disabled
> in the NIC but the software indicator that it was disabled was not
> updated (vdev->notify_pa not set to 0). When the link came back up,
> enic did not re-enable notification in the NIC.
> 
> This affected bonding when a enic slave device link bounced.
> 
> The fix is to unconditionally enable notification when the enic is
> enabled.
> 
> Fixes: 9913fbb91df0 ("enic/base: common code")
> 
> Signed-off-by: John Daley 
> Reviewed-by: Nelson Escobar 

Applied, thanks


[dpdk-dev] [PATCH] enic: fix TX hang when number of packets > queue size

2016-03-30 Thread Thomas Monjalon
2016-03-25 17:43, John Daley:
> If the nb_pkts parameter to rte_eth_tx_burst() was greater than
> the TX descriptor count, a completion was not being requested
> from the NIC, so descriptors would not be released back to the
> host causing a lock-up.
> 
> Introduce a limit of how many TX descriptors can be used in a single
> call to the enic PMD burst TX function before requesting a completion.
> 
> Fixes: d739ba4c6abf ("enic: improve Tx packet rate")
> 
> Signed-off-by: John Daley 

Applied, thanks


[dpdk-dev] New Defects reported by Coverity Scan for DPDK Data Plane Development Kit

2016-03-30 Thread Mcnamara, John
Hi, 

The following is forwarded from latest Coverity scan on the DPDK head. As 
usual, I will send out semi-automated emails to the authors of the new defects.

In the meantime you can then review the defects online at:

http://scan.coverity.com/projects/dpdk-data-plane-development-kit

You can register as "Contributor/Member" for the DPDK Coverity here:

http://scan.coverity.com/users/sign_up

John.


Hi,

Please find the latest report on new defect(s) introduced to DPDK Data Plane 
Development Kit found with Coverity Scan.

9 new defect(s) introduced to DPDK Data Plane Development Kit found with 
Coverity Scan.
3 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent 
build analyzed by Coverity Scan.

New defect(s) Reported-by: Coverity Scan Showing 9 of 9 defect(s)


** CID 124575:(STRING_OVERFLOW)
/examples/l2fwd-crypto/main.c: 1005 in l2fwd_crypto_parse_args_long_options()
/examples/l2fwd-crypto/main.c: 982 in l2fwd_crypto_parse_args_long_options()



*** CID 124575:(STRING_OVERFLOW)
/examples/l2fwd-crypto/main.c: 1005 in l2fwd_crypto_parse_args_long_options()
999 
1000/* Authentication options */
1001else if (strcmp(lgopts[option_index].name, "auth_algo") == 0) {
1002retval = parse_auth_algo(>auth_xform.auth.algo,
1003optarg);
1004if (retval == 0)
>>> CID 124575:(STRING_OVERFLOW)
>>> You might overrun the 32 byte fixed-size string 
>>> "options->string_auth_algo" by copying "optarg" without checking the length.
1005strcpy(options->string_auth_algo, optarg);
1006return retval;
1007}
1008 
1009else if (strcmp(lgopts[option_index].name, "auth_op") == 0)
1010return parse_auth_op(>auth_xform.auth.op,
/examples/l2fwd-crypto/main.c: 982 in l2fwd_crypto_parse_args_long_options()
976 
977 /* Cipher options */
978 else if (strcmp(lgopts[option_index].name, "cipher_algo") == 0) 
{
979 retval = 
parse_cipher_algo(>cipher_xform.cipher.algo,
980 optarg);
981 if (retval == 0)
>>> CID 124575:(STRING_OVERFLOW)
>>> You might overrun the 32 byte fixed-size string 
>>> "options->string_cipher_algo" by copying "optarg" without checking the 
>>> length.
982 strcpy(options->string_cipher_algo, optarg);
983 return retval;
984 }
985 
986 else if (strcmp(lgopts[option_index].name, "cipher_op") == 0)
987 return parse_cipher_op(>cipher_xform.cipher.op,

** CID 124567:  Memory - corruptions  (OVERRUN)
/examples/ip_pipeline/init.c: 246 in app_init_eal()



*** CID 124567:  Memory - corruptions  (OVERRUN)
/examples/ip_pipeline/init.c: 246 in app_init_eal()
240 
241 if (p->socket_mem) {
242 snprintf(buffer,
243 sizeof(buffer),
244 "--socket-mem=%s",
245 p->socket_mem);
>>> CID 124567:  Memory - corruptions  (OVERRUN)
>>> Overrunning array "app->eal_argv" of 33 8-byte elements at element 
>>> index 33 (byte offset 264) using index "n_args++" (which evaluates to 33).
246 app->eal_argv[n_args++] = strdup(buffer);
247 }
248 
249 if (p->huge_dir) {
250 snprintf(buffer, sizeof(buffer), "--huge-dir=%s", 
p->huge_dir);
251 app->eal_argv[n_args++] = strdup(buffer);

** CID 124565:  Null pointer dereferences  (NULL_RETURNS)
/lib/librte_vhost/virtio-net.c: 296 in vhost_destroy_device()



*** CID 124565:  Null pointer dereferences  (NULL_RETURNS)
/lib/librte_vhost/virtio-net.c: 296 in vhost_destroy_device()
290  */
291 void
292 vhost_destroy_device(struct vhost_device_ctx ctx)
293 {
294 struct virtio_net *dev = get_device(ctx);
295 
>>> CID 124565:  Null pointer dereferences  (NULL_RETURNS)
>>> Dereferencing a null pointer "dev".
296 if (dev->flags & VIRTIO_DEV_RUNNING)
297 notify_ops->destroy_device(dev);
298 
299 cleanup_device(dev, 1);
300 free_device(dev);
301 

** CID 124564:  Control flow issues  (MISSING_BREAK)
/app/test-pmd/cmdline.c: 8219 in cmd_flow_director_filter_parsed()



*** CID 124564:  Control flow issues  (MISSING_BREAK)
/app/test-pmd/cmdline.c: 

[dpdk-dev] [PATCH 0/2] Fix multiple build errors for Amazon ENA driver

2016-03-30 Thread Thomas Monjalon
2016-03-29 14:43, Daniel Mrzyglod:
> First patch cover multiple error for ICC.
>   a. mixed enum with other types
>   b. variable is used uninitialized
> 
> Patches for FreeBSD:
>   a. Target was not setup
>   b. ETIME is not avaliable in FreeBSD 
> 
> Daniel Mrzyglod (2):
>   ena: icc fix compilation errors
>   ena: Fix Compilation for freebsd

Applied, thanks


[dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

2016-03-30 Thread Thomas Monjalon
2016-03-30 09:36, Stephen Hemminger:
> On Wed, 30 Mar 2016 10:06:36 -0400
> Aaron Conole  wrote:
> > --- a/drivers/net/e1000/Makefile
> > +++ b/drivers/net/e1000/Makefile
> > @@ -54,6 +54,9 @@ else
> >  #
> >  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
> >  CFLAGS_BASE_DRIVER += -Wno-unused-variable
> > +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> > +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> > +endif
> >  endif
> 
> NAK, don't do it to the whole file.
> Fix the code (best option)
> or use a pragma for the small area which is broken for other reasons.

Stephen, your solutions do not work because Aaron has not been allowed
to modify this file.
As long as we are not allowed to modify the Intel base drivers,
I don't see any problem to hide some warnings in them.
The warnings could help us to clean the code or fix some bugs but
we are not allowed to...
It is the responsibility of the driver maintainer to keep this nasty code.


[dpdk-dev] Question on examples/multi_process app

2016-03-30 Thread Harish Patil
>>
>>
>>
>>
>>
>>Hi Harish,
>>
>>> >> >
>>> >> >> -Original Message-
>>> >> >> From: Richardson, Bruce
>>> >> >> Sent: Wednesday, March 23, 2016 11:45 AM
>>> >> >> To: Ananyev, Konstantin
>>> >> >> Cc: Harish Patil; dev at dpdk.org
>>> >> >> Subject: Re: [dpdk-dev] Question on examples/multi_process app
>>> >> >>
>>> >> >> On Wed, Mar 23, 2016 at 11:09:17AM +, Ananyev, Konstantin
>>>wrote:
>>> >> >> > Hi everyone,
>>> >> >> >
>>> >> >> > > -Original Message-
>>> >> >> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Bruce
>>> >> >>Richardson
>>> >> >> > > Sent: Tuesday, March 22, 2016 9:38 PM
>>> >> >> > > To: Harish Patil
>>> >> >> > > Cc: dev at dpdk.org
>>> >> >> > > Subject: Re: [dpdk-dev] Question on examples/multi_process
>>>app
>>> >> >> > >
>>> >> >> > > On Tue, Mar 22, 2016 at 08:03:42PM +, Harish Patil wrote:
>>> >> >> > > > Hi,
>>> >> >> > > > I have a question regarding symmetric_mp and mp_server
>>> >> >>applications under
>>> >> >> > > > examples/multi_process. In those apps,
>>> >> >>rte_eth_promiscuous_enable() is
>>> >> >> > > > called before rte_eth_dev_start(). Is this the correct way
>>>to
>>> >> >>initialize
>>> >> >> > > > the port/device? As per the description in
>>> >> >> > > > http://dpdk.org/doc/api/rte__ethdev_8h.html:
>>> >> >> > > >
>>> >> >> > > > "The functions exported by the application Ethernet API to
>>> >>setup
>>> >> >>a device
>>> >> >> > > > designated by its port identifier must be invoked in the
>>> >> >>following order:
>>> >> >> > > >
>>> >> >> > > > * rte_eth_dev_configure()
>>> >> >> > > > * rte_eth_tx_queue_setup()
>>> >> >> > > > * rte_eth_rx_queue_setup()
>>> >> >> > > > * rte_eth_dev_start()
>>> >> >> > > >
>>> >> >> > > > Then, the network application can invoke, in any order, the
>>> >> >>functions
>>> >> >> > > > exported by the Ethernet API to get the MAC address of a
>>>given
>>> >> >>device, to
>>> >> >> > > > get the speed and the status of a device physical link, to
>>> >> >> > > > receive/transmit [burst of] packets, and so on.?
>>> >> >> > > >
>>> >> >> > > > So should I consider this as an application issue or
>>>whether
>>> >>the
>>> >> >>PMD is
>>> >> >> > > > expected to handle it? If PMD is to handle it, then should
>>>the
>>> >> >>PMD be:
>>> >> >> > > >
>>> >> >> > > > 1) Rejecting the Promisc config? OR
>>> >> >> > > > 2) Cache the config and apply when dev_start() is called at
>>> >>later
>>> >> >>point?
>>> >> >> >
>>> >> >> > Yes as I remember 2) is done.
>>> >> >> > dev_start() invokes rte_eth_dev_config_restore(), which
>>>restores
>>> >> >> > promisc mode, mac addresses, etc.
>>> >> >> >
>>> >> >> > > >
>>> >> >> > > > Thanks,
>>> >> >> > > > Harish
>>> >> >> > > >
>>> >> >> > > Good question. I think most/all of the Intel adapters, - for
>>> >>which
>>> >> >>the app was
>>> >> >> > > originally written, way back in the day when there were only
>>>2
>>> >>PMDs
>>> >> >>in DPDK :)
>>> >> >> > > - will handle the promiscuous mode call either before or
>>>after
>>> >>the
>>> >> >>dev start.
>>> >> >> > > Assuming that's the case, and if it makes life easier for
>>>other
>>> >> >>driver writers,
>>> >> >> > > we should indeed standardize on one supported way of doing
>>> >>things -
>>> >> >>the way
>>> >> >> > > specified in the documentation being that one way, I would
>>>guess.
>>> >> >> > >
>>> >> >> > > So, e1000, ixgbe maintainers - do you see any issues with
>>>forcing
>>> >> >>the promiscuous
>>> >> >> > > mode set API to be called after the call to dev_start()?
>>> >> >> >
>>> >> >> > Not sure, why do we need to enforce that restriction?
>>> >> >> > Is there any problem with current way?
>>> >>
>>> >> Yes, at least with the our driver/firmware interface. The
>>>port/device
>>> >> bring-up is carried out in a certain order which requires port
>>>config
>>> >>like
>>> >> promisc mode is called after dev_start().
>>> >>
>>> >> >>
>>> >> >> It complicates things for driver writers is all,
>>> >> >
>>> >> >Not sure how?
>>> >> >All this replay is done at rte_ethdev layer.
>>> >> >Honestly, so far I don't remember any complaint about promisc
>>>on/off.
>>> >> >
>>> >> >> and conflicts slightly with
>>> >> >> what is stated in the docs.
>>> >> >
>>> >> >Update the docs? :)
>>> >>
>>> >> Anyway, please let me know what you guys decide? If the app is
>>>changed
>>> >> then nothing needs to done on driver side. Otherwise I have to think
>>>of
>>> >> how to handle this.
>>> >>
>>> >
>>> >So you are saying that for your device if
>>>dev_ops->promiscuous_enable()
>>> >is called before dev_ops->dev_start(), it would cause  a problem
>>>right?
>>> >Konstantin
>>> >
>>> >
>>> 
>>> Yes, with the way it is implemented currently it would pose a problem.
>>> Please note that it can be addressed in the driver, not an issue.
>>>However,
>>> I wanted to be sure if the app behavior is correct. Either ways, please
>>> let me know - I can take care of both.
>>
>>If that's a real HW limitation, then my opinion yes, we 

[dpdk-dev] [PATCH v2] hash: fix to support multi process

2016-03-30 Thread Pablo de Lara
Hash library used a function pointer to choose a different
key compare function, depending on the key size.
As a result, multiple processes could not use the same hash table,
as the function addresses vary from one process to another.

Instead, a jump table is used, so each process has its own
function addresses, accessing this table with an index stored
in the hash table (note that using a custom key compare function
is not supported in multi-process mode).

Fixes: 48a399119619 ("hash: replace with cuckoo hash implementation")

Signed-off-by: Pablo de Lara 
Acked-by: Bruce Richardson 
---

Changes in v2:
- Added missing const
- Added extra info in documentation

 doc/guides/prog_guide/hash_lib.rst |  8 
 doc/guides/rel_notes/release_16_04.rst |  8 
 lib/librte_hash/rte_cuckoo_hash.c  | 85 ++
 3 files changed, 82 insertions(+), 19 deletions(-)

diff --git a/doc/guides/prog_guide/hash_lib.rst 
b/doc/guides/prog_guide/hash_lib.rst
index 8f2cdbf..7944640 100644
--- a/doc/guides/prog_guide/hash_lib.rst
+++ b/doc/guides/prog_guide/hash_lib.rst
@@ -87,6 +87,14 @@ or stored in the hash table itself.
 The example hash tables in the L2/L3 Forwarding sample applications defines 
which port to forward a packet to based on a packet flow identified by the 
five-tuple lookup.
 However, this table could also be used for more sophisticated features and 
provide many other functions and actions that could be performed on the packets 
and flows.

+Multi-process support
+-
+
+The hash library can be used in a multi-process environment, minding that only 
lookups are thread-safe.
+The only function that can only be used in single-process mode is 
rte_hash_set_cmp_func(), which sets up
+a custom compare function, which is assigned to a function pointer (therefore, 
it is not supported in
+multi-process mode).
+
 Implementation Details
 --

diff --git a/doc/guides/rel_notes/release_16_04.rst 
b/doc/guides/rel_notes/release_16_04.rst
index 79d76e1..e6cc34d 100644
--- a/doc/guides/rel_notes/release_16_04.rst
+++ b/doc/guides/rel_notes/release_16_04.rst
@@ -404,6 +404,14 @@ Libraries
   Fix crc32c hash functions to return a valid crc32c value for data lengths
   not multiple of 4 bytes.

+* **hash: Fixed hash library to support multi-process mode.**
+
+  Fix hash library to support multi-process mode, using a jump table,
+  instead of storing a function pointer to the key compare function.
+  Multi-process mode only works with the built-in compare functions,
+  however a custom compare function (not in the jump table) can only
+  be used in single-process mode.
+
 * **librte_port: Fixed segmentation fault for ring and ethdev writer nodrop.**

   Fixed core dump issue on txq and swq when dropless is set to yes.
diff --git a/lib/librte_hash/rte_cuckoo_hash.c 
b/lib/librte_hash/rte_cuckoo_hash.c
index 71b5b76..bdba557 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -102,6 +102,41 @@ EAL_REGISTER_TAILQ(rte_hash_tailq)

 #define LCORE_CACHE_SIZE   8

+/*
+ * All different options to select a key compare function,
+ * based on the key size and custom function.
+ */
+enum cmp_jump_table_case {
+   KEY_CUSTOM = 0,
+   KEY_16_BYTES,
+   KEY_32_BYTES,
+   KEY_48_BYTES,
+   KEY_64_BYTES,
+   KEY_80_BYTES,
+   KEY_96_BYTES,
+   KEY_112_BYTES,
+   KEY_128_BYTES,
+   KEY_OTHER_BYTES,
+   NUM_KEY_CMP_CASES,
+};
+
+/*
+ * Table storing all different key compare functions
+ * (multi-process supported)
+ */
+const rte_hash_cmp_eq_t cmp_jump_table[NUM_KEY_CMP_CASES] = {
+   NULL,
+   rte_hash_k16_cmp_eq,
+   rte_hash_k32_cmp_eq,
+   rte_hash_k48_cmp_eq,
+   rte_hash_k64_cmp_eq,
+   rte_hash_k80_cmp_eq,
+   rte_hash_k96_cmp_eq,
+   rte_hash_k112_cmp_eq,
+   rte_hash_k128_cmp_eq,
+   memcmp
+};
+
 struct lcore_cache {
unsigned len; /**< Cache len */
void *objs[LCORE_CACHE_SIZE]; /**< Cache objects */
@@ -115,7 +150,10 @@ struct rte_hash {
uint32_t key_len;   /**< 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_cmp_eq; /**< Function used to compare keys. 
*/
+   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 key_entry_size; /**< Size of each key entry. */
@@ -187,7 +225,16 @@ rte_hash_find_existing(const char *name)

 void rte_hash_set_cmp_func(struct rte_hash *h, rte_hash_cmp_eq_t func)
 {
-  

[dpdk-dev] [PATCH] mlx5: fix RETA table size

2016-03-30 Thread Yaacov Hazan
Change RETA table size to use 256 entries for better performance.

Fixes: ebb30ec64a68 ("mlx5: increase RETA table size")

Signed-off-by: Yaacov Hazan 
---
 drivers/net/mlx5/mlx5_defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index 195440c..09207d9 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -52,7 +52,7 @@
 #define MLX5_PMD_TX_PER_COMP_REQ 64

 /* RSS Indirection table size. */
-#define RSS_INDIRECTION_TABLE_SIZE 512
+#define RSS_INDIRECTION_TABLE_SIZE 256

 /* Maximum number of Scatter/Gather Elements per Work Request. */
 #ifndef MLX5_PMD_SGE_WR_N
-- 
1.9.1



[dpdk-dev] [PATCH] bonding: fix bond link detect in non-interrupt mode

2016-03-30 Thread Thomas Monjalon
Anyone to review, please?

2016-03-25 17:44, John Daley:
> From: Nelson Escobar 
> 
> Stopping then re-starting a bond interface containing slaves that
> used polling for link detection caused the bond to think all slave
> links were down and inactive.
> 
> Move the start of the polling for link from slave_add() to
> bond_ethdev_start() and in bond_ethdev_stop() make sure we clear
> the last_link_status of the slaves.
> 
> Signed-off-by: Nelson Escobar 
> Signed-off-by: John Daley 
> ---
>  drivers/net/bonding/rte_eth_bond_pmd.c | 27 +--
>  1 file changed, 17 insertions(+), 10 deletions(-)



[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Thomas Monjalon
2016-03-30 14:15, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > 2016-03-30 13:57, Dumitrescu, Cristian:
> > > I think the correct fix is:
> > > #if defined(__x86_64__) && (defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> > || defined(RTE_MACHINE_CPUFLAG_CRC32))
> > >
> > > We'll test it and send a patch asap.
> > 
> > I had prepared this patch. Please be inspired:
> > 
> > examples/ip_pipeline: fix SSE4.2 optimization branch
> > 
> > The branch was disabled because of a typo in the SSE4.2 flag.
> > Change also the x86_64 flag to use a DPDK one.
> > 
> > Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64 without
> > SSE4.2")
> > 
> > -#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> > +#if defined(RTE_ARCH_X86_64) &&
> > defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> 
> Acked-by: Cristian Dumitrescu 

I thought you wanted to send a patch with another CPUFLAG (CRC32)?


[dpdk-dev] [PATCH 4/4] autotest: fix func reentrancy

2016-03-30 Thread Olivier Matz
The previous code in func_reentrancy autotest was doing in parallel
something close to:

  name = "common_name";
  do several times {
  obj = allocate_an_object(name)   // obj = ring, mempool, hash, lpm, ...
  if (obj == NULL && lookup(name) == NULL)
  return TEST_FAIL;
  }

This code is not safe. For instance:

   mempool_create() is called on core 0, it creates a ring. At the same
   time on core 1, mempool_create() is called too and the creation of the
   ring fails (EEXIST). But the mempool lookup can fail on core 1 if
   the mempool is not added in the list by core 0.

This commit fixes the func_reentrancy autotest that now works with all
tested class of objects.

Signed-off-by: Olivier Matz 
---
 app/test/test_func_reentrancy.c | 31 +--
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 5d09296..300a3bc 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -83,6 +83,7 @@ typedef void (*case_clean_t)(unsigned lcore_id);

 #define MAX_LCORES RTE_MAX_MEMZONE / (MAX_ITER_TIMES * 4U)

+static rte_atomic32_t obj_count = RTE_ATOMIC32_INIT(0);
 static rte_atomic32_t synchro = RTE_ATOMIC32_INIT(0);

 #define WAIT_SYNCHRO_FOR_SLAVES()   do{ \
@@ -100,6 +101,7 @@ test_eal_init_once(__attribute__((unused)) void *arg)

WAIT_SYNCHRO_FOR_SLAVES();

+   rte_atomic32_set(_count, 1); /* silent the check in the caller */
if (rte_eal_init(0, NULL) != -1)
return -1;

@@ -122,8 +124,8 @@ ring_create_lookup(__attribute__((unused)) void *arg)
/* create the same ring simultaneously on all threads */
for (i = 0; i < MAX_ITER_TIMES; i++) {
rp = rte_ring_create("fr_test_once", 4096, SOCKET_ID_ANY, 0);
-   if ((NULL == rp) && (rte_ring_lookup("fr_test_once") == NULL))
-   return -1;
+   if (rp != NULL)
+   rte_atomic32_inc(_count);
}

/* create/lookup new ring several times */
@@ -172,8 +174,8 @@ mempool_create_lookup(__attribute__((unused)) void *arg)
NULL, NULL,
my_obj_init, NULL,
SOCKET_ID_ANY, 0);
-   if ((NULL == mp) && (rte_mempool_lookup("fr_test_once") == 
NULL))
-   return -1;
+   if (mp != NULL)
+   rte_atomic32_inc(_count);
}

/* create/lookup new ring several times */
@@ -238,8 +240,8 @@ hash_create_free(__attribute__((unused)) void *arg)
hash_params.name = "fr_test_once";
for (i = 0; i < MAX_ITER_TIMES; i++) {
handle = rte_hash_create(_params);
-   if ((NULL == handle) && (rte_hash_find_existing("fr_test_once") 
== NULL))
-   return -1;
+   if (handle != NULL)
+   rte_atomic32_inc(_count);
}

/* create mutiple times simultaneously */
@@ -306,8 +308,8 @@ fbk_create_free(__attribute__((unused)) void *arg)
fbk_params.name = "fr_test_once";
for (i = 0; i < MAX_ITER_TIMES; i++) {
handle = rte_fbk_hash_create(_params);
-   if ((NULL == handle) && 
(rte_fbk_hash_find_existing("fr_test_once") == NULL))
-   return -1;
+   if (handle != NULL)
+   rte_atomic32_inc(_count);
}

/* create mutiple fbk tables simultaneously */
@@ -372,8 +374,8 @@ lpm_create_free(__attribute__((unused)) void *arg)
/* create the same lpm simultaneously on all threads */
for (i = 0; i < MAX_ITER_TIMES; i++) {
lpm = rte_lpm_create("fr_test_once",  SOCKET_ID_ANY, );
-   if ((NULL == lpm) && (rte_lpm_find_existing("fr_test_once") == 
NULL))
-   return -1;
+   if (lpm != NULL)
+   rte_atomic32_inc(_count);
}

/* create mutiple fbk tables simultaneously */
@@ -432,10 +434,12 @@ launch_test(struct test_case *pt_case)
unsigned lcore_id;
unsigned cores_save = rte_lcore_count();
unsigned cores = RTE_MIN(cores_save, MAX_LCORES);
+   unsigned count;

if (pt_case->func == NULL)
return -1;

+   rte_atomic32_set(_count, 0);
rte_atomic32_set(, 0);

RTE_LCORE_FOREACH_SLAVE(lcore_id) {
@@ -462,6 +466,13 @@ launch_test(struct test_case *pt_case)
pt_case->clean(lcore_id);
}

+   count = rte_atomic32_read(_count);
+   if (count != 1) {
+   printf("%s: common object allocated %d times (should be 1)\n",
+   pt_case->name, count);
+   ret = -1;
+   }
+
return ret;
 }

-- 
2.1.4



[dpdk-dev] [PATCH 3/4] hash: keep the list locked at creation

2016-03-30 Thread Olivier Matz
To avoid a race condition while creating a new hash object, the
list has to be locked before the lookup, and released only once the
new object is added in the list.

Signed-off-by: Olivier Matz 
---
 lib/librte_hash/rte_cuckoo_hash.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c 
b/lib/librte_hash/rte_cuckoo_hash.c
index ccec2db..18351fa 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -228,11 +228,17 @@ rte_hash_create(const struct rte_hash_parameters *params)

snprintf(hash_name, sizeof(hash_name), "HT_%s", params->name);

-   /* Guarantee there's no existing */
-   h = rte_hash_find_existing(params->name);
-   if (h != NULL) {
+   rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
+
+   /* guarantee there's no existing */
+   TAILQ_FOREACH(te, hash_list, next) {
+   h = (struct rte_hash *) te->data;
+   if (strncmp(name, h->name, RTE_HASH_NAMESIZE) == 0)
+   break;
+   }
+   if (te != NULL) {
rte_errno = EEXIST;
-   return NULL;
+   goto err;
}

te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0);
@@ -359,13 +365,13 @@ rte_hash_create(const struct rte_hash_parameters *params)
for (i = 1; i < params->entries + 1; i++)
rte_ring_sp_enqueue(r, (void *)((uintptr_t) i));

-   rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
te->data = (void *) h;
TAILQ_INSERT_TAIL(hash_list, te, next);
rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);

return h;
 err:
+   rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
rte_free(te);
rte_free(h);
rte_free(buckets);
-- 
2.1.4



[dpdk-dev] [PATCH 2/4] hash: allocation of an existing object should fail

2016-03-30 Thread Olivier Matz
Change the API of rte_hash*_create() functions to return NULL and set
rte_errno to EEXIST when the object name already exists.

These functions were returning a pointer to the existing object in that
case, but it is a problem as the caller did not know if the object had
to be freed or not.

Doing this change also makes the hash API more consistent with the other
APIs (mempool, rings, ...).

Signed-off-by: Olivier Matz 
---
 lib/librte_hash/rte_cuckoo_hash.c | 6 --
 lib/librte_hash/rte_fbk_hash.c| 5 -
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c 
b/lib/librte_hash/rte_cuckoo_hash.c
index 71b5b76..ccec2db 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -230,8 +230,10 @@ rte_hash_create(const struct rte_hash_parameters *params)

/* Guarantee there's no existing */
h = rte_hash_find_existing(params->name);
-   if (h != NULL)
-   return h;
+   if (h != NULL) {
+   rte_errno = EEXIST;
+   return NULL;
+   }

te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0);
if (te == NULL) {
diff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c
index 8752a47..ba1e475 100644
--- a/lib/librte_hash/rte_fbk_hash.c
+++ b/lib/librte_hash/rte_fbk_hash.c
@@ -140,8 +140,11 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params 
*params)
if (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0)
break;
}
-   if (te != NULL)
+   if (te != NULL) {
+   ht = NULL;
+   rte_errno = EEXIST;
goto exit;
+   }

te = rte_zmalloc("FBK_HASH_TAILQ_ENTRY", sizeof(*te), 0);
if (te == NULL) {
-- 
2.1.4



[dpdk-dev] [PATCH 1/4] lpm: allocation of an existing object should fail

2016-03-30 Thread Olivier Matz
Change the API of rte_lpm*_create() functions to return NULL and set
rte_errno to EEXIST when the object name already exists.

These functions were returning a pointer to the existing object in that
case, but it is a problem as the caller did not know if the object had
to be freed or not.

Doing this change also makes the lpm API more consistent with the other
APIs (mempool, rings, ...).

Signed-off-by: Olivier Matz 
---
 app/test/test_lpm6.c  |  2 +-
 lib/librte_lpm/rte_lpm.c  | 10 --
 lib/librte_lpm/rte_lpm6.c |  5 -
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/app/test/test_lpm6.c b/app/test/test_lpm6.c
index 1f88d7a..b464342 100644
--- a/app/test/test_lpm6.c
+++ b/app/test/test_lpm6.c
@@ -222,7 +222,7 @@ test1(void)

/* rte_lpm6_create: lpm name == LPM2 */
lpm3 = rte_lpm6_create("LPM1", SOCKET_ID_ANY, );
-   TEST_LPM_ASSERT(lpm3 == lpm1);
+   TEST_LPM_ASSERT(lpm3 == NULL);

rte_lpm6_free(lpm1);
rte_lpm6_free(lpm2);
diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index af5811c..62e74bb 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -209,8 +209,11 @@ rte_lpm_create_v20(const char *name, int socket_id, int 
max_rules,
if (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)
break;
}
-   if (te != NULL)
+   if (te != NULL) {
+   lpm = NULL;
+   rte_errno = EEXIST;
goto exit;
+   }

/* allocate tailq entry */
te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
@@ -280,8 +283,11 @@ rte_lpm_create_v1604(const char *name, int socket_id,
if (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)
break;
}
-   if (te != NULL)
+   if (te != NULL) {
+   lpm = NULL;
+   rte_errno = EEXIST;
goto exit;
+   }

/* allocate tailq entry */
te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index 4c44cd7..9877a30 100644
--- a/lib/librte_lpm/rte_lpm6.c
+++ b/lib/librte_lpm/rte_lpm6.c
@@ -182,8 +182,11 @@ rte_lpm6_create(const char *name, int socket_id,
if (strncmp(name, lpm->name, RTE_LPM6_NAMESIZE) == 0)
break;
}
-   if (te != NULL)
+   if (te != NULL) {
+   lpm = NULL;
+   rte_errno = EEXIST;
goto exit;
+   }

/* allocate tailq entry */
te = rte_zmalloc("LPM6_TAILQ_ENTRY", sizeof(*te), 0);
-- 
2.1.4



[dpdk-dev] [PATCH 0/4] fix lpm and hash creation

2016-03-30 Thread Olivier Matz
Seen while trying to fix the func_reentrancy autotest. The
series addresses several issues:

1/ Hash and lpm return a pointer to an existing object if the user requests the
   creation with an already existing name. This look dangerous: when an object
   is returned, the user does not know if it should be freed or not.

2/ There is a race condition in cuckoo_hash as the lock is not held in
   rte_hash_create(). We could find some cases where NULL is returned when the
   object already exists (ex: when rte_ring_create() fails).

3/ There is a race condition func_reentrancy that can fail even if the tested
   API behaves correctly.


Changes since RFC:

- split the patch in 4 patches
- on error, set rte_errno to EEXIST when relevant
- fix locking in cuckoo_hash creation

Olivier Matz (4):
  lpm: allocation of an existing object should fail
  hash: allocation of an existing object should fail
  hash: keep the list locked at creation
  autotest: fix func reentrancy

 app/test/test_func_reentrancy.c   | 31 +--
 app/test/test_lpm6.c  |  2 +-
 lib/librte_hash/rte_cuckoo_hash.c | 18 +-
 lib/librte_hash/rte_fbk_hash.c|  5 -
 lib/librte_lpm/rte_lpm.c  | 10 --
 lib/librte_lpm/rte_lpm6.c |  5 -
 6 files changed, 51 insertions(+), 20 deletions(-)

-- 
2.1.4



[dpdk-dev] [PATCH v2 0/2] Out of place operations for symmetric crypto

2016-03-30 Thread John Griffin
On 29/03/16 15:14, Fiona Trahe wrote:
> From: Arek Kusztal 
>
> This patch adds out of place operations for qat symmetric crypto PMD,
> i.e. the result of the operation can be written to the destination buffer
> instead of overwriting the source buffer as done in "in-place" operation.
>
> v2:
>   - updated commit message to clarify out-of-place meaning
>
>
> Arek Kusztal (2):
>driver/crypto: out-of-place symmetric operations
>app/test: added test for out-of-place symmetric operations
>
>   app/test/test_cryptodev.c   |  495 
> ++-
>   doc/guides/cryptodevs/qat.rst   |1 -
>   drivers/crypto/qat/qat_crypto.c |   22 +-
>   3 files changed, 504 insertions(+), 14 deletions(-)
>

Acked-by: John Griffin 


[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Dumitrescu, Cristian


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, March 30, 2016 4:50 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Zhang,
> Roy Fan ; Hunt, David 
> Subject: Re: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> 2016-03-30 14:15, Dumitrescu, Cristian:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > 2016-03-30 13:57, Dumitrescu, Cristian:
> > > > I think the correct fix is:
> > > > #if defined(__x86_64__) &&
> (defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> > > || defined(RTE_MACHINE_CPUFLAG_CRC32))
> > > >
> > > > We'll test it and send a patch asap.
> > >
> > > I had prepared this patch. Please be inspired:
> > >
> > > examples/ip_pipeline: fix SSE4.2 optimization branch
> > >
> > > The branch was disabled because of a typo in the SSE4.2 flag.
> > > Change also the x86_64 flag to use a DPDK one.
> > >
> > > Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64
> without
> > > SSE4.2")
> > >
> > > -#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> > > +#if defined(RTE_ARCH_X86_64) &&
> > > defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> >
> > Acked-by: Cristian Dumitrescu 
> 
> I thought you wanted to send a patch with another CPUFLAG (CRC32)?

The extra flag is good, but not absolutely required, as SSE4.2 implies support 
for CRC32 instruction.

The CRC32 flag might be useful when a CPU architecture other than Intel 
supports the CRC32 instruction, but I am not sure whether such CPU architecture 
exists. Anyway, the SSE4.2 || CRC32 pattern is already present in several DPDK 
files, so I was looking to observe it as well.

I thought you considered the CRC32 flag to be redundant and decided to remove 
it on purpose.

I am OK if you want to go ahead with your patch right now, otherwise we can 
send a patch tomorrow?

Thanks,
Cristian



[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Thomas Monjalon
2016-03-30 13:57, Dumitrescu, Cristian:
> I think the correct fix is:
> #if defined(__x86_64__) && (defined(RTE_MACHINE_CPUFLAG_SSE4_2) || 
> defined(RTE_MACHINE_CPUFLAG_CRC32))
> 
> We'll test it and send a patch asap.

I had prepared this patch. Please be inspired:

examples/ip_pipeline: fix SSE4.2 optimization branch

The branch was disabled because of a typo in the SSE4.2 flag.
Change also the x86_64 flag to use a DPDK one.

Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64 without 
SSE4.2")

-#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
+#if defined(RTE_ARCH_X86_64) && defined(RTE_MACHINE_CPUFLAG_SSE4_2)



[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Thomas Monjalon
2016-03-30 13:24, Dumitrescu, Cristian:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > The compiler cannot use _mm_crc32_u64:
> > 
> > examples/ip_pipeline/pipeline/hash_func.h:165:9:
> > error: implicit declaration of function '_mm_crc32_u64' is invalid in C99
> > 
> > Fixes: 947024a26df7 ("examples/ip_pipeline: rework passthrough pipeline")
> > 
> > Signed-off-by: Thomas Monjalon 
[...]
> > -#if defined(__x86_64__)
> > +#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> 
> Hi Thomas,
> 
> This is not the correct fix, as RTE_CPUFLAG_SSE4_2 is a flag that can
> only be tested at run-time (as result of calling function
> rte_cpu_get_flag_enabled()), not at build-time.

Yes you're right. It is an error, the word MACHINE is missing.
The flag should be RTE_MACHINE_CPUFLAG_SSE4_2.

> The reason it appears to fix the build issue you are mentioning is the fact
> that this change results in disabling the  __x86_64__ code branch.
> 
> We need to revert this patch and look for a better option.
> 
> What is the compiler that generates the build issue you are mentioning?
> We could not reproduce it with gcc 5 (gcc 5.3.1).

It fails with gcc-5.2.0 and clang-3.6.2 for machine "default".


[dpdk-dev] [PATCH] scripts: check commit formatting

2016-03-30 Thread Bruce Richardson
On Wed, Mar 30, 2016 at 10:44:14PM +0800, Yuanhan Liu wrote:
> On Wed, Mar 30, 2016 at 03:27:40PM +0100, Bruce Richardson wrote:
> > On Wed, Mar 30, 2016 at 09:46:34AM +0800, Yuanhan Liu wrote:
> > > On Tue, Mar 29, 2016 at 11:29:46PM +0200, Thomas Monjalon wrote:
> > > > The git messages have three parts:
> > > > 1/ the headline
> > > > 2/ the explanations
> > > > 3/ the footer tags
> > > > 
> > > > The headline helps to quickly browse an history or catch instantly the
> > > > purpose of a commit. Making it short with some consistent wording
> > > > allows to easily parse it or match some patterns.
> > > > 
> > > > The explanations must give some keys like the reason of the change.
> > > > Nothing can be automatically checked for this part.
> > > 
> > > Actually, I think we might be able to do 2 tests here:
> > > 
> > > - space line between paragraphs.
> > > 
> > > - lines over 80 chars.
> > 
> > 75 chars for commit messages, and 50 for commit titles :-)
> 
> I'd agree that 75 char is better. My personal preference is actually
> 68 :) 
> 
I just go with what vim uses on my machine, because vim is always right :-)


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Stephen Hemminger
On Wed, 30 Mar 2016 22:16:51 +
Harish Patil  wrote:

> >
> >On Tue, 29 Mar 2016 22:28:20 -0700
> >Rasesh Mody  wrote:
> >
> >> +
> >> +static void qede_print_adapter_info(struct qede_dev *qdev)
> >> +{
> >> +  struct ecore_dev *edev = >edev;
> >> +  struct qed_dev_info *info = >dev_info.common;
> >> +  char ver_str[QED_DRV_VER_STR_SIZE] = { 0 };
> >> +
> >> +  RTE_LOG(INFO, PMD,
> >> +" Chip details : %s%d\n",
> >> +ECORE_IS_BB(edev) ? "BB" : "AH",
> >> +CHIP_REV_IS_A0(edev) ? 0 : 1);
> >> +
> >> +  sprintf(ver_str, "%s %s_%d.%d.%d.%d", QEDE_PMD_VER_PREFIX,
> >> +  edev->ver_str, QEDE_PMD_VERSION_MAJOR, QEDE_PMD_VERSION_MINOR,
> >> +  QEDE_PMD_VERSION_REVISION, QEDE_PMD_VERSION_PATCH);
> >> +  strcpy(qdev->drv_ver, ver_str);
> >> +  RTE_LOG(INFO, PMD, " Driver version : %s\n", ver_str);
> >> +
> >> +  ver_str[0] = '\0';
> >> +  sprintf(ver_str, "%d.%d.%d.%d", info->fw_major, info->fw_minor,
> >> +  info->fw_rev, info->fw_eng);
> >> +  RTE_LOG(INFO, PMD, " Firmware version : %s\n", ver_str);
> >> +
> >> +  ver_str[0] = '\0';
> >> +  sprintf(ver_str, "%d.%d.%d.%d",
> >> +  (info->mfw_rev >> 24) & 0xff,
> >> +  (info->mfw_rev >> 16) & 0xff,
> >> +  (info->mfw_rev >> 8) & 0xff, (info->mfw_rev) & 0xff);
> >> +  RTE_LOG(INFO, PMD, " Management firmware version : %s\n", ver_str);
> >> +
> >> +  RTE_LOG(INFO, PMD, " Firmware file : %s\n", QEDE_FW_FILE_NAME);
> >
> >This means the driver is far too chatty in the logs.
> >Can't this be made DEBUG level?
> >
> Not clear what is the issue here?
> RTE_LOG is used here to display basic adapter info like firmware/driver
> versions etc without the need to enable any debug flags.
> The driver debug logging is under the control of appropriate debug flags.
> 

The DPDK log messages end up being examined by tech support and customers.
Too much code puts extra stuff in so it is hard to find the real problem.
This is obviously debug info, so either:
  1) make it conditionally compiled
  2) change log level to DEBUG
  3) remove it.


[dpdk-dev] [PATCH] scripts: check commit formatting

2016-03-30 Thread Bruce Richardson
On Wed, Mar 30, 2016 at 09:46:34AM +0800, Yuanhan Liu wrote:
> On Tue, Mar 29, 2016 at 11:29:46PM +0200, Thomas Monjalon wrote:
> > The git messages have three parts:
> > 1/ the headline
> > 2/ the explanations
> > 3/ the footer tags
> > 
> > The headline helps to quickly browse an history or catch instantly the
> > purpose of a commit. Making it short with some consistent wording
> > allows to easily parse it or match some patterns.
> > 
> > The explanations must give some keys like the reason of the change.
> > Nothing can be automatically checked for this part.
> 
> Actually, I think we might be able to do 2 tests here:
> 
> - space line between paragraphs.
> 
> - lines over 80 chars.

75 chars for commit messages, and 50 for commit titles :-)

/Bruce
> 
>   --yliu


[dpdk-dev] librte_pmd_ixgbe implementation of ixgbe_dev_rx_queue_count

2016-03-30 Thread Bruce Richardson
On Tue, Mar 29, 2016 at 09:54:18AM -0700, Stephen Hemminger wrote:
> On Tue, 29 Mar 2016 10:31:19 +0100
> Bruce Richardson  wrote:
> 
> > On Mon, Mar 28, 2016 at 06:45:26PM -0700, Mohammad El-Shabani wrote:
> > > Hi,
> > > Looking into why it hurts performance, I see that ixgbe_dev_rx_queue_count
> > > is implemented a scan of elements of rx descriptors, which is very
> > > expensive. I am wondering why its implemented the way it is. Could it not
> > > just read the head location from the driver?
> > > 
> > > Thanks!
> > > Mohammad El-Shabani
> > 
> > It's likely that reading the head location from the driver will be even 
> > slower
> > than scanning the descriptor rings in memory. Access to PCI is very much 
> > slower
> > than accessing memory - especially since on platforms with DDIO, many memory
> > accesses will actually be cache reads.
> > 
> > That being said, I haven't actually written a test to prove this out, so 
> > feel
> > free to try out the head pointer read method instead and see if it improves
> > things. The results may vary depending on how far ahead needs to be scanned,
> > but certainly for the empty ring case, the descriptor scan method will be 
> > far
> > faster than a head read.
> > 
> > Regards,
> > /Bruce
> 
> Also the most common use case is "is there any more packets ready before
> I go to sleep on epoll", and the descriptor done API tells more than
> is needed.

Yes, it's not designed for that case. For the are-there-any-more-packets query,
the rx_burst api is the one to call. :-)
The rx_queue_count API is for the case where you are under load and need to see
beyond the max count returned by rx_burst before you process the burst of 
packets.

/Bruce


[dpdk-dev] [PATCH] Fix KNI compilation under Wind River Linux 6.0 recent RCPLs.

2016-03-30 Thread Stephen Hemminger
On Wed, 30 Mar 2016 12:13:35 -0600
Lee Roberts  wrote:

> skb_set_hash() has been backported to recent Wind River Linux 6.0 RCPLs.
> As a result, the corresponding stanza in kcompat.h must be removed.
> Similar patches have already been applied for RHEL, SLES and Ubuntu.
> 
> Wind River Linux does not provide convenient macros for kernel version
> identification.  Add macros to Makefile to identify the Wind River Linux
> version.
> 
> Signed-off-by: Lee Roberts 
> ---
>  lib/librte_eal/linuxapp/kni/Makefile  |  8 
>  lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h | 13 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/Makefile 
> b/lib/librte_eal/linuxapp/kni/Makefile
> index ac99d3f..6310615 100644
> --- a/lib/librte_eal/linuxapp/kni/Makefile
> +++ b/lib/librte_eal/linuxapp/kni/Makefile
> @@ -51,6 +51,14 @@ UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE 
> $(RTE_KERNELDIR)/include/ge
>  MODULE_CFLAGS += 
> -D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_CODE))"
>  endif
>  
> +ifeq ($(shell lsb_release -si 2>/dev/null),wrlinux)
> +WRLINUX_MAJOR := $(shell lsb_release -sr | cut -d. -f1)
> +WRLINUX_MINOR := $(shell lsb_release -sr | cut -d. -f2)
> +WRLINUX_RCPL  := $(shell lsb_release -sr | cut -d. -f4)
> +MODULE_CFLAGS += 
> -D"WRLINUX_RELEASE_CODE=WRLINUX_RELEASE_VERSION($(WRLINUX_MAJOR),$(WRLINUX_MINOR))"
> +MODULE_CFLAGS += -D"WRLINUX_RCPL=$(WRLINUX_RCPL)"
> +endif
> +

Do we want to require DPDK to work in the face of every weird vendor
kernel backport. This is a road to nowhere...

One more reason to get kernel drivers upstream.


[dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

2016-03-30 Thread Stephen Hemminger
On Wed, 30 Mar 2016 19:12:39 +0200
Thomas Monjalon  wrote:

> 2016-03-30 09:36, Stephen Hemminger:
> > On Wed, 30 Mar 2016 10:06:36 -0400
> > Aaron Conole  wrote:
> > > --- a/drivers/net/e1000/Makefile
> > > +++ b/drivers/net/e1000/Makefile
> > > @@ -54,6 +54,9 @@ else
> > >  #
> > >  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
> > >  CFLAGS_BASE_DRIVER += -Wno-unused-variable
> > > +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> > > +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> > > +endif
> > >  endif
> > 
> > NAK, don't do it to the whole file.
> > Fix the code (best option)
> > or use a pragma for the small area which is broken for other reasons.
> 
> Stephen, your solutions do not work because Aaron has not been allowed
> to modify this file.
> As long as we are not allowed to modify the Intel base drivers,
> I don't see any problem to hide some warnings in them.
> The warnings could help us to clean the code or fix some bugs but
> we are not allowed to...
> It is the responsibility of the driver maintainer to keep this nasty code.

ok, but the policy of "base drivers are allowed to be unmaintainable" is
an albatross around the neck of DPDK. There is a reason such a policy was 
rejected
in Linux.


[dpdk-dev] [PATCH 1/4] lpm: allocation of an existing object should fail

2016-03-30 Thread Stephen Hemminger
On Wed, 30 Mar 2016 17:30:24 +0200
Olivier Matz  wrote:

> diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
> index 4c44cd7..9877a30 100644
> --- a/lib/librte_lpm/rte_lpm6.c
> +++ b/lib/librte_lpm/rte_lpm6.c
> @@ -182,8 +182,11 @@ rte_lpm6_create(const char *name, int socket_id,
>   if (strncmp(name, lpm->name, RTE_LPM6_NAMESIZE) == 0)
>   break;
>   }
> - if (te != NULL)
> + if (te != NULL) {
> + lpm = NULL;
> + rte_errno = EEXIST;
>   goto exit;
> + }
>  
>   /* allocate tailq entry */
>  

with older memzone model, objects in huge memory area were never freed.
That means when application restarts it finds the old LPM and works.
With your change it would break such an application.


[dpdk-dev] [PATCH] vmxnet3: remove asserts that confuse coverity

2016-03-30 Thread Stephen Hemminger
These asserts are only for debugging and never fired during
any testing, but they confuse coverity's null tracking.

Signed-off-by: Stephen Hemminger 
---
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c 
b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index 66b0eed..f72156a 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -710,7 +710,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf 
**rx_pkts, uint16_t nb_pkts)
 * the last mbuf of the current packet.
 */
if (rcd->sop) {
-   VMXNET3_ASSERT(rxq->start_seg != NULL);
VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_HEAD);

if (unlikely(rcd->len == 0)) {
@@ -729,7 +728,6 @@ vmxnet3_recv_pkts(void *rx_queue, struct rte_mbuf 
**rx_pkts, uint16_t nb_pkts)
struct rte_mbuf *start = rxq->start_seg;

VMXNET3_ASSERT(rxd->btype == VMXNET3_RXD_BTYPE_BODY);
-   VMXNET3_ASSERT(start != NULL);

start->pkt_len += rxm->data_len;
start->nb_segs++;
-- 
2.1.4



[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Dumitrescu, Cristian


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, March 30, 2016 3:07 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Singh, Jasvinder ; Zhang,
> Roy Fan ; Hunt, David 
> Subject: Re: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> 2016-03-30 13:57, Dumitrescu, Cristian:
> > I think the correct fix is:
> > #if defined(__x86_64__) && (defined(RTE_MACHINE_CPUFLAG_SSE4_2)
> || defined(RTE_MACHINE_CPUFLAG_CRC32))
> >
> > We'll test it and send a patch asap.
> 
> I had prepared this patch. Please be inspired:
> 
> examples/ip_pipeline: fix SSE4.2 optimization branch
> 
> The branch was disabled because of a typo in the SSE4.2 flag.
> Change also the x86_64 flag to use a DPDK one.
> 
> Fixes: 28377375c6c0 ("examples/ip_pipeline: fix build for x86_64 without
> SSE4.2")
> 
> -#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> +#if defined(RTE_ARCH_X86_64) &&
> defined(RTE_MACHINE_CPUFLAG_SSE4_2)

Acked-by: Cristian Dumitrescu 



[dpdk-dev] [PATCH] librte_ether: fix comments for filters

2016-03-30 Thread Thomas Monjalon
2016-03-29 11:04, Jingjing Wu:
> This patch fixes comments for tunnel filters and flow director flows.
> e.g. states fields which are in big endian.
> 
> Fixes: 7b1312891b69 (ethdev: add IP in GRE tunnel)
> Fixes: d69be32d4d78 (ethdev: structures to add or delete flow director)
> Signed-off-by: Jingjing Wu 

Applied, thanks


[dpdk-dev] [PATCH 7/7] l2fwd-crypto: extend crypto information

2016-03-30 Thread Pablo de Lara
Display extra crypto information (algorithms, keys/IV/AAD used, chain...),
so user can know exactly what operations are being carried out.

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c | 84 ++--
 1 file changed, 82 insertions(+), 2 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index 6de29c5..9c95392 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 

 enum cdev_type {
CDEV_TYPE_ANY,
@@ -634,8 +635,6 @@ l2fwd_main_loop(struct l2fwd_crypto_options *options)

RTE_LOG(INFO, L2FWD, "entering main loop on lcore %u\n", lcore_id);

-   l2fwd_crypto_options_print(options);
-
for (i = 0; i < qconf->nb_rx_ports; i++) {

portid = qconf->rx_port_list[i];
@@ -708,6 +707,14 @@ l2fwd_main_loop(struct l2fwd_crypto_options *options)
port_cparams[i].dev_id);
}

+   l2fwd_crypto_options_print(options);
+
+   /*
+* Initialize previous tsc timestamp before the loop,
+* to avoid showing the port statistics immediately,
+* so user can see the crypto information.
+*/
+   prev_tsc = rte_rdtsc();
while (1) {

cur_tsc = rte_rdtsc();
@@ -1213,8 +1220,45 @@ l2fwd_crypto_default_options(struct l2fwd_crypto_options 
*options)
 }

 static void
+display_cipher_info(struct l2fwd_crypto_options *options)
+{
+   printf("\n Cipher information ---\n");
+   printf("Algorithm: %s\n",
+   supported_cipher_algo[options->cipher_xform.cipher.algo]);
+   rte_hexdump(stdout, "Cipher key:",
+   options->cipher_xform.cipher.key.data,
+   options->cipher_xform.cipher.key.length);
+   rte_hexdump(stdout, "IV:", options->iv.data, options->iv.length);
+}
+
+static void
+display_auth_info(struct l2fwd_crypto_options *options)
+{
+   printf("\n Authentication information ---\n");
+   printf("Algorithm: %s\n",
+   supported_auth_algo[options->auth_xform.auth.algo]);
+   rte_hexdump(stdout, "Auth key:",
+   options->auth_xform.auth.key.data,
+   options->auth_xform.auth.key.length);
+   rte_hexdump(stdout, "AAD:", options->aad.data, options->aad.length);
+}
+
+static void
 l2fwd_crypto_options_print(struct l2fwd_crypto_options *options)
 {
+   char string_cipher_op[MAX_STR_LEN];
+   char string_auth_op[MAX_STR_LEN];
+
+   if (options->cipher_xform.cipher.op == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
+   strcpy(string_cipher_op, "Encrypt");
+   else
+   strcpy(string_cipher_op, "Decrypt");
+
+   if (options->auth_xform.auth.op == RTE_CRYPTO_AUTH_OP_GENERATE)
+   strcpy(string_auth_op, "Auth generate");
+   else
+   strcpy(string_auth_op, "Auth verify");
+
printf("Options:-\nn");
printf("portmask: %x\n", options->portmask);
printf("ports per lcore: %u\n", options->nb_ports_per_lcore);
@@ -1226,6 +1270,42 @@ l2fwd_crypto_options_print(struct l2fwd_crypto_options 
*options)

printf("sessionless crypto: %s\n",
options->sessionless ? "enabled" : "disabled");
+
+   if (options->ckey_param && (options->ckey_random_size != -1))
+   printf("Cipher key already parsed, ignoring size of random 
key\n");
+
+   if (options->akey_param && (options->akey_random_size != -1))
+   printf("Auth key already parsed, ignoring size of random 
key\n");
+
+   if (options->iv_param && (options->iv_random_size != -1))
+   printf("IV already parsed, ignoring size of random IV\n");
+
+   if (options->aad_param && (options->aad_random_size != -1))
+   printf("AAD already parsed, ignoring size of random AAD\n");
+
+   printf("\nCrypto chain: ");
+   switch (options->xform_chain) {
+   case L2FWD_CRYPTO_CIPHER_HASH:
+   printf("Input --> %s --> %s --> Output\n",
+   string_cipher_op, string_auth_op);
+   display_cipher_info(options);
+   display_auth_info(options);
+   break;
+   case L2FWD_CRYPTO_HASH_CIPHER:
+   printf("Input --> %s --> %s --> Output\n",
+   string_auth_op, string_cipher_op);
+   display_cipher_info(options);
+   display_auth_info(options);
+   break;
+   case L2FWD_CRYPTO_HASH_ONLY:
+   printf("Input --> %s --> Output\n", string_auth_op);
+   display_auth_info(options);
+   break;
+   case L2FWD_CRYPTO_CIPHER_ONLY:
+   printf("Input --> %s --> Output\n", string_cipher_op);
+   display_cipher_info(options);
+   break;
+   }
 }

 /* Parse the argument given in the command line of the 

[dpdk-dev] [PATCH 6/7] l2fwd-crypto: use key-value list of supported algorithms

2016-03-30 Thread Pablo de Lara
In order to ease the parsing and display of supported algorithms
in the application, two new arrays are created, which contains
the strings of the different cipher and authentication algorithms,

These lists are used to parse the algorithms from the command line,
and will be used to display crypto information to the user.

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c  | 106 +-
 lib/librte_cryptodev/rte_crypto_sym.h |   6 +-
 2 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index c561270..6de29c5 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -133,6 +133,9 @@ struct l2fwd_key {
phys_addr_t phys_addr;
 };

+char supported_auth_algo[RTE_CRYPTO_AUTH_LIST_END][MAX_STR_LEN];
+char supported_cipher_algo[RTE_CRYPTO_CIPHER_LIST_END][MAX_STR_LEN];
+
 /** l2fwd crypto application command line options */
 struct l2fwd_crypto_options {
unsigned portmask;
@@ -164,8 +167,6 @@ struct l2fwd_crypto_options {
int digest_size;

uint16_t block_size;
-   char string_auth_algo[MAX_STR_LEN];
-   char string_cipher_algo[MAX_STR_LEN];
char string_type[MAX_STR_LEN];
 };

@@ -328,6 +329,32 @@ print_stats(void)
printf("\n\n");
 }

+static void
+fill_supported_algorithm_tables(void)
+{
+   unsigned i;
+
+   for (i = 0; i < RTE_CRYPTO_AUTH_LIST_END; i++)
+   strcpy(supported_auth_algo[i], "NOT_SUPPORTED");
+
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_AES_GCM], "AES_GCM");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_MD5_HMAC], "MD5_HMAC");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_NULL], "NULL");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_SHA1_HMAC], "SHA1_HMAC");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_SHA224_HMAC], "SHA224_HMAC");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_SHA256_HMAC], "SHA256_HMAC");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_SHA384_HMAC], "SHA384_HMAC");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_SHA512_HMAC], "SHA512_HMAC");
+   strcpy(supported_auth_algo[RTE_CRYPTO_AUTH_SNOW3G_UIA2], "SNOW3G_UIA2");
+
+   for (i = 0; i < RTE_CRYPTO_CIPHER_LIST_END; i++)
+   strcpy(supported_cipher_algo[i], "NOT_SUPPORTED");
+
+   strcpy(supported_cipher_algo[RTE_CRYPTO_CIPHER_AES_CBC], "AES_CBC");
+   strcpy(supported_cipher_algo[RTE_CRYPTO_CIPHER_AES_GCM], "AES_GCM");
+   strcpy(supported_cipher_algo[RTE_CRYPTO_CIPHER_NULL], "NULL");
+   strcpy(supported_cipher_algo[RTE_CRYPTO_CIPHER_SNOW3G_UEA2], 
"SNOW3G_UEA2");
+}


 static int
@@ -864,18 +891,13 @@ parse_crypto_opt_chain(struct l2fwd_crypto_options 
*options, char *optarg)
 static int
 parse_cipher_algo(enum rte_crypto_cipher_algorithm *algo, char *optarg)
 {
-   if (strcmp("AES_CBC", optarg) == 0) {
-   *algo = RTE_CRYPTO_CIPHER_AES_CBC;
-   return 0;
-   } else if (strcmp("AES_GCM", optarg) == 0) {
-   *algo = RTE_CRYPTO_CIPHER_AES_GCM;
-   return 0;
-   } else if (strcmp("NULL", optarg) == 0) {
-   *algo = RTE_CRYPTO_CIPHER_NULL;
-   return 0;
-   } else if (strcmp("SNOW3G_UEA2", optarg) == 0) {
-   *algo = RTE_CRYPTO_CIPHER_SNOW3G_UEA2;
-   return 0;
+   unsigned i;
+
+   for (i = 0; i < RTE_CRYPTO_CIPHER_LIST_END; i++) {
+   if (!strcmp(supported_cipher_algo[i], optarg)) {
+   *algo = i;
+   return 0;
+   }
}

printf("Cipher algorithm  not supported!\n");
@@ -945,33 +967,13 @@ parse_size(int *size, const char *q_arg)
 static int
 parse_auth_algo(enum rte_crypto_auth_algorithm *algo, char *optarg)
 {
-   if (strcmp("AES_GCM", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_AES_GCM;
-   return 0;
-   } else if (strcmp("MD5_HMAC", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_MD5_HMAC;
-   return 0;
-   } else if (strcmp("NULL", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_NULL;
-   return 0;
-   } else if (strcmp("SHA1_HMAC", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_SHA1_HMAC;
-   return 0;
-   } else if (strcmp("SHA224_HMAC", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_SHA224_HMAC;
-   return 0;
-   } else if (strcmp("SHA256_HMAC", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_SHA256_HMAC;
-   return 0;
-   }  else if (strcmp("SHA384_HMAC", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_SHA384_HMAC;
-   return 0;
-   } else if (strcmp("SHA512_HMAC", optarg) == 0) {
-   *algo = RTE_CRYPTO_AUTH_SHA512_HMAC;
-   return 0;
-   } else if (strcmp("SNOW3G_UIA2", optarg) == 0) 

[dpdk-dev] [PATCH 5/7] l2fwd-crypto: fix ambiguous input key size

2016-03-30 Thread Pablo de Lara
Some crypto algorithms support more than one key size
(including cipher key, authentication key, IV and AAD),
but the app was using always the minimum size.

These changes allows the user to use an specific size,
either from the string provided with cipher_key, auth_key, iv and ADD 
parameters,
or from the values provided with cipher_key_random_size,
auth_key_random_size, iv_random_size and aad_random_size.

This also allows the user to specify the digest size.

Fixes: 1df9c0109f4c ("examples/l2fwd-crypto: parse key parameters")

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c | 257 +--
 1 file changed, 247 insertions(+), 10 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index c323b55..c561270 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -147,15 +147,21 @@ struct l2fwd_crypto_options {

struct rte_crypto_sym_xform cipher_xform;
unsigned ckey_param;
+   int ckey_random_size;

struct l2fwd_key iv;
unsigned iv_param;
+   int iv_random_size;

struct rte_crypto_sym_xform auth_xform;
uint8_t akey_param;
+   int akey_random_size;

struct l2fwd_key aad;
unsigned aad_param;
+   int aad_random_size;
+
+   int digest_size;

uint16_t block_size;
char string_auth_algo[MAX_STR_LEN];
@@ -799,12 +805,17 @@ l2fwd_crypto_usage(const char *prgname)
"  --cipher_algo ALGO\n"
"  --cipher_op ENCRYPT / DECRYPT\n"
"  --cipher_key KEY\n"
+   "  --cipher_key_random_size SIZE: size of cipher key when 
generated randomly\n"
"  --iv IV\n"
+   "  --iv_random_size SIZE: size of IV when generated randomly\n"

"  --auth_algo ALGO\n"
"  --auth_op GENERATE / VERIFY\n"
"  --auth_key KEY\n"
+   "  --auth_key_random_size SIZE: size of auth key when generated 
randomly\n"
"  --aad AAD\n"
+   "  --aad_random_size SIZE: size of AAD when generated 
randomly\n"
+   "  --digest_size SIZE: size of digest to be 
generated/verified\n"

"  --sessionless\n",
   prgname);
@@ -906,6 +917,27 @@ parse_key(uint8_t *data, char *input_arg)
data[byte_count++] = (uint8_t)number;
}

+   return byte_count;
+}
+
+/** Parse size param*/
+static int
+parse_size(int *size, const char *q_arg)
+{
+   char *end = NULL;
+   unsigned long n;
+
+   /* parse hexadecimal string */
+   n = strtoul(q_arg, , 10);
+   if ((q_arg[0] == '\0') || (end == NULL) || (*end != '\0'))
+   n = 0;
+
+   if (n == 0) {
+   printf("invalid size\n");
+   return -1;
+   }
+
+   *size = n;
return 0;
 }

@@ -993,14 +1025,30 @@ l2fwd_crypto_parse_args_long_options(struct 
l2fwd_crypto_options *options,

else if (strcmp(lgopts[option_index].name, "cipher_key") == 0) {
options->ckey_param = 1;
-   return parse_key(options->cipher_xform.cipher.key.data, optarg);
+   options->cipher_xform.cipher.key.length =
+   parse_key(options->cipher_xform.cipher.key.data, 
optarg);
+   if (options->cipher_xform.cipher.key.length > 0)
+   return 0;
+   else
+   return -1;
}

+   else if (strcmp(lgopts[option_index].name, "cipher_key_random_size") == 
0)
+   return parse_size(>ckey_random_size, optarg);
+
else if (strcmp(lgopts[option_index].name, "iv") == 0) {
options->iv_param = 1;
-   return parse_key(options->iv.data, optarg);
+   options->iv.length =
+   parse_key(options->iv.data, optarg);
+   if (options->iv.length > 0)
+   return 0;
+   else
+   return -1;
}

+   else if (strcmp(lgopts[option_index].name, "iv_random_size") == 0)
+   return parse_size(>iv_random_size, optarg);
+
/* Authentication options */
else if (strcmp(lgopts[option_index].name, "auth_algo") == 0) {
retval = parse_auth_algo(>auth_xform.auth.algo,
@@ -1016,12 +1064,34 @@ l2fwd_crypto_parse_args_long_options(struct 
l2fwd_crypto_options *options,

else if (strcmp(lgopts[option_index].name, "auth_key") == 0) {
options->akey_param = 1;
-   return parse_key(options->auth_xform.auth.key.data, optarg);
+   options->auth_xform.auth.key.length =
+   parse_key(options->auth_xform.auth.key.data, optarg);
+   if (options->auth_xform.auth.key.length > 0)
+   return 0;
+   else
+   return -1;
+   }
+
+   else if 

[dpdk-dev] [PATCH 4/7] l2fwd-crypto: fix length of random IV/AAD

2016-03-30 Thread Pablo de Lara
App was generating a random IV/AAD of only 4 bytes,
instead of the actual length, since it was using sizeof(length).

Fixes: 27cf2d1b18e1 ("examples/l2fwd-crypto: discover capabilities")

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index 1b0c229..c323b55 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -643,7 +643,7 @@ l2fwd_main_loop(struct l2fwd_crypto_options *options)
port_cparams[i].aad.phys_addr = 
options->aad.phys_addr;
if (!options->aad_param)

generate_random_key(port_cparams[i].aad.data,
-   
sizeof(port_cparams[i].aad.length));
+   port_cparams[i].aad.length);

}

@@ -661,7 +661,7 @@ l2fwd_main_loop(struct l2fwd_crypto_options *options)
port_cparams[i].iv.phys_addr = options->iv.phys_addr;
if (!options->iv_param)
generate_random_key(port_cparams[i].iv.data,
-   
sizeof(port_cparams[i].iv.length));
+   port_cparams[i].iv.length);

port_cparams[i].cipher_algo = 
options->cipher_xform.cipher.algo;
}
-- 
2.5.5



[dpdk-dev] [PATCH 3/7] l2fwd-crypto: add missing string initialization

2016-03-30 Thread Pablo de Lara
When passing the preferred crypto device type in the command line parameters,
the string (HW/SW/ANY) was not being saved, which is used
for error information to the user.

Fixes: 27cf2d1b18e1 ("examples/l2fwd-crypto: discover capabilities")

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index fd30826..1b0c229 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -968,8 +968,12 @@ l2fwd_crypto_parse_args_long_options(struct 
l2fwd_crypto_options *options,
 {
int retval;

-   if (strcmp(lgopts[option_index].name, "cdev_type") == 0)
-   return parse_cryptodev_type(>type, optarg);
+   if (strcmp(lgopts[option_index].name, "cdev_type") == 0) {
+   retval = parse_cryptodev_type(>type, optarg);
+   if (retval == 0)
+   strcpy(options->string_type, optarg);
+   return retval;
+   }

else if (strcmp(lgopts[option_index].name, "chain") == 0)
return parse_crypto_opt_chain(options, optarg);
-- 
2.5.5



[dpdk-dev] [PATCH 2/7] l2fwd-crypto: rename period parameter

2016-03-30 Thread Pablo de Lara
L2fwd-crypto app is based on L2fwd app and it inherits
some of its parameters (such as portmask, queues per core...).

The parameter period (period of time between statistic updates)
is -T in L2fwd, but was -t in L2fwd-crypto, so for consistency,
it is changed back to -T

Fixes: 387259bd6c67 ("examples/l2fwd-crypto: add sample application)

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index 0cb46c2..fd30826 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -790,7 +790,7 @@ l2fwd_crypto_usage(const char *prgname)
"  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
"  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
"  -s manage all ports from single lcore\n"
-   "  -t PERIOD: statistics will be refreshed each PERIOD seconds"
+   "  -T PERIOD: statistics will be refreshed each PERIOD seconds"
" (0 to disable, 10 default, 86400 maximum)\n"

"  --cdev_type HW / SW / ANY\n"
@@ -1220,7 +1220,7 @@ l2fwd_crypto_parse_args(struct l2fwd_crypto_options 
*options,
break;

/* timer period */
-   case 't':
+   case 'T':
retval = l2fwd_crypto_parse_timer_period(options,
optarg);
if (retval < 0) {
-- 
2.5.5



[dpdk-dev] [PATCH 1/7] l2fwd-crypto: add missing new line character in help

2016-03-30 Thread Pablo de Lara
Fixes: 387259bd6c67 ("examples/l2fwd-crypto: add sample application)

Signed-off-by: Pablo de Lara 
---
 examples/l2fwd-crypto/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c
index 5fd4ff1..0cb46c2 100644
--- a/examples/l2fwd-crypto/main.c
+++ b/examples/l2fwd-crypto/main.c
@@ -789,7 +789,7 @@ l2fwd_crypto_usage(const char *prgname)
printf("%s [EAL options] --\n"
"  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
"  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
-   "  -s manage all ports from single lcore"
+   "  -s manage all ports from single lcore\n"
"  -t PERIOD: statistics will be refreshed each PERIOD seconds"
" (0 to disable, 10 default, 86400 maximum)\n"

-- 
2.5.5



[dpdk-dev] [PATCH 0/7] L2fwd-crypto fixes/enhancements

2016-03-30 Thread Pablo de Lara
This patches fixes some small issues in L2fwd-crypto
app and also improves the app, making it more flexible
(accepting different key sizes)
and readable (information display improvement).

Pablo de Lara (7):
  l2fwd-crypto: add missing new line character in help
  l2fwd-crypto: rename period parameter
  l2fwd-crypto: add missing string initialization
  l2fwd-crypto: fix length of random IV/AAD
  l2fwd-crypto: fix ambiguous input key size
  l2fwd-crypto: use key-value list of supported algorithms
  l2fwd-crypto: extend crypto information

 examples/l2fwd-crypto/main.c  | 465 --
 lib/librte_cryptodev/rte_crypto_sym.h |   6 +-
 2 files changed, 398 insertions(+), 73 deletions(-)

-- 
2.5.5



[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Dumitrescu, Cristian


> -Original Message-
> From: Dumitrescu, Cristian
> Sent: Wednesday, March 30, 2016 2:24 PM
> To: 'Thomas Monjalon' ; dev at dpdk.org
> Cc: Singh, Jasvinder ; Zhang, Roy Fan
> ; Hunt, David 
> Subject: RE: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> Importance: High
> 
> 
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Tuesday, February 16, 2016 6:46 AM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> > x86_64 without SSE4.2
> >
> > The compiler cannot use _mm_crc32_u64:
> >
> > examples/ip_pipeline/pipeline/hash_func.h:165:9:
> > error: implicit declaration of function '_mm_crc32_u64' is invalid in C99
> >
> > Fixes: 947024a26df7 ("examples/ip_pipeline: rework passthrough
> pipeline")
> >
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  examples/ip_pipeline/pipeline/hash_func.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/examples/ip_pipeline/pipeline/hash_func.h
> > b/examples/ip_pipeline/pipeline/hash_func.h
> > index 7846300..1953ad4 100644
> > --- a/examples/ip_pipeline/pipeline/hash_func.h
> > +++ b/examples/ip_pipeline/pipeline/hash_func.h
> > @@ -152,7 +152,7 @@ hash_xor_key64(void *key, __rte_unused uint32_t
> > key_size, uint64_t seed)
> > return (xor0 >> 32) ^ xor0;
> >  }
> >
> > -#if defined(__x86_64__)
> > +#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> >
> >  #include 
> >
> > --
> > 2.7.0
> 
> Hi Thomas,
> 
> This is not the correct fix, as RTE_CPUFLAG_SSE4_2 is a flag that can only be
> tested at run-time (as result of calling function rte_cpu_get_flag_enabled()),
> not at build-time.
> 
> The reason it appears to fix the build issue you are mentioning is the fact 
> that
> this change results in disabling the  __x86_64__ code branch.
> 
> We need to revert this patch and look for a better option.
> 
> What is the compiler that generates the build issue you are mentioning? We
> could not reproduce it with gcc 5 (gcc 5.3.1).
> 
> Thanks,
> Cristian

I think the correct fix is:
#if defined(__x86_64__) && (defined(RTE_MACHINE_CPUFLAG_SSE4_2) || 
defined(RTE_MACHINE_CPUFLAG_CRC32))

We'll test it and send a patch asap.

Thanks,
Cristian



[dpdk-dev] [PATCH v2] drivers/crypto: Fix anonymous union initialization in crypto PMDs

2016-03-30 Thread Fiona Trahe

In SUSE11-SP3 i686 platform, with gcc 4.5.1, there are compile issues, e.g:
  null_crypto_pmd_ops.c:44:3: error:
  unknown field 'sym' specified in initializer
  cc1: warnings being treated as errors

The member in anonymous union initialization should be inside '{}',
otherwise it will report an error.

Fixes: 26c2e4ad5ad4 ("cryptodev: add capabilities discovery")

v2:
 - extends fix to cover all crypto pmds.

Signed-off-by: Michael Qiu 
Signed-off-by: Fiona Trahe 
---
 drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c   | 16 +++---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 64 +++---
 drivers/crypto/null/null_crypto_pmd_ops.c  | 16 +++---
 drivers/crypto/qat/qat_crypto.c| 74 +-
 drivers/crypto/snow3g/rte_snow3g_pmd_ops.c | 16 +++---
 5 files changed, 93 insertions(+), 93 deletions(-)

diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c 
b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
index 387f8d1..4dec8dd 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c
@@ -41,9 +41,9 @@
 static const struct rte_cryptodev_capabilities aesni_gcm_pmd_capabilities[] = {
{   /* AES GCM (AUTH) */
.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
-   .sym = {
+   {.sym = {
.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
-   .auth = {
+   {.auth = {
.algo = RTE_CRYPTO_AUTH_AES_GCM,
.block_size = 16,
.key_size = {
@@ -61,14 +61,14 @@ static const struct rte_cryptodev_capabilities 
aesni_gcm_pmd_capabilities[] = {
.max = 12,
.increment = 4
}
-   }
-   }
+   }, }
+   }, }
},
{   /* AES GCM (CIPHER) */
.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
-   .sym = {
+   {.sym = {
.xform_type = RTE_CRYPTO_SYM_XFORM_CIPHER,
-   .cipher = {
+   {.cipher = {
.algo = RTE_CRYPTO_CIPHER_AES_GCM,
.block_size = 16,
.key_size = {
@@ -81,8 +81,8 @@ static const struct rte_cryptodev_capabilities 
aesni_gcm_pmd_capabilities[] = {
.max = 16,
.increment = 0
}
-   }
-   }
+   }, }
+   }, }
},
RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
 };
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c 
b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index 5a439e6..3806a66 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -42,9 +42,9 @@
 static const struct rte_cryptodev_capabilities aesni_mb_pmd_capabilities[] = {
{   /* MD5 HMAC */
.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
-   .sym = {
+   {.sym = {
.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
-   .auth = {
+   {.auth = {
.algo = RTE_CRYPTO_AUTH_MD5_HMAC,
.block_size = 64,
.key_size = {
@@ -58,14 +58,14 @@ static const struct rte_cryptodev_capabilities 
aesni_mb_pmd_capabilities[] = {
.increment = 0
},
.aad_size = { 0 }
-   }
-   }
+   }, }
+   }, }
},
{   /* SHA1 HMAC */
.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
-   .sym = {
+   {.sym = {
.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
-   .auth = {
+   {.auth = {
.algo = RTE_CRYPTO_AUTH_SHA1_HMAC,
.block_size = 64,
.key_size = {
@@ -79,14 +79,14 @@ static const struct rte_cryptodev_capabilities 
aesni_mb_pmd_capabilities[] = {
.increment = 0
},
.aad_size = { 0 }
-   }
-   }
+   }, }
+   }, }
},
{   /* SHA224 HMAC */
.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
-   .sym = {
+   {.sym = {
.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
-   .auth = {
+   {.auth = {

[dpdk-dev] [PATCH v3 00/10] qede: Add qede PMD

2016-03-30 Thread Bruce Richardson
On Tue, Mar 29, 2016 at 08:52:38PM +, Rasesh Mody wrote:
> Hi Bruce,
> 
> > From: Bruce Richardson [mailto:bruce.richardson at intel.com]
> > Sent: Tuesday, March 22, 2016 4:30 AM
> > 
> > On Tue, Mar 22, 2016 at 11:21:25AM +, Richardson, Bruce wrote:
> > > I've had a quick scan over this patchset, and as you've probably seen I've
> > made some public comments on it. General comments on the whole
> > patchset are:
> > > * Please run checkpatch on the patchset and clear up as many issues as
> > > you can. There are a number of typos called out which especially must
> > > be fixed. Both myself and Thomas always run checkpatch against patches
> > > before applying them. [I suggest using Thomas's checkpatches.sh script
> > > to do the checks as it disables many unnecessary warnings from
> > > checkpatch]
> > > * Please put in commit descriptions for all patches bar those doing 
> > > trivial
> > things. The first three patches probably don't need a commit message, but
> > the rest do.
> > >
> > > /Bruce
> > >
> > > > -Original Message-
> > > > From: Rasesh Mody [mailto:rasesh.mody at qlogic.com]
> > > > Sent: Saturday, March 19, 2016 12:53 AM
> > > > To: thomas.monjalon at 6wind.com; Richardson, Bruce
> > > > 
> > > > Cc: dev at dpdk.org; ameen.rahman at qlogic.com; harish.patil at 
> > > > qlogic.com;
> > > > sony.chacko at qlogic.com; Rasesh Mody 
> > > > Subject: [PATCH v3 00/10] qede: Add qede PMD
> > > >
> > > > Submitting v3 patch series for QEDE PMD. There is no code change
> > > > from v2 series except PMD version change. Earlier we had generated
> > > > and tested the
> > > > v2 series against dpdk tree then latest.
> > > >
> > > > The v3 series includes:
> > > >  - Patches generated and tested against latest dpdk-next-net
> > > >  - Reworked MAINTAINERS patch to make it apply cleanly
> > > >  - Incorporated Overview.rst update in the documentation patch
> > > >
> > > > Please Apply.
> > > >
> > > > Thanks!
> > > > Rasesh
> > > >
> > > > Rasesh Mody (10):
> > > >   qede: Add maintainers
> > > >   qede: Add documentation
> > > >   qede: Add license file
> > > >   qede: Add base driver
> > > >   qede: Add core driver
> > > >   qede: Add L2 support
> > > >   qede: Add SRIOV support
> > > >   qede: Add attention support
> > > >   qede: Add DCBX support
> > > >   qede: Enable PMD build
> > > >
> > Clang gives a compile error after applying this patchset. Please 
> > investigate.
> > 
> > == Build drivers/net/qede
> >   CC base/ecore_dev.o
> > fatal error: unknown warning option '-Wno-shift-negative-value'; did you
> > mean
> >   '-Wno-shift-sign-overflow'? [-Wunknown-warning-option]
> > /home/bruce/next-net/dpdk-next-net/mk/internal/rte.compile-
> > pre.mk:126: recipe for target 'base/ecore_dev.o' failed
> > 
> > This is seen with clang 3.6 on Fedora 23.
> > "clang version 3.6.0 (tags/RELEASE_360/final)"
> 
> We had compiled all our previously submitted driver patches against clang 
> v3.8 on  RH7.1 and didn't see a similar error. The 
> '-Wno-shift-negative-value' option got introduced after 3.6.0 release.  Pls. 
> let us know if we need to support a minimum version of clang. 
> "clang version 3.8.0 (trunk 249596) (llvm/trunk 249595)"
>  

Thanks for the update. I'm not sure that we have a minimum version of clang that
we have to support - we tend to test more with gcc than clang. If the latest
clang version most common distro's compiles this fine, I'd say it's ok. [The 
latest
on Fedora 23 is actually 3.7, so we are ok with that].
Anyone in the community have a minimum clang version that they need DPDK to
support?

> > Similarly, 32-bit (i686) build fails:
> > 
> > == Build drivers/net/qede
> >   CC base/ecore_dev.o
> > /home/bruce/next-net/dpdk-next-
> > net/drivers/net/qede/base/ecore_dev.c: In function
> > ?ecore_chain_alloc_sanity_check?:
> > /home/bruce/next-net/dpdk-next-
> > net/drivers/net/qede/base/ecore_dev.c:2571:32: error: format ?%lx?
> > expects argument of type ?long unsigned int?, but argument 6 has type ?u64
> > {aka long long unsigned int}? [-Werror=format=] compilation terminated due
> > to -Wfatal-errors.
> > cc1: all warnings being treated as errors
> > 
> > This is seen with gcc 5.3.1 on Fedora 23.
> 
> For 32-bit, we are preparing our driver to compile against gcc version 4.3.4.
> 
> Similary, we had compile tested all our previously submitted driver patches 
> on FreeBSD using:
> FreeBSD clang version 3.6.1 and
> gcc version 4.8.5

Great, thanks.

/Bruce

> 
> Thanks!
> Rasesh
> 
> > Regards,
> > /Bruce


[dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for x86_64 without SSE4.2

2016-03-30 Thread Dumitrescu, Cristian


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, February 16, 2016 6:46 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/4] examples/ip_pipeline: fix build for
> x86_64 without SSE4.2
> 
> The compiler cannot use _mm_crc32_u64:
> 
> examples/ip_pipeline/pipeline/hash_func.h:165:9:
> error: implicit declaration of function '_mm_crc32_u64' is invalid in C99
> 
> Fixes: 947024a26df7 ("examples/ip_pipeline: rework passthrough pipeline")
> 
> Signed-off-by: Thomas Monjalon 
> ---
>  examples/ip_pipeline/pipeline/hash_func.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ip_pipeline/pipeline/hash_func.h
> b/examples/ip_pipeline/pipeline/hash_func.h
> index 7846300..1953ad4 100644
> --- a/examples/ip_pipeline/pipeline/hash_func.h
> +++ b/examples/ip_pipeline/pipeline/hash_func.h
> @@ -152,7 +152,7 @@ hash_xor_key64(void *key, __rte_unused uint32_t
> key_size, uint64_t seed)
>   return (xor0 >> 32) ^ xor0;
>  }
> 
> -#if defined(__x86_64__)
> +#if defined(__x86_64__) && defined(RTE_CPUFLAG_SSE4_2)
> 
>  #include 
> 
> --
> 2.7.0

Hi Thomas,

This is not the correct fix, as RTE_CPUFLAG_SSE4_2 is a flag that can only be 
tested at run-time (as result of calling function rte_cpu_get_flag_enabled()), 
not at build-time.

The reason it appears to fix the build issue you are mentioning is the fact 
that this change results in disabling the  __x86_64__ code branch.

We need to revert this patch and look for a better option.

What is the compiler that generates the build issue you are mentioning? We 
could not reproduce it with gcc 5 (gcc 5.3.1).

Thanks,
Cristian



[dpdk-dev] [PATCH 0/4] port: fix and test bugs in tx_bulk ops

2016-03-30 Thread Thomas Monjalon
2016-03-28 16:51, Robert Sanford:
> This patch series does the following:
> 
> * enhances port ring writer test, to send two large, but not full
>   bursts; exposes ring writer buffer overflow
> * fixes ring writer buffer overflow
> * fixes full burst checks in ethdev, ring, and sched f_tx_bulk ops
> * fixes ethdev writer, to send bursts no larger than specified max

Cristian, a fast review would be helpful to integrate these fixes
in 16.04.


[dpdk-dev] [PATCH 1/2] Fix CPU and memory parameters on IBM POWER8

2016-03-30 Thread Thomas Monjalon
2016-03-25 09:48, David Marchand:
> On Fri, Mar 25, 2016 at 9:11 AM, Chao Zhu  
> wrote:
> > This patch fixes the max logic number and memory channel number settings
> > on IBM POWER8 platform.
> > 1. The max number of logic cores of a POWER8 processor is 96. Normally,
> >there are two sockets on a server. So the max number of logic cores
> >are 192. So this parch set CONFIG_RTE_MAX_LCORE to 256.
> 
> This is a power8 configuration item, this should go to power8 config
> file, not common_base.
> 
> > 2. Currently, the max number of memory channels are hardcoded to 4. However,
> >on a POWER8 machine, the max number of memory channels are 8. To fix 
> > this,
> >CONFIG_RTE_MAX_NCHANNELS is added to do the configuration.
> 
> I don't see any reason why we would need a max value for force_nchannel.
> We should just get rid of this check, this is an obscure parameter for
> most people, so people playing with it know what they are doing
> (hopefully ?).
> 
> On the other hand, if power8 has some specifics about it, maybe we
> should introduce some default value in a arch eal header for other
> dpdk components to use (like in mempool).
> Thoughts ?

Chao? We are running out of time for 16.04.


[dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

2016-03-30 Thread Thomas Monjalon
2016-03-24 08:54, Panu Matilainen:
> --- a/drivers/net/e1000/Makefile
> +++ b/drivers/net/e1000/Makefile
> @@ -54,6 +54,9 @@ else
>   #
>   CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>   CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> +endif

Aaron, have you tested this solution?
Are you going to provide a v3?


[dpdk-dev] [PATCH] Fix KNI compilation under Wind River Linux 6.0 recent RCPLs.

2016-03-30 Thread Lee Roberts
skb_set_hash() has been backported to recent Wind River Linux 6.0 RCPLs.
As a result, the corresponding stanza in kcompat.h must be removed.
Similar patches have already been applied for RHEL, SLES and Ubuntu.

Wind River Linux does not provide convenient macros for kernel version
identification.  Add macros to Makefile to identify the Wind River Linux
version.

Signed-off-by: Lee Roberts 
---
 lib/librte_eal/linuxapp/kni/Makefile  |  8 
 lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h | 13 +
 2 files changed, 21 insertions(+)

diff --git a/lib/librte_eal/linuxapp/kni/Makefile 
b/lib/librte_eal/linuxapp/kni/Makefile
index ac99d3f..6310615 100644
--- a/lib/librte_eal/linuxapp/kni/Makefile
+++ b/lib/librte_eal/linuxapp/kni/Makefile
@@ -51,6 +51,14 @@ UBUNTU_KERNEL_CODE := $(shell echo `grep UTS_RELEASE 
$(RTE_KERNELDIR)/include/ge
 MODULE_CFLAGS += 
-D"UBUNTU_KERNEL_CODE=UBUNTU_KERNEL_VERSION($(UBUNTU_KERNEL_CODE))"
 endif

+ifeq ($(shell lsb_release -si 2>/dev/null),wrlinux)
+WRLINUX_MAJOR := $(shell lsb_release -sr | cut -d. -f1)
+WRLINUX_MINOR := $(shell lsb_release -sr | cut -d. -f2)
+WRLINUX_RCPL  := $(shell lsb_release -sr | cut -d. -f4)
+MODULE_CFLAGS += 
-D"WRLINUX_RELEASE_CODE=WRLINUX_RELEASE_VERSION($(WRLINUX_MAJOR),$(WRLINUX_MINOR))"
+MODULE_CFLAGS += -D"WRLINUX_RCPL=$(WRLINUX_RCPL)"
+endif
+
 # this lib needs main eal
 DEPDIRS-y += lib/librte_eal/linuxapp/eal

diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h
index e2cf71e..b25a35f 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h
@@ -730,6 +730,17 @@ struct _kc_ethtool_pauseparam {
 #define UBUNTU_KERNEL_CODE 0
 #endif

+/* Wind River release codes must be specified from Makefile */
+#ifndef WRLINUX_RELEASE_VERSION
+#define WRLINUX_RELEASE_VERSION(a,b) (((a) * 256) + (b))
+#endif
+#ifndef WRLINUX_RELEASE_CODE
+#define WRLINUX_RELEASE_CODE 0
+#endif
+#ifndef WRLINUX_RCPL
+#define WRLINUX_RCPL 0
+#endif
+
 #ifdef __KLOCWORK__
 #ifdef ARRAY_SIZE
 #undef ARRAY_SIZE
@@ -3868,6 +3879,7 @@ static inline struct sk_buff 
*__kc__vlan_hwaccel_put_tag(struct sk_buff *skb,
 && (UBUNTU_RELEASE_CODE == UBUNTU_RELEASE_VERSION(12,4) \
  || UBUNTU_RELEASE_CODE == UBUNTU_RELEASE_VERSION(14,4
 #if (!(SLE_VERSION_CODE == SLE_VERSION(12,0,0)))
+#if (!(WRLINUX_RELEASE_CODE == WRLINUX_RELEASE_VERSION(6,0) && WRLINUX_RCPL >= 
26))
 #ifdef NETIF_F_RXHASH
 #define PKT_HASH_TYPE_L3 0
 static inline void
@@ -3876,6 +3888,7 @@ skb_set_hash(struct sk_buff *skb, __u32 hash, 
__always_unused int type)
skb->rxhash = hash;
 }
 #endif /* NETIF_F_RXHASH */
+#endif /* < WRLINUX */
 #endif /* < SLES12 */
 #endif /* < 3.13.0-30.54 (Ubuntu 14.04) */
 #endif /* < RHEL7 */
-- 
1.9.1



[dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

2016-03-30 Thread Burakov, Anatoly
Hi Mauricio,

Good points. Would you be willing to prepare a patch to fix these issues?

Thanks,
Anatoly

From: Mauricio V?squez [mailto:mauricio.vasquezber...@studenti.polito.it]
Sent: Wednesday, March 30, 2016 10:13 AM
To: Burakov, Anatoly 
Cc: dev at dpdk.org; Gonzalez Monroy, Sergio ; Richardson, Bruce 
Subject: Re: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

Hi Anatoly,
Thank you very much, I did not take into account that detail.
I have two additional concerns about it:
1. I think it is possible to have a race condition. The memzone is marked as 
not freeable after it has been added to the ivshmem device, then it is possible 
to free the memzone just after it has been  added to the metadata but before it 
is marked as not freeable.
Shouldn't the memzone be marked before adding the memzone to the ivshmem device?

2. Are the #ifdefs necessary?, we already are in  a file that will only 
compiled when ivshmem is enabled.
Thanks,
Mauricio Vasquez,


On Wed, Mar 30, 2016 at 10:29 AM, Burakov, Anatoly mailto:anatoly.burakov at intel.com>> wrote:
Hi Mauricio,

You?re not missing anything. It would be done this way, if the memzone 
parameter wasn?t const. But it is const, so we have to find the memzone in 
config to edit it.

Thanks,
Anatoly

From: Mauricio V?squez [mailto:mauricio.vasquezbernal at 
studenti.polito.it]
Sent: Wednesday, March 30, 2016 9:22 AM
To: dev at dpdk.org
Cc: Gonzalez Monroy, Sergio mailto:sergio.gonzalez.monroy at intel.com>>; Richardson, Bruce 
mailto:bruce.richardson at intel.com>>; Burakov, 
Anatoly mailto:anatoly.burakov at intel.com>>
Subject: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

Dear All,
I was looking at that patch, I can understand its functionality but not its 
implementation..
Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would not 
be sufficient? After all, the goal is to mark the memzone as used by ivshmem to 
forbid  freeing it.
Please corrected if I am missing something.
Thank you,
Mauricio Vasquez,



[dpdk-dev] [PATCH 0/4] port: fix and test bugs in tx_bulk ops

2016-03-30 Thread Dumitrescu, Cristian


> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, March 30, 2016 12:00 PM
> To: Dumitrescu, Cristian 
> Cc: dev at dpdk.org; Robert Sanford 
> Subject: Re: [dpdk-dev] [PATCH 0/4] port: fix and test bugs in tx_bulk ops
> 
> 2016-03-28 16:51, Robert Sanford:
> > This patch series does the following:
> >
> > * enhances port ring writer test, to send two large, but not full
> >   bursts; exposes ring writer buffer overflow
> > * fixes ring writer buffer overflow
> > * fixes full burst checks in ethdev, ring, and sched f_tx_bulk ops
> > * fixes ethdev writer, to send bursts no larger than specified max
> 
> Cristian, a fast review would be helpful to integrate these fixes
> in 16.04.

Please note Robert's patches are not trivial and the potential impact is quite 
high. We'll keep you update We are working on reviewing them and will send 
update on our progress.

These potential changes come up very late into the 16.04 cycle. Given their 
nature, once they get merged, one RC cycle is required by the validation team 
to fully test them, so I want to make sure I flag this to you as early as 
possible.



[dpdk-dev] [PATCH 1/2 v2] lib/librte_lpm: Fix anonymous union initialization issue

2016-03-30 Thread Michael Qiu
In SUSE11-SP3 i686 platform, with gcc 4.5.1, there is a
compile issue:
rte_lpm.c: In function ?add_depth_small_v20?:
rte_lpm.c:778:7: error: unknown field ?next_hop?
specified in initializer
cc1: warnings being treated as errors
The root casue is gcc only allow anonymous union initialized
according to the field it is defined. But next_hop is defined
in different field when in different platform(Endian).

One solution is add if define in the code to avoid this issue,
but there is a simple way, initialize it separately later.

Fixes: afc5c914a083 ("lpm: fix big endian support")

Signed-off-by: Michael Qiu 
---
v2 --> v1:
Fixes whilespace issue around "="

 lib/librte_lpm/rte_lpm.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index af5811c..efd507e 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -744,11 +744,11 @@ add_depth_small_v20(struct rte_lpm_v20 *lpm, uint32_t ip, 
uint8_t depth,
lpm->tbl24[i].depth <= depth)) {

struct rte_lpm_tbl_entry_v20 new_tbl24_entry = {
-   { .next_hop = next_hop, },
.valid = VALID,
.valid_group = 0,
.depth = depth,
};
+   new_tbl24_entry.next_hop = next_hop;

/* Setting tbl24 entry in one go to avoid race
 * conditions
@@ -775,8 +775,8 @@ add_depth_small_v20(struct rte_lpm_v20 *lpm, uint32_t ip, 
uint8_t depth,
.valid = VALID,
.valid_group = VALID,
.depth = depth,
-   .next_hop = next_hop,
};
+   new_tbl8_entry.next_hop = next_hop;

/*
 * Setting tbl8 entry in one go to avoid
@@ -975,10 +975,9 @@ add_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t 
ip_masked, uint8_t depth,
struct rte_lpm_tbl_entry_v20 new_tbl8_entry = {
.valid = VALID,
.depth = depth,
-   .next_hop = next_hop,
.valid_group = lpm->tbl8[i].valid_group,
};
-
+   new_tbl8_entry.next_hop = next_hop;
/*
 * Setting tbl8 entry in one go to avoid race
 * condition
@@ -1375,9 +1374,9 @@ delete_depth_small_v20(struct rte_lpm_v20 *lpm, uint32_t 
ip_masked,
.valid = VALID,
.valid_group = VALID,
.depth = sub_rule_depth,
-   .next_hop = lpm->rules_tbl
-   [sub_rule_index].next_hop,
};
+   new_tbl8_entry.next_hop =
+   lpm->rules_tbl[sub_rule_index].next_hop;

for (i = tbl24_index; i < (tbl24_index + tbl24_range); i++) {

@@ -1639,9 +1638,10 @@ delete_depth_big_v20(struct rte_lpm_v20 *lpm, uint32_t 
ip_masked,
.valid = VALID,
.depth = sub_rule_depth,
.valid_group = lpm->tbl8[tbl8_group_start].valid_group,
-   .next_hop = lpm->rules_tbl[sub_rule_index].next_hop,
};

+   new_tbl8_entry.next_hop =
+   lpm->rules_tbl[sub_rule_index].next_hop;
/*
 * Loop through the range of entries on tbl8 for which the
 * rule_to_delete must be modified.
-- 
1.9.3



[dpdk-dev] [PATCH v13 0/8] ethdev: 100G and link speed API refactoring

2016-03-30 Thread Thomas Monjalon
2016-03-26 02:27, Marc Sune:
> There are still too few tests and reviews, especially for
> autonegotiation with Intel devices (patch #6).
> I would not be surprised to see some bugs in this rework.
> 
> The capabilities must be adapted per device. It can be
> improved in a separate patch.
> 
> It will be integrated in 16.04-rc3.
> Please test and review shortly, thanks!

Still no feedback for e1000 and i40e devices.
Should we consider it is working fine?


[dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

2016-03-30 Thread Mauricio Vásquez
Hi Anatoly,

Thank you very much, I did not take into account that detail.

I have two additional concerns about it:

1. I think it is possible to have a race condition. The memzone is marked
as not freeable after it has been added to the ivshmem device, then it is
possible to free the memzone just after it has been  added to the metadata
but before it is marked as not freeable.
Shouldn't the memzone be marked before adding the memzone to the ivshmem
device?

2. Are the #ifdefs necessary?, we already are in  a file that will only
compiled when ivshmem is enabled.

Thanks,

Mauricio Vasquez,


On Wed, Mar 30, 2016 at 10:29 AM, Burakov, Anatoly <
anatoly.burakov at intel.com> wrote:

> Hi Mauricio,
>
>
>
> You?re not missing anything. It would be done this way, if the memzone
> parameter wasn?t const. But it is const, so we have to find the memzone in
> config to edit it.
>
>
>
> Thanks,
>
> Anatoly
>
>
>
> *From:* Mauricio V?squez [mailto:mauricio.vasquezbernal at studenti.polito.it]
>
> *Sent:* Wednesday, March 30, 2016 9:22 AM
> *To:* dev at dpdk.org
> *Cc:* Gonzalez Monroy, Sergio ;
> Richardson, Bruce ; Burakov, Anatoly <
> anatoly.burakov at intel.com>
> *Subject:* Question about cd10c42eb5bc ("mem: fix ivshmem freeing")
>
>
>
> Dear All,
>
> I was looking at that patch, I can understand its functionality but not
> its implementation..
>
> Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would
> not be sufficient? After all, the goal is to mark the memzone as used by
> ivshmem to forbid  freeing it.
>
> Please corrected if I am missing something.
>
> Thank you,
>
> Mauricio Vasquez,
>


[dpdk-dev] [PATCH] enic: expose RX missed packets counter

2016-03-30 Thread John Daley
Update the 'imissed' counter with the number of packets dropped
by the NIC.

Fixes: fefed3d1e62c ("enic: new driver")

Signed-off-by: John Daley 
Reviewed-by: Nelson Escobar 
---
 drivers/net/enic/enic_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 2f79cf0..e3da51d 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -246,6 +246,8 @@ void enic_dev_stats_get(struct enic *enic, struct 
rte_eth_stats *r_stats)
r_stats->ierrors = stats->rx.rx_errors;
r_stats->oerrors = stats->tx.tx_errors;

+   r_stats->imissed = stats->rx.rx_drop;
+
r_stats->imcasts = stats->rx.rx_multicast_frames_ok;
r_stats->rx_nombuf = stats->rx.rx_no_bufs;
 }
-- 
2.7.0



[dpdk-dev] DPDK: receive single packet at a time

2016-03-30 Thread Mohan Prasad
Hi Bruce,

Could not get it working by disabling the vector PMD, Do you have any
example where it works?

Thanks,
Mohan

On Tue, Mar 29, 2016 at 6:46 PM, Bruce Richardson <
bruce.richardson at intel.com> wrote:

> On Tue, Mar 29, 2016 at 06:31:58PM +0530, Mohan Prasad wrote:
> > Hi,
> >
> > I have tried this and it does not work
> >
>
> What type of NIC are you using. If you are using ixgbe or i40e, try
> disabling
> the vector PMD in your build-time configuration to see if it makes a
> difference.
>
> However, why do you want to receive just a single packet at a time. Why
> not just
> receive a burst of packets and then process them one at a time? It's much
> more
> efficient that way, and you should get better performance from your
> application.
>
> /Bruce
>
>
> > Thanks,
> > Mohan
> > On Mar 29, 2016 6:26 PM, "Wiles, Keith"  wrote:
> >
> > > >Hi,
> > > >
> > > >Is there any option to receive single packet at a time with dpdk?
> > >
> > > Not sure if this is the answer you are looking for, but if you just
> > > request a single packet with
> > >
> > > struct rte_mbuf *mbuf;
> > > rte_eth_rx_burst(port_id, queue_id, , 1);
> > >
> > > will return only one packet as a time.
> > > >
> > > >Thanks,
> > > >Mohan
> > > >
> > >
> > >
> > > Regards,
> > > Keith
> > >
> > >
> > >
> > >
> > >
>


[dpdk-dev] [PATCH] ixgbe/base: fix VF multi-queue failure

2016-03-30 Thread Wenzhuo Lu
When starting testpmd with multiple queues on a ixgbe VF
port, it failed with this printing, "nb_rxq(4) is greater
than max_rx_queues(1)".

The root cause is the VF doesn't get the right max rx queue
number from PF and it uses the default value 1.
VF max rx queue number is set by PF through mailbox messages.
The message for this setting only supports version 1.1. As
message version is updated to 1.2, VF cannot parse the rx queue
number setting message correctly.

This patch raise a specific base code update for this issue.

Fixes: 72dec9e37a84 ("ixgbe: support multicast promiscuous mode on VF")
Signed-off-by: Wenzhuo Lu 
---
 drivers/net/ixgbe/base/ixgbe_vf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ixgbe/base/ixgbe_vf.c 
b/drivers/net/ixgbe/base/ixgbe_vf.c
index dbb5194..40dc1c8 100644
--- a/drivers/net/ixgbe/base/ixgbe_vf.c
+++ b/drivers/net/ixgbe/base/ixgbe_vf.c
@@ -675,6 +675,7 @@ int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int 
*num_tcs,
/* do nothing if API doesn't support ixgbevf_get_queues */
switch (hw->api_version) {
case ixgbe_mbox_api_11:
+   case ixgbe_mbox_api_12:
break;
default:
return 0;
-- 
1.9.3



[dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

2016-03-30 Thread Mauricio Vásquez
Dear All,

I was looking at that patch, I can understand its functionality but not its
implementation..

Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would
not be sufficient? After all, the goal is to mark the memzone as used by
ivshmem to forbid  freeing it.

Please corrected if I am missing something.

Thank you,

Mauricio Vasquez,


[dpdk-dev] Must kni be associated with a dpdk port?

2016-03-30 Thread ALeX Wang
Hi,

I want to use 'rte_kni_alloc()' to create a kernel iface and
use it to test application rx.  From the api and example in
'examples/kni/main.c', i saw the 'conf' argument is assigned
with pci info of a dpdk port.

Want to ask if this is compulsory...  Must kni always be
used together with a dpdk port?

Thanks,
-- 
Alex Wang,


[dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

2016-03-30 Thread Aaron Conole
The register read/write mphy functions have misleading whitespace around
the `locked` check. This cleanup merely preserves the existing functionality
and suppresses future gcc versions' "misleading indentation" warning.

Suggested-by: Panu Matilainen 
Signed-off-by: Aaron Conole 
---
v2:
* Changed from "whitespace-only" fix to a functional change
  moving the phy writes into protection of the `if (locked)`
  code
* Added "Fixes" line.

v3:
* Instead of changing the code, change to suppress the compiler warning
  when using gcc6+. This was tested with both gcc6 and gcc5 using gnu
  make 4.0 and gnu bash 4.3.42 on a fedora 23 system.

 drivers/net/e1000/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index ccd2b7b..f4879e6 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -54,6 +54,9 @@ else
 #
 CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
 CFLAGS_BASE_DRIVER += -Wno-unused-variable
+ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
+endif
 endif

 #
-- 
2.5.5


[dpdk-dev] [PATCH v13 5/8] ethdev: add speed capabilities

2016-03-30 Thread Marc
On 29 March 2016 at 15:31, Alejandro Lucero 
wrote:

> For nfp.c, speed_capa should be ETH_LINK_SPEED_40G instead of 
> ETH_LINK_SPEED_50G.
> By the way, the change in patch 4 sets the right link speed using the new
> constants.
>
> Regards
>

Noted for v14, thanks

Marc


>
> On Sat, Mar 26, 2016 at 1:27 AM, Marc Sune  wrote:
>
>> The speed capabilities of a device can be retrieved with
>> rte_eth_dev_info_get().
>>
>> The new field speed_capa is initialized in the drivers without
>> taking care of device characteristics in this patch.
>> When the capabilities of a driver are accurate, the table in
>> overview.rst must be filled.
>>
>> Signed-off-by: Marc Sune 
>> ---
>>  doc/guides/nics/overview.rst   |  1 +
>>  doc/guides/rel_notes/release_16_04.rst |  8 
>>  drivers/net/bnx2x/bnx2x_ethdev.c   |  1 +
>>  drivers/net/cxgbe/cxgbe_ethdev.c   |  1 +
>>  drivers/net/e1000/em_ethdev.c  |  4 
>>  drivers/net/e1000/igb_ethdev.c |  4 
>>  drivers/net/ena/ena_ethdev.c   |  9 +
>>  drivers/net/fm10k/fm10k_ethdev.c   |  4 
>>  drivers/net/i40e/i40e_ethdev.c |  8 
>>  drivers/net/ixgbe/ixgbe_ethdev.c   |  8 
>>  drivers/net/mlx4/mlx4.c|  6 ++
>>  drivers/net/mlx5/mlx5_ethdev.c |  8 
>>  drivers/net/nfp/nfp_net.c  |  2 ++
>>  lib/librte_ether/rte_ethdev.h  | 21 +
>>  14 files changed, 85 insertions(+)
>>
>> diff --git a/doc/guides/nics/overview.rst b/doc/guides/nics/overview.rst
>> index 542479a..62f1868 100644
>> --- a/doc/guides/nics/overview.rst
>> +++ b/doc/guides/nics/overview.rst
>> @@ -86,6 +86,7 @@ Most of these differences are summarized below.
>>e   e   e   e   e
>>e
>>c   c   c   c   c
>>c
>>  = = = = = = = = = = = = = = = = = = = = = = = =
>> = = = = = = = = =
>> +   speed capabilities
>> link status  X   X X
>>  X X
>> link status eventX X
>>X
>> queue status event
>>X
>> diff --git a/doc/guides/rel_notes/release_16_04.rst
>> b/doc/guides/rel_notes/release_16_04.rst
>> index 79d76e1..9e7b0b7 100644
>> --- a/doc/guides/rel_notes/release_16_04.rst
>> +++ b/doc/guides/rel_notes/release_16_04.rst
>> @@ -47,6 +47,11 @@ This section should contain new features added in this
>> release. Sample format:
>>A new function ``rte_pktmbuf_alloc_bulk()`` has been added to allow
>> the user
>>to allocate a bulk of mbufs.
>>
>> +* **Added device link speed capabilities.**
>> +
>> +  The structure ``rte_eth_dev_info`` has now a ``speed_capa`` bitmap,
>> which
>> +  allows the application to know the supported speeds of each device.
>> +
>>  * **Added new poll-mode driver for Amazon Elastic Network Adapters
>> (ENA).**
>>
>>The driver operates variety of ENA adapters through feature negotiation
>> @@ -456,6 +461,9 @@ This section should contain API changes. Sample
>> format:
>>All drivers are now counting the missed packets only once, i.e.
>> drivers will
>>not increment ierrors anymore for missed packets.
>>
>> +* The ethdev structure ``rte_eth_dev_info`` was changed to support device
>> +  speed capabilities.
>> +
>>  * The functions ``rte_eth_dev_udp_tunnel_add`` and
>> ``rte_eth_dev_udp_tunnel_delete``
>>have been renamed into ``rte_eth_dev_udp_tunnel_port_add`` and
>>``rte_eth_dev_udp_tunnel_port_delete``.
>> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c
>> b/drivers/net/bnx2x/bnx2x_ethdev.c
>> index a3c6c01..897081f 100644
>> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
>> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
>> @@ -327,6 +327,7 @@ bnx2x_dev_infos_get(struct rte_eth_dev *dev,
>> __rte_unused struct rte_eth_dev_inf
>> dev_info->min_rx_bufsize = BNX2X_MIN_RX_BUF_SIZE;
>> dev_info->max_rx_pktlen  = BNX2X_MAX_RX_PKT_LEN;
>> dev_info->max_mac_addrs  = BNX2X_MAX_MAC_ADDRS;
>> +   dev_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_20G;
>>  }
>>
>>  static void
>> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c
>> b/drivers/net/cxgbe/cxgbe_ethdev.c
>> index 8845c76..bb134e5 100644
>> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
>> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
>> @@ -171,6 +171,7 @@ static void cxgbe_dev_info_get(struct rte_eth_dev
>> *eth_dev,
>>
>> device_info->rx_desc_lim = cxgbe_desc_lim;
>> device_info->tx_desc_lim = cxgbe_desc_lim;
>> +   device_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_40G;
>>  }
>>
>>  static void cxgbe_dev_promiscuous_enable(struct rte_eth_dev *eth_dev)
>> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
>> index 473d77f..d5f8c7f 100644
>> --- a/drivers/net/e1000/em_ethdev.c
>> +++ b/drivers/net/e1000/em_ethdev.c
>> @@ -1054,6 +1054,10 @@ eth_em_infos_get(struct rte_eth_dev *dev, struct
>> 

[dpdk-dev] e1000: randomly loosing link change events triggered by the peer

2016-03-30 Thread Marc
On 29 March 2016 at 03:19, Lu, Wenzhuo  wrote:

> Hi Marc,
>
>
>
> *From:* marc.sune at gmail.com [mailto:marc.sune at gmail.com] *On Behalf Of *
> Marc
> *Sent:* Monday, March 28, 2016 7:03 PM
> *To:* Lu, Wenzhuo
> *Cc:* dev at dpdk.org
> *Subject:* Re: e1000: randomly loosing link change events triggered by
> the peer
>
>
>
>
>
>
>
> On 28 March 2016 at 03:54, Lu, Wenzhuo  wrote:
>
> Hi Marc
>
>
>
> *From:* Marc Sune [mailto:marcdevel at gmail.com]
> *Sent:* Saturday, March 26, 2016 9:43 AM
> *To:* dev at dpdk.org; Lu, Wenzhuo
> *Subject:* e1000: randomly loosing link change events triggered by the
> peer
>
>
>
> I found that in the current HEAD in master testing it with an I218-LM in
> autoneg mode, when link is forced to be renegociated by the peer (e.g. via
> ethtool on a peer Linux box) _some_ change events are lost.
>
>
>
> It is quite random, but it seems to happen more while changing the speed
> than when changing the duplex mode.
>
>
>
> However, when one or more link change events have been lost and the phy
> medium is disconnected and reconnected, the link speed and duplex mode are
> then correctly updated.
>
>
>
> Marc
>
>
>
> [Wenzhuo] Thanks for let us know this issue. May I ask some questions? Do
> you mean this NIC 0x155A?
>
>
>
> 0x15A2,  I218-LM (rev 03)
>
>
>
> EAL: PCI device :00:19.0 on NUMA socket -1
>
> EAL:   probe driver: 8086:15a2 rte_em_pmd
>
> EAL:   PCI memory mapped at 0x7f85cf40
>
> EAL:   PCI memory mapped at 0x7f85cf42
>
> PMD: eth_em_dev_init(): port_id 0 vendorID=0x8086 deviceID=0x15a2
>
>
>
> I think this is not a NIC issue, but a general problem of the driver (or
> em code).
>
> [Wenzhuo] I don?t have a 15a2 on hand. I?m using i350. I haven?t hit the
> same problem.  As you said it?s random. Would you like to let me know why
> you think it?s general not NIC specific? Thanks.
>

It was random but it was happening very frequently.


>
>
> About how to reproduce this problem, you mean use these CLIs, ethtool ?s
> xxx advertise xxx, ethtool ?s xxx duplex half/full, to change the peer
> port?s configuration?
>
>
>
> Correct. I modified l2fwd to check link status and print it on each port
> stats print iteration. Then from the peer I modified the link properties
> via ethtool.
>
> [Wenzhuo] Would you like to give me a patch of your change? Thanks.
>


Unfortunately I checkedout that code, as it was a hack to test, but it was
as simple as calling check_all_ports_link_status() in print_stats() for
l2fwd example, so that was reporting link status periodically.

Marc


>
>
> The result is that transitions from autoneg speeds and/or duplex mode
> settings are randomly not detected (rte_eth_link in rte_eth_dev_data is not
> updated), and it prints not-up-to-date state.
>
>
>
> Marc
>


[dpdk-dev] [PATCH v13 6/8] ethdev: redesign link speed config

2016-03-30 Thread Marc
On 29 March 2016 at 08:18, Xing, Beilei  wrote:

>
>
> > -Original Message-
> > From: Marc Sune [mailto:marcdevel at gmail.com]
> > Sent: Saturday, March 26, 2016 9:27 AM
> > To: Thomas Monjalon ; Xu, Qian Q
> > ; Xing, Beilei ;
> dev at dpdk.org;
> > Ananyev, Konstantin ; Lu, Wenzhuo
> > ; Richardson, Bruce  > intel.com>;
> > Glynn, Michael J 
> > Cc: Marc Sune 
> > Subject: [PATCH v13 6/8] ethdev: redesign link speed config
> >
>
> > a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index a98e8eb..6cc2da0 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -2193,32 +2195,21 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> >   if (err)
> >   goto error;
> >
> > + speed = 0x0;
> > + if (*link_speeds & ETH_LINK_SPEED_10G)
> > + speed |= IXGBE_LINK_SPEED_10GB_FULL;
> > + if (*link_speeds & ETH_LINK_SPEED_1G)
> > + speed |= IXGBE_LINK_SPEED_1GB_FULL;
> > + if (*link_speeds & ETH_LINK_SPEED_100M)
> > + speed |= IXGBE_LINK_SPEED_100_FULL;
> > +
> >   err = ixgbe_setup_link(hw, speed, link_up);
> >   if (err)
> >   goto error;
>
> Hi Marc,
> According to ixgbe HW, link speed shouldn't be 0 when setting up,
> Otherwise device will start fail. So we need to set speed if link_speed
> is ETH_LINK_SPEED_AUTONEG. Following code is for reference.
>
> speed = 0x0;
> if ((*link_speeds & 0x1) == ETH_LINK_SPEED_AUTONEG)
> speed = (hw->mac.type != ixgbe_mac_82598EB) ?
> IXGBE_LINK_SPEED_82599_AUTONEG :
> IXGBE_LINK_SPEED_82598_AUTONEG;
> else {
> if (*link_speeds & ETH_LINK_SPEED_10G)
> speed |= IXGBE_LINK_SPEED_10GB_FULL;
> if (*link_speeds & ETH_LINK_SPEED_1G)
> speed |= IXGBE_LINK_SPEED_1GB_FULL;
> if (*link_speeds & ETH_LINK_SPEED_100M)
> speed |= IXGBE_LINK_SPEED_100_FULL;
> }
>

Beilei,

OK, thanks.

Can you/someone please try v13 + this modification, so that we make sure
this is the final version for ixgbe?

Regards
Marc


>
> Beilei Xing
> Thanks
>


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Stephen Hemminger
On Tue, 29 Mar 2016 22:28:20 -0700
Rasesh Mody  wrote:

> +
> +void qede_config_rx_mode(struct rte_eth_dev *eth_dev)
> +{
> + struct qede_dev *qdev = eth_dev->data->dev_private;
> + struct ecore_dev *edev = >edev;
> + /* TODO: - QED_FILTER_TYPE_UCAST */
> + enum qed_filter_rx_mode_type accept_flags =
> + QED_FILTER_RX_MODE_TYPE_REGULAR;
> + struct qed_filter_params rx_mode;
> + int rc;
> +
> + /* Configure the struct for the Rx mode */
> + memset(_mode, 0, sizeof(struct qed_filter_params));
> + rx_mode.type = QED_FILTER_TYPE_RX_MODE;
> +
> + rc = qede_set_ucast_rx_mac(qdev, QED_FILTER_XCAST_TYPE_REPLACE,
> +eth_dev->data->mac_addrs[0].addr_bytes);
> + if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1) {
> + accept_flags = QED_FILTER_RX_MODE_TYPE_PROMISC;
> + } else {
> + rc = qede_set_ucast_rx_mac(qdev, QED_FILTER_XCAST_TYPE_ADD,
> +eth_dev->data->
> +mac_addrs[0].addr_bytes);
> + if (rc) {
> + DP_ERR(edev, "Unable to add filter\n");
> + return;
> + }
> + }
> +
> + /* take care of VLAN mode */
> + if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1) {
> + qede_config_accept_any_vlan(qdev, true);
> + } else if (!qdev->non_configured_vlans) {
> + /* If we dont have non-configured VLANs and promisc
> +  * is not set, then check if we need to disable
> +  * accept_any_vlan mode.
> +  * Because in this case, accept_any_vlan mode is set
> +  * as part of IFF_RPOMISC flag handling.
> +  */
> + qede_config_accept_any_vlan(qdev, false);
> + }
> + rx_mode.filter.accept_flags = accept_flags;
> + (void)qdev->ops->filter_config(edev, _mode);

Please don't use lint suppressing dummy (void) casts. I don't think GCC or any
check tool needs these. Better yet check the return value and log errors.


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Stephen Hemminger
On Tue, 29 Mar 2016 22:28:20 -0700
Rasesh Mody  wrote:

> +static void qede_config_accept_any_vlan(struct qede_dev *qdev, bool action)
> +{
> + struct ecore_dev *edev = >edev;
> + struct qed_update_vport_params params;
> + int rc;
> +
> + /* Proceed only if action actually needs to be performed */
> + if (qdev->accept_any_vlan == action)
> + return;
> +
> + memset(, 0, sizeof(params));
> +
> + params.vport_id = 0;
> + params.accept_any_vlan = action;
> + params.update_accept_any_vlan_flg = 1;

Minor nit. A lot of this code uses memset() then sets structure elements.
Why not just use C99 style initialization:

struct qed_update_vport_params params = {
.vport_id = 0,
.accept_any_vlan = action,
.update_accept_any_vlan = 1,
};



[dpdk-dev] [PATCH] scripts: check commit formatting

2016-03-30 Thread Yuanhan Liu
On Tue, Mar 29, 2016 at 11:29:46PM +0200, Thomas Monjalon wrote:
> The git messages have three parts:
> 1/ the headline
> 2/ the explanations
> 3/ the footer tags
> 
> The headline helps to quickly browse an history or catch instantly the
> purpose of a commit. Making it short with some consistent wording
> allows to easily parse it or match some patterns.
> 
> The explanations must give some keys like the reason of the change.
> Nothing can be automatically checked for this part.

Actually, I think we might be able to do 2 tests here:

- space line between paragraphs.

- lines over 80 chars.

--yliu


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Stephen Hemminger
On Tue, 29 Mar 2016 22:28:20 -0700
Rasesh Mody  wrote:

> +static void
> +qede_alloc_etherdev(struct qede_dev *qdev, struct qed_dev_eth_info *info)
> +{
> + rte_memcpy(>dev_info, info, sizeof(*info));

Why bother with rte_memcpy here? why not just assignment or memcpy()?

> + qdev->num_tc = qdev->dev_info.num_tc;
> + qdev->ops = qed_ops;
> +}


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Stephen Hemminger
On Tue, 29 Mar 2016 22:28:20 -0700
Rasesh Mody  wrote:

> +
> +static void
> +qede_dev_info_get(struct rte_eth_dev *eth_dev,
> +   struct rte_eth_dev_info *dev_info)
> +{
> + struct qede_dev *qdev = eth_dev->data->dev_private;
> + struct ecore_dev *edev = >edev;
> +
> + PMD_INIT_FUNC_TRACE(edev);
> +
> + dev_info->min_rx_bufsize = (uint32_t)(ETHER_MIN_MTU +
> +   QEDE_ETH_OVERHEAD);
> + dev_info->max_rx_pktlen = (uint32_t)ETH_TX_MAX_NON_LSO_PKT_LEN;
> + dev_info->rx_desc_lim = qede_rx_desc_lim;
> + dev_info->tx_desc_lim = qede_tx_desc_lim;
> + /* Fix it for 8 queues for now */
> + dev_info->max_rx_queues = 8;
> + dev_info->max_tx_queues = 8;
> + dev_info->max_mac_addrs = (uint32_t)(RESC_NUM(>hwfns[0],
> +   ECORE_MAC));
> + dev_info->max_vfs = (uint16_t)NUM_OF_VFS(>edev);
> + dev_info->driver_name = qdev->drv_ver;
> + dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
> + dev_info->flow_type_rss_offloads = (uint64_t)QEDE_RSS_OFFLOAD_ALL;
> + dev_info->default_txconf = (struct rte_eth_txconf) {
> + .txq_flags = QEDE_TXQ_FLAGS,};

Please indent this initialization, like other drivers
dev_info->default_txconf = (struct rte_eth_txconf) {
.txq_flags = QEDE_TXQ_FLAGS,
};


[dpdk-dev] [PATCH v4 05/10] qede: Add core driver

2016-03-30 Thread Stephen Hemminger
On Tue, 29 Mar 2016 22:28:20 -0700
Rasesh Mody  wrote:

> +
> +static void qede_print_adapter_info(struct qede_dev *qdev)
> +{
> + struct ecore_dev *edev = >edev;
> + struct qed_dev_info *info = >dev_info.common;
> + char ver_str[QED_DRV_VER_STR_SIZE] = { 0 };
> +
> + RTE_LOG(INFO, PMD,
> +   " Chip details : %s%d\n",
> +   ECORE_IS_BB(edev) ? "BB" : "AH",
> +   CHIP_REV_IS_A0(edev) ? 0 : 1);
> +
> + sprintf(ver_str, "%s %s_%d.%d.%d.%d", QEDE_PMD_VER_PREFIX,
> + edev->ver_str, QEDE_PMD_VERSION_MAJOR, QEDE_PMD_VERSION_MINOR,
> + QEDE_PMD_VERSION_REVISION, QEDE_PMD_VERSION_PATCH);
> + strcpy(qdev->drv_ver, ver_str);
> + RTE_LOG(INFO, PMD, " Driver version : %s\n", ver_str);
> +
> + ver_str[0] = '\0';
> + sprintf(ver_str, "%d.%d.%d.%d", info->fw_major, info->fw_minor,
> + info->fw_rev, info->fw_eng);
> + RTE_LOG(INFO, PMD, " Firmware version : %s\n", ver_str);
> +
> + ver_str[0] = '\0';
> + sprintf(ver_str, "%d.%d.%d.%d",
> + (info->mfw_rev >> 24) & 0xff,
> + (info->mfw_rev >> 16) & 0xff,
> + (info->mfw_rev >> 8) & 0xff, (info->mfw_rev) & 0xff);
> + RTE_LOG(INFO, PMD, " Management firmware version : %s\n", ver_str);
> +
> + RTE_LOG(INFO, PMD, " Firmware file : %s\n", QEDE_FW_FILE_NAME);

This means the driver is far too chatty in the logs.
Can't this be made DEBUG level?


[dpdk-dev] [PATCH v3 2/7] drivers/net/e1000: Suppress misleading indentation warning

2016-03-30 Thread Stephen Hemminger
On Wed, 30 Mar 2016 10:06:36 -0400
Aaron Conole  wrote:

> The register read/write mphy functions have misleading whitespace around
> the `locked` check. This cleanup merely preserves the existing functionality
> and suppresses future gcc versions' "misleading indentation" warning.
> 
> Suggested-by: Panu Matilainen 
> Signed-off-by: Aaron Conole 
> ---
> v2:
> * Changed from "whitespace-only" fix to a functional change
>   moving the phy writes into protection of the `if (locked)`
>   code
> * Added "Fixes" line.
> 
> v3:
> * Instead of changing the code, change to suppress the compiler warning
>   when using gcc6+. This was tested with both gcc6 and gcc5 using gnu
>   make 4.0 and gnu bash 4.3.42 on a fedora 23 system.
> 
>  drivers/net/e1000/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
> index ccd2b7b..f4879e6 100644
> --- a/drivers/net/e1000/Makefile
> +++ b/drivers/net/e1000/Makefile
> @@ -54,6 +54,9 @@ else
>  #
>  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>  CFLAGS_BASE_DRIVER += -Wno-unused-variable
> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
> +endif
>  endif
>  
>  #

NAK, don't do it to the whole file.
Fix the code (best option)
or use a pragma for the small area which is broken for other reasons.


[dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

2016-03-30 Thread Aaron Conole
Thomas Monjalon  writes:

> 2016-03-24 08:54, Panu Matilainen:
>> --- a/drivers/net/e1000/Makefile
>> +++ b/drivers/net/e1000/Makefile
>> @@ -54,6 +54,9 @@ else
>>   #
>>   CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
>>   CFLAGS_BASE_DRIVER += -Wno-unused-variable
>> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
>> +CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
>> +endif
>
> Aaron, have you tested this solution?
> Are you going to provide a v3?

I haven't yet tested this solution, but if folks are that opposed to
changing the code, then I will test it and post a v3 of this particular
patch in the series. 

Thanks so much for the reviews and time on this (Panu AND Thomas :-)),
-Aaron


[dpdk-dev] [RFC] vhost user: add error handling for fd > 1023

2016-03-30 Thread Xie, Huawei
On 3/18/2016 5:15 PM, Patrik Andersson wrote:
> Protect against DPDK crash when allocation of listen fd >= 1023.
> For events on fd:s >1023, the current implementation will trigger
> an abort due to access outside of allocated bit mask.
>
> Corrections would include:
>
>   * Match fdset_add() signature in fd_man.c to fd_man.h
>   * Handling of return codes from fdset_add()
>   * Addition of check of fd number in fdset_add_fd()
>
> ---
>
> The rationale behind the suggested code change is that,
> fdset_event_dispatch() could attempt access outside of the FD_SET
> bitmask if there is an event on a file descriptor that in turn
> looks up a virtio file descriptor with a value > 1023.
> Such an attempt will lead to an abort() and a restart of any
> vswitch using DPDK.
>
> A discussion topic exist in the ovs-discuss mailing list that can
> provide a little more background:
>
> http://openvswitch.org/pipermail/discuss/2016-February/020243.html

Thanks for catching this. Could you give more details on how to
accumulating fds?
The buggy code is based on the fact that FD_SETSIZE limits the number of
file descriptors, which might be true on Windows. However from the
manual, it says clearly it limits the value of file descriptors.
The use of select is for portability. I have been wondering if it is
truly that important. Use poll could simplify the code a bit, for
example we need add timeout to select so that another thread could
insert/remove a fd into/from the monitored list.

Any comments on using poll/epoll?

>
> Signed-off-by: Patrik Andersson 
> ---
>  lib/librte_vhost/vhost_user/fd_man.c | 11 ++-
>  lib/librte_vhost/vhost_user/vhost-net-user.c | 22 --
>  2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/lib/librte_vhost/vhost_user/fd_man.c 
> b/lib/librte_vhost/vhost_user/fd_man.c
> index 087aaed..c691339 100644
> --- a/lib/librte_vhost/vhost_user/fd_man.c
> +++ b/lib/librte_vhost/vhost_user/fd_man.c
> @@ -71,20 +71,22 @@ fdset_find_free_slot(struct fdset *pfdset)
>   return fdset_find_fd(pfdset, -1);
>  }
>  
> -static void
> +static int
>  fdset_add_fd(struct fdset  *pfdset, int idx, int fd,
>   fd_cb rcb, fd_cb wcb, void *dat)
>  {
>   struct fdentry *pfdentry;
>  
> - if (pfdset == NULL || idx >= MAX_FDS)

seems we had better change the definition of MAX_FDS and
MAX_VHOST_SERVER to FD_SETSIZE or add some build time check.

> - return;
> + if (pfdset == NULL || idx >= MAX_FDS || fd >= FD_SETSIZE)
> + return -1;
>  
>   pfdentry = >fd[idx];
>   pfdentry->fd = fd;
>   pfdentry->rcb = rcb;
>   pfdentry->wcb = wcb;
>   pfdentry->dat = dat;
> +
> + return 0;
>  }
>  
>  /**
> @@ -150,12 +152,11 @@ fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, 
> fd_cb wcb, void *dat)
>  
>   /* Find a free slot in the list. */
>   i = fdset_find_free_slot(pfdset);
> - if (i == -1) {
> + if (i == -1 || fdset_add_fd(pfdset, i, fd, rcb, wcb, dat) < 0) {
>   pthread_mutex_unlock(>fd_mutex);
>   return -2;
>   }
>  
> - fdset_add_fd(pfdset, i, fd, rcb, wcb, dat);
>   pfdset->num++;
>  
>   pthread_mutex_unlock(>fd_mutex);
> diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c 
> b/lib/librte_vhost/vhost_user/vhost-net-user.c
> index df2bd64..778851d 100644
> --- a/lib/librte_vhost/vhost_user/vhost-net-user.c
> +++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
> @@ -288,6 +288,7 @@ vserver_new_vq_conn(int fd, void *dat, __rte_unused int 
> *remove)
>   int fh;
>   struct vhost_device_ctx vdev_ctx = { (pid_t)0, 0 };
>   unsigned int size;
> + int ret;
>  
>   conn_fd = accept(fd, NULL, NULL);
>   RTE_LOG(INFO, VHOST_CONFIG,
> @@ -317,8 +318,15 @@ vserver_new_vq_conn(int fd, void *dat, __rte_unused int 
> *remove)
>  
>   ctx->vserver = vserver;
>   ctx->fh = fh;
> - fdset_add(_vhost_server.fdset,
> + ret = fdset_add(_vhost_server.fdset,
>   conn_fd, vserver_message_handler, NULL, ctx);
> + if (ret < 0) {
> + free(ctx);
> + close(conn_fd);
> + RTE_LOG(ERR, VHOST_CONFIG,
> + "failed to add fd %d into vhost server fdset\n",
> + conn_fd);
> + }
>  }
>  
>  /* callback when there is message on the connfd */
> @@ -453,6 +461,7 @@ int
>  rte_vhost_driver_register(const char *path)
>  {
>   struct vhost_server *vserver;
> + int ret;
>  
>   pthread_mutex_lock(_vhost_server.server_mutex);
>  
> @@ -478,8 +487,17 @@ rte_vhost_driver_register(const char *path)
>  
>   vserver->path = strdup(path);
>  
> - fdset_add(_vhost_server.fdset, vserver->listenfd,
> + ret = fdset_add(_vhost_server.fdset, vserver->listenfd,
>   vserver_new_vq_conn, NULL, vserver);
> + if (ret < 0) {
> + pthread_mutex_unlock(_vhost_server.server_mutex);
> + RTE_LOG(ERR, 

[dpdk-dev] Issue with binding NIC of type 82545EM to dpdk

2016-03-30 Thread Al Patel
Hi Lu,

I do see the module there, but seems it is built against incorrect kernel
version and thus it is not loading:

# insmod ./dpdk/kmod/igb_uio.ko
insmod: ERROR: could not insert module ./dpdk/kmod/igb_uio.ko: Invalid
module format


dmesg:

[54394.611703] igb_uio: version magic '4.2.3-300.fc23.x86_64 SMP mod_unload
' should be '4.4.6-300.fc23.x86_64 SMP mod_unload '

uname -a:

# uname -a
Linux kube-node1 4.4.6-300.fc23.x86_64 #1 SMP Wed Mar 16 22:10:37 UTC 2016
x86_64 x86_64 x86_64 GNU/Linux


I removed that kernel source and rebuild it - now it is loading and I am
able to find the device to igb_uio and
do see /dev/uio0 created.

I do see the device in dpdk now.

Thanks much.
-a


On Tue, Mar 29, 2016 at 8:51 PM, Lu, Wenzhuo  wrote:

> Hi Al,
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Al Patel
> > Sent: Wednesday, March 30, 2016 1:14 AM
> > To: Yigit, Ferruh
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] Issue with binding NIC of type 82545EM to dpdk
> >
> > Ferruh,
> >
> > On my fedora23, I don't have igb_uio
> >
> > # modprobe igb_uio
> > modprobe: FATAL: Module igb_uio not found in directory
> > /lib/modules/4.4.6-300.fc23.x86_64
> After compiling dpdk, normally you can find igb_uio here,
> dpdk/x86_64-native-linuxapp-gcc/kmod/igb_uio.ko.
> Please refer http://www.dpdk.org/doc/guides/linux_gsg/build_dpdk.html.
>


[dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

2016-03-30 Thread Burakov, Anatoly
Hi Mauricio,

You?re not missing anything. It would be done this way, if the memzone 
parameter wasn?t const. But it is const, so we have to find the memzone in 
config to edit it.

Thanks,
Anatoly

From: Mauricio V?squez [mailto:mauricio.vasquezber...@studenti.polito.it]
Sent: Wednesday, March 30, 2016 9:22 AM
To: dev at dpdk.org
Cc: Gonzalez Monroy, Sergio ; Richardson, 
Bruce ; Burakov, Anatoly 
Subject: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

Dear All,
I was looking at that patch, I can understand its functionality but not its 
implementation..
Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would not 
be sufficient? After all, the goal is to mark the memzone as used by ivshmem to 
forbid  freeing it.
Please corrected if I am missing something.
Thank you,
Mauricio Vasquez,


[dpdk-dev] Issue with binding NIC of type 82545EM to dpdk

2016-03-30 Thread Lu, Wenzhuo
Hi Al,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Al Patel
> Sent: Wednesday, March 30, 2016 1:14 AM
> To: Yigit, Ferruh
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] Issue with binding NIC of type 82545EM to dpdk
> 
> Ferruh,
> 
> On my fedora23, I don't have igb_uio
> 
> # modprobe igb_uio
> modprobe: FATAL: Module igb_uio not found in directory
> /lib/modules/4.4.6-300.fc23.x86_64
After compiling dpdk, normally you can find igb_uio here, 
dpdk/x86_64-native-linuxapp-gcc/kmod/igb_uio.ko.
Please refer http://www.dpdk.org/doc/guides/linux_gsg/build_dpdk.html.


[dpdk-dev] [PATCH] scripts: check commit formatting

2016-03-30 Thread Thomas Monjalon
The git messages have three parts:
1/ the headline
2/ the explanations
3/ the footer tags

The headline helps to quickly browse an history or catch instantly the
purpose of a commit. Making it short with some consistent wording
allows to easily parse it or match some patterns.

The explanations must give some keys like the reason of the change.
Nothing can be automatically checked for this part.

The footer contains some tags to find the origin of a bug or who
was working on it.

This script is doing some basic checks on parts 1 and 3.

Signed-off-by: Thomas Monjalon 
---
 scripts/check-git-log.sh | 119 +++
 1 file changed, 119 insertions(+)
 create mode 100755 scripts/check-git-log.sh

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
new file mode 100755
index 000..483abcb
--- /dev/null
+++ b/scripts/check-git-log.sh
@@ -0,0 +1,119 @@
+#! /bin/sh
+
+# BSD LICENSE
+#
+# Copyright 2016 6WIND S.A.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+#   * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#   * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in
+# the documentation and/or other materials provided with the
+# distribution.
+#   * Neither the name of 6WIND S.A. nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# Check commit logs (headlines and references)
+#
+# If any doubt about the formatting, please check in the most recent history:
+#  git log --format='%>|(15)%cr   %s' --reverse | grep -i 
+
+range=${1:-origin/master..}
+
+headlines=$(git log --format='%s' $range)
+tags=$(git log --format='%b' $range | grep -i -e 'by *:' -e 'fix.*:')
+fixes=$(git log --format='%h %s' $range | grep -i ': *fix' | cut -d' ' -f1)
+
+# check headline format (spacing, no punctuation, no code)
+bad=$(echo "$headlines" | grep \
+   -e '' \
+   -e '^ ' \
+   -e ' $' \
+   -e '\.$' \
+   -e '[,;!?&|]' \
+   -e ':.*_' \
+   -e '^[^:]*$' \
+   -e ':[^ ]' \
+   -e ' :' \
+   | sed 's,^,\t,')
+[ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
+
+# check headline label for common typos
+bad=$(echo "$headlines" | grep \
+   -e '^example[:/]' \
+   -e '^apps/' \
+   -e '^testpmd' \
+   -e 'test-pmd' \
+   -e '^bond:' \
+   | sed 's,^,\t,')
+[ -z "$bad" ] || printf "Wrong headline label:\n$bad\n"
+
+# check headline lowercase for first words
+bad=$(echo "$headlines" | grep \
+   -e '^.*[A-Z].*:' \
+   -e ': *[A-Z]' \
+   | sed 's,^,\t,')
+[ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
+
+# check headline uppercase (Rx/Tx, VF, L2, MAC, Linux, ARM...)
+bad=$(echo "$headlines" | grep \
+   -e 'rx\|tx\|RX\|TX' \
+   -e '\<[pv]f\>' \
+   -e '\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   -e ':.*\' \
+   | sed 's,^,\t,')
+[ -z "$bad" ] || printf "Wrong headline lowercase:\n$bad\n"
+
+# check tags spelling
+bad=$(echo "$tags" |
+   grep -v 
'^\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by: [^,]* 
<.*@.*>$' |
+   grep -v '^Fixes: [0-9a-f]\{12\} (".*")$' |
+   sed 's,^.,\t&,')
+[ -z "$bad" ] || printf "Wrong tag:\n$bad\n"
+
+# check missing Fixes: tag
+bad=$(for fix in $fixes ; do
+   git log --format='%b' -1 $fix | grep -q '^Fixes: ' ||
+   git log --format='\t%s' -1 $fix
+done)
+[ -z "$bad" ] || printf "Missing 'Fixes' tag:\n$bad\n"
+
+# check Fixes: reference
+IFS='
+'
+fixtags=$(echo "$tags" | grep '^Fixes: ')
+bad=$(for fixtag in