[PATCH 0/6] DPDK 1.6.1 fixes

[PATCH 0/6] DPDK 1.6.1 fixes

[Stephen Hemminger]

These are patches for issues found during integration of 1.6.1
and also during product development on earlier release.

[PATCH 1/6] rte_mbuf: copy offload flags when doing attach/clone of mbuf

[Stephen Hemminger]

[-- Attachment #1: mbuf-ol-flags.patch --]
[-- Type: text/plain, Size: 659 bytes --]

rte_pktmbuf_attach copies the packet meta data but does not
copy the offload flags. This means that cloned packets lose
their offload settings such as vlan tag.

Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

--- a/lib/librte_mbuf/rte_mbuf.h	2013-12-06 08:39:43.000000000 -0800
+++ b/lib/librte_mbuf/rte_mbuf.h	2014-03-06 15:16:44.525440437 -0800
@@ -624,6 +624,7 @@ static inline void rte_pktmbuf_attach(st
 	mi->pkt.next = NULL;
 	mi->pkt.pkt_len = mi->pkt.data_len;
 	mi->pkt.nb_segs = 1;
+	mi->ol_flags = md->ol_flags;
 
 	__rte_mbuf_sanity_check(mi, RTE_MBUF_PKT, 1);
 	__rte_mbuf_sanity_check(md, RTE_MBUF_PKT, 0);

Re: [PATCH 1/6] rte_mbuf: copy offload flags when doing attach/clone of mbuf

[Thomas Monjalon]

07/03/2014 10:13, Stephen Hemminger:
> rte_pktmbuf_attach copies the packet meta data but does not
> copy the offload flags. This means that cloned packets lose
> their offload settings such as vlan tag.
> 
> Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

Acked and applied for version 1.6.0r2.

Thank you
-- 
Thomas

[PATCH 2/6] qos: use rte_zmalloc instead of memzone for allocation

[Stephen Hemminger]

[-- Attachment #1: sched-rte-malloc.patch --]
[-- Type: text/plain, Size: 1875 bytes --]

Memory zone's are inflexible and can not be destroyed.
The size is fixed when initially created therefor QoS parameters
could not be modified at run time, because table size for a subport
might change.

Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

--- a/lib/librte_sched/rte_sched.c	2013-08-02 03:41:05.000000000 -0700
+++ b/lib/librte_sched/rte_sched.c	2014-03-06 15:18:37.094154057 -0800
@@ -37,7 +37,7 @@
 #include <rte_common.h>
 #include <rte_log.h>
 #include <rte_memory.h>
-#include <rte_memzone.h>
+#include <rte_malloc.h>
 #include <rte_cycles.h>
 #include <rte_prefetch.h>
 #include <rte_branch_prediction.h>
@@ -613,7 +613,6 @@ struct rte_sched_port *
 rte_sched_port_config(struct rte_sched_port_params *params)
 {
 	struct rte_sched_port *port = NULL;
-	const struct rte_memzone *mz = NULL;
 	uint32_t mem_size, bmp_mem_size, n_queues_per_port, i;
 	
 	/* Check user parameters. Determine the amount of memory to allocate */
@@ -623,21 +622,10 @@ rte_sched_port_config(struct rte_sched_p
 	}
 	
 	/* Allocate memory to store the data structures */
-	mz = rte_memzone_lookup(params->name);
-	if (mz) {
-		/* Use existing memzone, provided that its size is big enough */
-		if (mz->len < mem_size) {
-			return NULL;
-		}
-	} else {
-		/* Create new memzone */
-		mz = rte_memzone_reserve(params->name, mem_size, params->socket, 0);		
-		if (mz == NULL) {
-			return NULL;
-		}
+	port = rte_zmalloc("qos_params", mem_size, CACHE_LINE_SIZE);
+	if (port == NULL) {
+		return NULL;
 	}
-	memset(mz->addr, 0, mem_size);
-	port = (struct rte_sched_port *) mz->addr;
 
 	/* User parameters */
 	port->n_subports_per_port = params->n_subports_per_port;
@@ -716,9 +704,9 @@ rte_sched_port_free(struct rte_sched_por
 	if (port == NULL){
 		return;
 	}
+
 	rte_bitmap_free(port->bmp);
-	
-	return;
+	rte_free(port);
 }
 
 static void

Re: [PATCH 2/6] qos: use rte_zmalloc instead of memzone for allocation

[Thomas Monjalon]

07/03/2014 10:13, Stephen Hemminger:
> Memory zone's are inflexible and can not be destroyed.
> The size is fixed when initially created therefor QoS parameters
> could not be modified at run time, because table size for a subport
> might change.
> 
> Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

This change would probably deserve a description of the use case.
Please explain in the log and/or comments why you need to increase this size.

-- 
Thomas

[PATCH 3/6] rte_jhash: make arg to rte_jhash2 const

[Stephen Hemminger]

[-- Attachment #1: jhash2-const.patch --]
[-- Type: text/plain, Size: 596 bytes --]

The argument to rte_jhash2() is not changed.

Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

diff --git a/lib/librte_hash/rte_jhash.h b/lib/librte_hash/rte_jhash.h
index e137d13..ae91bf7 100644
--- a/lib/librte_hash/rte_jhash.h
+++ b/lib/librte_hash/rte_jhash.h
@@ -149,7 +149,7 @@ rte_jhash(const void *key, uint32_t length, uint32_t initval)
  *   Calculated hash value.
  */
 static inline uint32_t
-rte_jhash2(uint32_t *k, uint32_t length, uint32_t initval)
+rte_jhash2(const uint32_t *k, uint32_t length, uint32_t initval)
 {
 	uint32_t a, b, c, len;
 

Re: [PATCH 3/6] rte_jhash: make arg to rte_jhash2 const

[Thomas Monjalon]

07/03/2014 10:13, Stephen Hemminger:
> The argument to rte_jhash2() is not changed.
> 
> Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

Acked and applied for version 1.6.0r2.

Thank you
-- 
Thomas

[PATCH 4/6] mempool: use GCC push/pop

[Stephen Hemminger]

[-- Attachment #1: mempool-opts.patch --]
[-- Type: text/plain, Size: 964 bytes --]

The include file should not change the GCC compile options for
the whole file being compiled, but only for the one inline function
that needs it. Using the push_options/pop_options fixes this.

Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

--- a/lib/librte_mempool/rte_mempool.h	2014-01-23 09:15:55.000000000 -0800
+++ b/lib/librte_mempool/rte_mempool.h	2014-03-06 15:19:56.551238180 -0800
@@ -314,6 +314,7 @@ static inline void __mempool_write_trail
  */
 #ifdef RTE_LIBRTE_MEMPOOL_DEBUG
 #ifndef __INTEL_COMPILER
+#pragma GCC push_options
 #pragma GCC diagnostic ignored "-Wcast-qual"
 #endif
 static inline void __mempool_check_cookies(const struct rte_mempool *mp,
@@ -380,7 +381,7 @@ static inline void __mempool_check_cooki
 	}
 }
 #ifndef __INTEL_COMPILER
-#pragma GCC diagnostic error "-Wcast-qual"
+#pragma GCC pop_options
 #endif
 #else
 #define __mempool_check_cookies(mp, obj_table_const, n, free) do {} while(0)

Re: [PATCH 4/6] mempool: use GCC push/pop

[Thomas Monjalon]

07/03/2014 10:13, Stephen Hemminger:
> The include file should not change the GCC compile options for
> the whole file being compiled, but only for the one inline function
> that needs it. Using the push_options/pop_options fixes this.
> 
> Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

Acked and applied for version 1.6.0r2.

Thank you
-- 
Thomas

[PATCH 5/6] xen: dont create dependency on cmdline library

[Stephen Hemminger]

[-- Attachment #1: xen-eth-aton.patch --]
[-- Type: text/plain, Size: 1593 bytes --]

The driver should not introduce an unnecessary dependency on the cmdline
code. We don't build that code since it is not used in our product.

Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

---
 lib/librte_pmd_xenvirt/rte_eth_xenvirt.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
index d844845..42a20ba 100644
--- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
+++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
@@ -53,8 +53,6 @@
 #include <rte_malloc.h>
 #include <rte_memcpy.h>
 #include <rte_string_fns.h>
-#include <cmdline_parse.h>
-#include <cmdline_parse_etheraddr.h>
 
 #include "rte_xen_lib.h"
 #include "virtqueue.h"
@@ -63,6 +61,9 @@
 #define VQ_DESC_NUM 256
 #define VIRTIO_MBUF_BURST_SZ 64
 
+/* defined in <netinet/ether.h> but that has conflicts with rte_ethdev.h */
+extern struct ether_addr *ether_aton(const char *);
+
 /* virtio_idx is increased after new device is created.*/
 static int virtio_idx = 0;
 
@@ -584,15 +585,14 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
 
 		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
 				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
-			if (cmdline_parse_etheraddr(NULL,
-							pair[1],
-							&dict->addr) < 0) {
+			struct ether_addr *ea = ether_aton(pair[1]);
+			if (ea == NULL) {
 				RTE_LOG(ERR, PMD,
 					"Invalid %s device ether address\n",
 					name);
 				goto err;
 			}
-
+			ether_addr_copy(ea, &dict->addr);
 			dict->addr_valid = 1;
 		}
 	}
-- 
1.7.10.4

Re: [PATCH 5/6] xen: dont create dependency on cmdline library

[Thomas Monjalon]

07/03/2014 10:13, Stephen Hemminger:
> The driver should not introduce an unnecessary dependency on the cmdline
> code. We don't build that code since it is not used in our product.

Your product is not the concern but I agree this dependency is unnecessary.
I'd just remove the comment about your product.

> +/* defined in <netinet/ether.h> but that has conflicts with rte_ethdev.h */
> +extern struct ether_addr *ether_aton(const char *);

It seems we should fix this conflict first.
I think the problem is about libc redefinitions in rte_ether.h.
Patches are welcome.

-- 
Thomas

[PATCH 6/6] ivshmem: fix errors identified by hardening

[Stephen Hemminger]

[-- Attachment #1: ivshmem-fortify.patch --]
[-- Type: text/plain, Size: 1104 bytes --]

Need to pass mode argument to open with O_CREAT.
Must check return value from ftruncate().

Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

diff --git a/lib/librte_eal/linuxapp/eal/eal_ivshmem.c b/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
index 6191fef..5f64873 100644
--- a/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
+++ b/lib/librte_eal/linuxapp/eal/eal_ivshmem.c
@@ -472,7 +472,7 @@ create_shared_config(void)
 	rte_snprintf(path, sizeof(path), IVSHMEM_CONFIG_PATH,
 			internal_config.hugefile_prefix);
 
-	fd = open(path, O_CREAT | O_RDWR);
+	fd = open(path, O_CREAT | O_RDWR, 0600);
 
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "Could not open %s: %s\n", path, strerror(errno));
@@ -486,7 +486,10 @@ create_shared_config(void)
 		return -1;
 	}
 
-	ftruncate(fd, sizeof(struct ivshmem_shared_config));
+	if (ftruncate(fd, sizeof(struct ivshmem_shared_config)) < 0) {
+		RTE_LOG(ERR, EAL, "ftruncate failed: %s\n", strerror(errno));
+		return -1;
+	}
 
 	ivshmem_config = mmap(NULL, sizeof(struct ivshmem_shared_config),
 			PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);

Re: [PATCH 6/6] ivshmem: fix errors identified by hardening

[Thomas Monjalon]

2014-03-07 10:13, Stephen Hemminger:
> Need to pass mode argument to open with O_CREAT.
> Must check return value from ftruncate().
> 
> Signed-off-by: Stephen Hemminger <shemming-43mecJUBy8ZBDgjK7y7TUQ@public.gmane.org>

Acked-by: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

It's applied for version 1.6.0r2.

Thanks
-- 
Thomas