[dpdk-dev] [PATCH 5/8] ACL: introduce RTE_ACL_MASKLEN_TO_BITMASK macro

2015-06-04 Thread Konstantin Ananyev
Introduce new RTE_ACL_MASKLEN_TO_BITMASK macro, that will be used
in several places inside librte_acl and it's UT.
Simplify iand cleanup build_trie() code a bit.

Signed-off-by: Konstantin Ananyev 
---
 lib/librte_acl/acl_bld.c | 16 +++-
 lib/librte_acl/rte_acl.h |  3 +++
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index 4bcf637..e144503 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -1262,19 +1262,9 @@ build_trie(struct acl_build_context *context, struct 
rte_acl_build_rule *head,
 * all higher bits.
 */
uint64_t mask;
-
-   if (fld->mask_range.u32 == 0) {
-   mask = 0;
-
-   /*
-* arithmetic right shift for the length of
-* the mask less the msb.
-*/
-   } else {
-   mask = -1 <<
-   (rule->config->defs[n].size *
-   CHAR_BIT - fld->mask_range.u32);
-   }
+   mask = RTE_ACL_MASKLEN_TO_BITMASK(
+   fld->mask_range.u32,
+   rule->config->defs[n].size);

/* gen a mini-trie for this field */
merge = acl_gen_mask_trie(context,
diff --git a/lib/librte_acl/rte_acl.h b/lib/librte_acl/rte_acl.h
index 8d9bbe5..bd8f892 100644
--- a/lib/librte_acl/rte_acl.h
+++ b/lib/librte_acl/rte_acl.h
@@ -122,6 +122,9 @@ enum {

 #defineRTE_ACL_INVALID_USERDATA0

+#defineRTE_ACL_MASKLEN_TO_BITMASK(v, s)\
+((v) == 0 ? (v) : (typeof(v))((uint64_t)-1 << ((s) * CHAR_BIT - (v
+
 /**
  * Miscellaneous data for ACL rule.
  */
-- 
1.8.5.3



[dpdk-dev] [PATCH 6/8] ACL: cleanup remove unused code from acl_bld.c

2015-06-04 Thread Konstantin Ananyev
Signed-off-by: Konstantin Ananyev 
---
 lib/librte_acl/acl_bld.c | 310 ---
 1 file changed, 310 deletions(-)

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index e144503..83669ac 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -120,10 +120,6 @@ static int acl_merge_trie(struct acl_build_context 
*context,
struct rte_acl_node *node_a, struct rte_acl_node *node_b,
uint32_t level, struct rte_acl_node **node_c);

-static int acl_merge(struct acl_build_context *context,
-   struct rte_acl_node *node_a, struct rte_acl_node *node_b,
-   int move, int a_subset, int level);
-
 static void
 acl_deref_ptr(struct acl_build_context *context,
struct rte_acl_node *node, int index);
@@ -415,58 +411,6 @@ acl_intersect_type(const struct rte_acl_bitset *a_bits,
 }

 /*
- * Check if all bits in the bitset are on
- */
-static int
-acl_full(struct rte_acl_node *node)
-{
-   uint32_t n;
-   bits_t all_bits = -1;
-
-   for (n = 0; n < RTE_ACL_BIT_SET_SIZE; n++)
-   all_bits &= node->values.bits[n];
-   return all_bits == -1;
-}
-
-/*
- * Check if all bits in the bitset are off
- */
-static int
-acl_empty(struct rte_acl_node *node)
-{
-   uint32_t n;
-
-   if (node->ref_count == 0) {
-   for (n = 0; n < RTE_ACL_BIT_SET_SIZE; n++) {
-   if (0 != node->values.bits[n])
-   return 0;
-   }
-   return 1;
-   } else {
-   return 0;
-   }
-}
-
-/*
- * Compute intersection of A and B
- * return 1 if there is an intersection else 0.
- */
-static int
-acl_intersect(struct rte_acl_bitset *a_bits,
-   struct rte_acl_bitset *b_bits,
-   struct rte_acl_bitset *intersect)
-{
-   uint32_t n;
-   bits_t all_bits = 0;
-
-   for (n = 0; n < RTE_ACL_BIT_SET_SIZE; n++) {
-   intersect->bits[n] = a_bits->bits[n] & b_bits->bits[n];
-   all_bits |= intersect->bits[n];
-   }
-   return all_bits != 0;
-}
-
-/*
  * Duplicate a node
  */
 static struct rte_acl_node *
@@ -534,63 +478,6 @@ acl_deref_ptr(struct acl_build_context *context,
 }

 /*
- * Exclude bitset from a node pointer
- * returns  0 if poiter was deref'd
- *  1 otherwise.
- */
-static int
-acl_exclude_ptr(struct acl_build_context *context,
-   struct rte_acl_node *node,
-   int index,
-   struct rte_acl_bitset *b_bits)
-{
-   int retval = 1;
-
-   /*
-* remove bitset from node pointer and deref
-* if the bitset becomes empty.
-*/
-   if (!acl_exclude(>ptrs[index].values,
-   >ptrs[index].values,
-   b_bits)) {
-   acl_deref_ptr(context, node, index);
-   node->ptrs[index].ptr = NULL;
-   retval = 0;
-   }
-
-   /* exclude bits from the composite bits for the node */
-   acl_exclude(>values, >values, b_bits);
-   return retval;
-}
-
-/*
- * Remove a bitset from src ptr and move remaining ptr to dst
- */
-static int
-acl_move_ptr(struct acl_build_context *context,
-   struct rte_acl_node *dst,
-   struct rte_acl_node *src,
-   int index,
-   struct rte_acl_bitset *b_bits)
-{
-   int rc;
-
-   if (b_bits != NULL)
-   if (!acl_exclude_ptr(context, src, index, b_bits))
-   return 0;
-
-   /* add src pointer to dst node */
-   rc = acl_add_ptr(context, dst, src->ptrs[index].ptr,
-   >ptrs[index].values);
-   if (rc < 0)
-   return rc;
-
-   /* remove ptr from src */
-   acl_exclude_ptr(context, src, index, >ptrs[index].values);
-   return 1;
-}
-
-/*
  * acl_exclude rte_acl_bitset from src and copy remaining pointer to dst
  */
 static int
@@ -650,203 +537,6 @@ acl_compact_node_ptrs(struct rte_acl_node *node_a)
}
 }

-/*
- * acl_merge helper routine.
- */
-static int
-acl_merge_intersect(struct acl_build_context *context,
-   struct rte_acl_node *node_a, uint32_t idx_a,
-   struct rte_acl_node *node_b, uint32_t idx_b,
-   int next_move, int level,
-   struct rte_acl_bitset *intersect_ptr)
-{
-   struct rte_acl_node *node_c;
-
-   /* Duplicate A for intersection */
-   node_c = acl_dup_node(context, node_a->ptrs[idx_a].ptr);
-
-   /* Remove intersection from A */
-   acl_exclude_ptr(context, node_a, idx_a, intersect_ptr);
-
-   /*
-* Added link from A to C for all transitions
-* in the intersection
-*/
-   if (acl_add_ptr(context, node_a, node_c, intersect_ptr) < 0)
-   return -1;
-
-   /* merge B->node into C */
-   return acl_merge(context, node_c, node_b->ptrs[idx_b].ptr, next_move,
-   0, level + 1);
-}
-
-
-/*
- * Merge the children of nodes A and B together.
- *
- * if match node
- * For each category
- * node A 

[dpdk-dev] [PATCH 7/8] ACL: fix remove ambiguity between rules at UT

2015-06-04 Thread Konstantin Ananyev
Some test rules had equal priorityi for the same category.
That can causes an ambiguity in build trie and test results.
Specify different priority value for each rule from the same category.

Signed-off-by: Konstantin Ananyev 
---
 app/test/test_acl.h | 52 ++--
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/app/test/test_acl.h b/app/test/test_acl.h
index 4af457d..4e8ff34 100644
--- a/app/test/test_acl.h
+++ b/app/test/test_acl.h
@@ -105,7 +105,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets traveling to 192.168.0.0/16 */
{
.data = {.userdata = 1, .category_mask = 
ACL_ALLOW_MASK,
-   .priority = 2},
+   .priority = 230},
.dst_addr = IPv4(192,168,0,0),
.dst_mask_len = 16,
.src_port_low = 0,
@@ -116,7 +116,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets traveling to 192.168.1.0/24 */
{
.data = {.userdata = 2, .category_mask = 
ACL_ALLOW_MASK,
-   .priority = 3},
+   .priority = 330},
.dst_addr = IPv4(192,168,1,0),
.dst_mask_len = 24,
.src_port_low = 0,
@@ -127,7 +127,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets traveling to 192.168.1.50 */
{
.data = {.userdata = 3, .category_mask = 
ACL_DENY_MASK,
-   .priority = 2},
+   .priority = 230},
.dst_addr = IPv4(192,168,1,50),
.dst_mask_len = 32,
.src_port_low = 0,
@@ -140,7 +140,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets traveling from 10.0.0.0/8 */
{
.data = {.userdata = 4, .category_mask = 
ACL_ALLOW_MASK,
-   .priority = 2},
+   .priority = 240},
.src_addr = IPv4(10,0,0,0),
.src_mask_len = 8,
.src_port_low = 0,
@@ -151,7 +151,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets traveling from 10.1.1.0/24 */
{
.data = {.userdata = 5, .category_mask = 
ACL_ALLOW_MASK,
-   .priority = 3},
+   .priority = 340},
.src_addr = IPv4(10,1,1,0),
.src_mask_len = 24,
.src_port_low = 0,
@@ -162,7 +162,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets traveling from 10.1.1.1 */
{
.data = {.userdata = 6, .category_mask = 
ACL_DENY_MASK,
-   .priority = 2},
+   .priority = 240},
.src_addr = IPv4(10,1,1,1),
.src_mask_len = 32,
.src_port_low = 0,
@@ -175,7 +175,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets with lower 7 bytes of VLAN tag equal to 
0x64  */
{
.data = {.userdata = 7, .category_mask = 
ACL_ALLOW_MASK,
-   .priority = 2},
+   .priority = 260},
.vlan = 0x64,
.vlan_mask = 0x7f,
.src_port_low = 0,
@@ -186,7 +186,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets with VLAN tags that have 0x5 in them */
{
.data = {.userdata = 8, .category_mask = 
ACL_ALLOW_MASK,
-   .priority = 2},
+   .priority = 260},
.vlan = 0x5,
.vlan_mask = 0x5,
.src_port_low = 0,
@@ -197,7 +197,7 @@ struct rte_acl_ipv4vlan_rule acl_test_rules[] = {
/* matches all packets with VLAN tag 5 */
{

[dpdk-dev] [PATCH 1/8] ACL: fix invalid rule wildness calculation for RTE_ACL_FIELD_TYPE_BITMASK

2015-06-04 Thread Konstantin Ananyev
Signed-off-by: Konstantin Ananyev 
---
 lib/librte_acl/acl_bld.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index db23b7b..e2db9bf 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -1362,6 +1362,9 @@ acl_calc_wildness(struct rte_acl_build_rule *head,
for (n = 0; n < config->num_fields; n++) {

double wild = 0;
+   uint64_t msk_val =
+   RTE_LEN2MASK(CHAR_BIT * config->defs[n].size,
+   typeof(msk_val));
double size = CHAR_BIT * config->defs[n].size;
int field_index = config->defs[n].field_index;
const struct rte_acl_field *fld = rule->f->field +
@@ -1369,8 +1372,8 @@ acl_calc_wildness(struct rte_acl_build_rule *head,

switch (rule->config->defs[n].type) {
case RTE_ACL_FIELD_TYPE_BITMASK:
-   wild = (size - __builtin_popcount(
-   fld->mask_range.u8)) /
+   wild = (size - __builtin_popcountll(
+   fld->mask_range.u64 & msk_val)) /
size;
break;

-- 
1.8.5.3



[dpdk-dev] [PATCH 2/8] ACL: code cleanup - use global RTE_LEN2MASK macro

2015-06-04 Thread Konstantin Ananyev
Signed-off-by: Konstantin Ananyev 
---
 app/test-acl/main.c| 3 ++-
 lib/librte_acl/acl_bld.c   | 3 ++-
 lib/librte_acl/rte_acl.c   | 3 ++-
 lib/librte_acl/rte_acl.h   | 2 +-
 lib/librte_acl/rte_acl_osdep.h | 2 --
 5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/test-acl/main.c b/app/test-acl/main.c
index 524c43a..be3d773 100644
--- a/app/test-acl/main.c
+++ b/app/test-acl/main.c
@@ -739,7 +739,8 @@ add_cb_rules(FILE *f, struct rte_acl_ctx *ctx)
return rc;
}

-   v.data.category_mask = LEN2MASK(RTE_ACL_MAX_CATEGORIES);
+   v.data.category_mask = RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES,
+   typeof(v.data.category_mask));
v.data.priority = RTE_ACL_MAX_PRIORITY - n;
v.data.userdata = n;

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index e2db9bf..19a4178 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -1772,7 +1772,8 @@ acl_bld(struct acl_build_context *bcx, struct rte_acl_ctx 
*ctx,
bcx->pool.alignment = ACL_POOL_ALIGN;
bcx->pool.min_alloc = ACL_POOL_ALLOC_MIN;
bcx->cfg = *cfg;
-   bcx->category_mask = LEN2MASK(bcx->cfg.num_categories);
+   bcx->category_mask = RTE_LEN2MASK(bcx->cfg.num_categories,
+   typeof(bcx->category_mask));
bcx->node_max = node_max;

rc = sigsetjmp(bcx->pool.fail, 0);
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index b6ddeeb..a54d531 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -271,7 +271,8 @@ acl_add_rules(struct rte_acl_ctx *ctx, const void *rules, 
uint32_t num)
 static int
 acl_check_rule(const struct rte_acl_rule_data *rd)
 {
-   if ((rd->category_mask & LEN2MASK(RTE_ACL_MAX_CATEGORIES)) == 0 ||
+   if ((RTE_LEN2MASK(RTE_ACL_MAX_CATEGORIES, typeof(rd->category_mask)) &
+   rd->category_mask) == 0 ||
rd->priority > RTE_ACL_MAX_PRIORITY ||
rd->priority < RTE_ACL_MIN_PRIORITY ||
rd->userdata == RTE_ACL_INVALID_USERDATA)
diff --git a/lib/librte_acl/rte_acl.h b/lib/librte_acl/rte_acl.h
index 3a93730..8d9bbe5 100644
--- a/lib/librte_acl/rte_acl.h
+++ b/lib/librte_acl/rte_acl.h
@@ -115,7 +115,7 @@ struct rte_acl_field {

 enum {
RTE_ACL_TYPE_SHIFT = 29,
-   RTE_ACL_MAX_INDEX = LEN2MASK(RTE_ACL_TYPE_SHIFT),
+   RTE_ACL_MAX_INDEX = RTE_LEN2MASK(RTE_ACL_TYPE_SHIFT, uint32_t),
RTE_ACL_MAX_PRIORITY = RTE_ACL_MAX_INDEX,
RTE_ACL_MIN_PRIORITY = 0,
 };
diff --git a/lib/librte_acl/rte_acl_osdep.h b/lib/librte_acl/rte_acl_osdep.h
index 81fdefb..41f7e3d 100644
--- a/lib/librte_acl/rte_acl_osdep.h
+++ b/lib/librte_acl/rte_acl_osdep.h
@@ -56,8 +56,6 @@
  * Common defines.
  */

-#defineLEN2MASK(ln)((uint32_t)(((uint64_t)1 << (ln)) - 1))
-
 #define DIM(x) RTE_DIM(x)

 #include 
-- 
1.8.5.3



[dpdk-dev] [PATCH 4/8] ACL: fix rebuilding a trie for subset of rules

2015-06-04 Thread Konstantin Ananyev
When rebuilding a trie for limited rule-set,
don't try to split the rule-set even further.

Signed-off-by: Konstantin Ananyev 
---
 lib/librte_acl/acl_bld.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index 8315d84..4bcf637 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -97,6 +97,7 @@ struct acl_build_context {
struct rte_acl_build_rule *build_rules;
struct rte_acl_config cfg;
int32_t   node_max;
+   int32_t   cur_node_max;
uint32_t  node;
uint32_t  num_nodes;
uint32_t  category_mask;
@@ -1337,7 +1338,7 @@ build_trie(struct acl_build_context *context, struct 
rte_acl_build_rule *head,
return NULL;

node_count = context->num_nodes - node_count;
-   if (node_count > context->node_max) {
+   if (node_count > context->cur_node_max) {
*last = prev;
return trie;
}
@@ -1536,7 +1537,7 @@ acl_build_index(const struct rte_acl_config *config, 
uint32_t *data_index)
 static struct rte_acl_build_rule *
 build_one_trie(struct acl_build_context *context,
struct rte_acl_build_rule *rule_sets[RTE_ACL_MAX_TRIES],
-   uint32_t n)
+   uint32_t n, int32_t node_max)
 {
struct rte_acl_build_rule *last;
struct rte_acl_config *config;
@@ -1553,6 +1554,8 @@ build_one_trie(struct acl_build_context *context,
context->data_indexes[n]);
context->tries[n].data_index = context->data_indexes[n];

+   context->cur_node_max = node_max;
+
context->bld_tries[n].trie = build_trie(context, rule_sets[n],
, >tries[n].count);

@@ -1587,7 +1590,7 @@ acl_build_tries(struct acl_build_context *context,

num_tries = n + 1;

-   last = build_one_trie(context, rule_sets, n);
+   last = build_one_trie(context, rule_sets, n, context->node_max);
if (context->bld_tries[n].trie == NULL) {
RTE_LOG(ERR, ACL, "Build of %u-th trie failed\n", n);
return -ENOMEM;
@@ -1618,8 +1621,11 @@ acl_build_tries(struct acl_build_context *context,
head = head->next)
head->config = config;

-   /* Rebuild the trie for the reduced rule-set. */
-   last = build_one_trie(context, rule_sets, n);
+   /*
+* Rebuild the trie for the reduced rule-set.
+* Don't try to split it any further.
+*/
+   last = build_one_trie(context, rule_sets, n, INT32_MAX);
if (context->bld_tries[n].trie == NULL || last != NULL) {
RTE_LOG(ERR, ACL, "Build of %u-th trie failed\n", n);
return -ENOMEM;
-- 
1.8.5.3



[dpdk-dev] [PATCH 3/8] ACL: add function to check rte_acl_build() input parameters

2015-06-04 Thread Konstantin Ananyev
Move check for build parameters into a separate function.
Simplify acl_calc_wildness() function.

Signed-off-by: Konstantin Ananyev 
---
 lib/librte_acl/acl_bld.c | 107 ---
 1 file changed, 54 insertions(+), 53 deletions(-)

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index 19a4178..8315d84 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -1350,7 +1350,7 @@ build_trie(struct acl_build_context *context, struct 
rte_acl_build_rule *head,
return trie;
 }

-static int
+static void
 acl_calc_wildness(struct rte_acl_build_rule *head,
const struct rte_acl_config *config)
 {
@@ -1362,10 +1362,10 @@ acl_calc_wildness(struct rte_acl_build_rule *head,
for (n = 0; n < config->num_fields; n++) {

double wild = 0;
-   uint64_t msk_val =
-   RTE_LEN2MASK(CHAR_BIT * config->defs[n].size,
+   uint32_t bit_len = CHAR_BIT * config->defs[n].size;
+   uint64_t msk_val = RTE_LEN2MASK(bit_len,
typeof(msk_val));
-   double size = CHAR_BIT * config->defs[n].size;
+   double size = bit_len;
int field_index = config->defs[n].field_index;
const struct rte_acl_field *fld = rule->f->field +
field_index;
@@ -1382,54 +1382,15 @@ acl_calc_wildness(struct rte_acl_build_rule *head,
break;

case RTE_ACL_FIELD_TYPE_RANGE:
-   switch (rule->config->defs[n].size) {
-   case sizeof(uint8_t):
-   wild = ((double)fld->mask_range.u8 -
-   fld->value.u8) / UINT8_MAX;
-   break;
-   case sizeof(uint16_t):
-   wild = ((double)fld->mask_range.u16 -
-   fld->value.u16) / UINT16_MAX;
-   break;
-   case sizeof(uint32_t):
-   wild = ((double)fld->mask_range.u32 -
-   fld->value.u32) / UINT32_MAX;
-   break;
-   case sizeof(uint64_t):
-   wild = ((double)fld->mask_range.u64 -
-   fld->value.u64) / UINT64_MAX;
-   break;
-   default:
-   RTE_LOG(ERR, ACL,
-   "%s(rule: %u) invalid %u-th "
-   "field, type: %hhu, "
-   "unknown size: %hhu\n",
-   __func__,
-   rule->f->data.userdata,
-   n,
-   rule->config->defs[n].type,
-   rule->config->defs[n].size);
-   return -EINVAL;
-   }
+   wild = (fld->mask_range.u64 & msk_val) -
+   (fld->value.u64 & msk_val);
+   wild = wild / msk_val;
break;
-
-   default:
-   RTE_LOG(ERR, ACL,
-   "%s(rule: %u) invalid %u-th "
-   "field, unknown type: %hhu\n",
-   __func__,
-   rule->f->data.userdata,
-   n,
-   rule->config->defs[n].type);
-   return -EINVAL;
-
}

rule->wildness[field_index] = (uint32_t)(wild * 100);
}
}
-
-   return 0;
 }

 static void
@@ -1602,7 +1563,6 @@ static int
 acl_build_tries(struct acl_build_context *context,
struct rte_acl_build_rule *head)
 {
-   int32_t rc;
uint32_t n, num_tries;
struct rte_acl_config *config;
struct rte_acl_build_rule *last;
@@ -1621,9 +1581,7 @@ acl_build_tries(struct acl_build_context *context,
context->tries[0].type = RTE_ACL_FULL_TRIE;

/* calc wildness of each field of each rule */
-   rc = acl_calc_wildness(head, config);
-   if (rc != 0)
-   return rc;
+   acl_calc_wildness(head, config);

for (n = 0;; 

[dpdk-dev] [PATCH 0/8] ACL: various fixes and cleanups

2015-06-04 Thread Konstantin Ananyev
This patch-set is based on:
[PATCHv2 0/3] ACL: Fix bug in acl_merge_trie() and add a new test-case for it 
to the UT.

Konstantin Ananyev (8):
 ACL: fix invalid rule wildness calculation for RTE_ACL_FIELD_TYPE_BITMASK
 ACL: code cleanup - use global RTE_LEN2MASK macro
 ACL: add function to check rte_acl_build() input parameters
 ACL: fix rebuilding a trie for subset of rules
 ACL: introduce RTE_ACL_MASKLEN_TO_BITMASK macro
 ACL: cleanup remove unused code from acl_bld.c
 ACL: fix remove ambiguity between rules at UT
 ACL: add new test-cases into UT

 app/test-acl/main.c|   3 +-
 app/test/test_acl.c| 431 +-
 app/test/test_acl.h|  52 ++---
 lib/librte_acl/acl_bld.c   | 455 +++--
 lib/librte_acl/rte_acl.c   |   3 +-
 lib/librte_acl/rte_acl.h   |   5 +-
 lib/librte_acl/rte_acl_osdep.h |   2 -
 7 files changed, 530 insertions(+), 421 deletions(-)

-- 
1.8.5.3



[dpdk-dev] [PATCH 8/8] ACL: add new test-cases into UT

2015-06-04 Thread Konstantin Ananyev
Add several new test cases for ACL to cover different build configurations.

Signed-off-by: Konstantin Ananyev 
---
 app/test/test_acl.c | 431 +++-
 1 file changed, 423 insertions(+), 8 deletions(-)

diff --git a/app/test/test_acl.c b/app/test/test_acl.c
index 6a032f9..3090246 100644
--- a/app/test/test_acl.c
+++ b/app/test/test_acl.c
@@ -47,6 +47,8 @@

 #define LEN RTE_ACL_MAX_CATEGORIES

+RTE_ACL_RULE_DEF(acl_ipv4vlan_rule, RTE_ACL_IPV4VLAN_NUM_FIELDS);
+
 struct rte_acl_param acl_param = {
.name = "acl_ctx",
.socket_id = SOCKET_ID_ANY,
@@ -62,6 +64,15 @@ struct rte_acl_ipv4vlan_rule acl_rule = {
.dst_port_high = UINT16_MAX,
 };

+const uint32_t ipv4_7tuple_layout[RTE_ACL_IPV4VLAN_NUM] = {
+   offsetof(struct ipv4_7tuple, proto),
+   offsetof(struct ipv4_7tuple, vlan),
+   offsetof(struct ipv4_7tuple, ip_src),
+   offsetof(struct ipv4_7tuple, ip_dst),
+   offsetof(struct ipv4_7tuple, port_src),
+};
+
+
 /* byteswap to cpu or network order */
 static void
 bswap_test_data(struct ipv4_7tuple *data, int len, int to_be)
@@ -195,13 +206,6 @@ test_classify_buid(struct rte_acl_ctx *acx,
const struct rte_acl_ipv4vlan_rule *rules, uint32_t num)
 {
int ret;
-   const uint32_t layout[RTE_ACL_IPV4VLAN_NUM] = {
-   offsetof(struct ipv4_7tuple, proto),
-   offsetof(struct ipv4_7tuple, vlan),
-   offsetof(struct ipv4_7tuple, ip_src),
-   offsetof(struct ipv4_7tuple, ip_dst),
-   offsetof(struct ipv4_7tuple, port_src),
-   };

/* add rules to the context */
ret = rte_acl_ipv4vlan_add_rules(acx, rules, num);
@@ -212,7 +216,8 @@ test_classify_buid(struct rte_acl_ctx *acx,
}

/* try building the context */
-   ret = rte_acl_ipv4vlan_build(acx, layout, RTE_ACL_MAX_CATEGORIES);
+   ret = rte_acl_ipv4vlan_build(acx, ipv4_7tuple_layout,
+   RTE_ACL_MAX_CATEGORIES);
if (ret != 0) {
printf("Line %i: Building ACL context failed!\n", __LINE__);
return ret;
@@ -412,6 +417,414 @@ test_build_ports_range(void)
return ret;
 }

+static void
+convert_rule(const struct rte_acl_ipv4vlan_rule *ri,
+   struct acl_ipv4vlan_rule *ro)
+{
+   ro->data = ri->data;
+
+   ro->field[RTE_ACL_IPV4VLAN_PROTO_FIELD].value.u8 = ri->proto;
+   ro->field[RTE_ACL_IPV4VLAN_VLAN1_FIELD].value.u16 = ri->vlan;
+   ro->field[RTE_ACL_IPV4VLAN_VLAN2_FIELD].value.u16 = ri->domain;
+   ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].value.u32 = ri->src_addr;
+   ro->field[RTE_ACL_IPV4VLAN_DST_FIELD].value.u32 = ri->dst_addr;
+   ro->field[RTE_ACL_IPV4VLAN_SRCP_FIELD].value.u16 = ri->src_port_low;
+   ro->field[RTE_ACL_IPV4VLAN_DSTP_FIELD].value.u16 = ri->dst_port_low;
+
+   ro->field[RTE_ACL_IPV4VLAN_PROTO_FIELD].mask_range.u8 = ri->proto_mask;
+   ro->field[RTE_ACL_IPV4VLAN_VLAN1_FIELD].mask_range.u16 = ri->vlan_mask;
+   ro->field[RTE_ACL_IPV4VLAN_VLAN2_FIELD].mask_range.u16 =
+   ri->domain_mask;
+   ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].mask_range.u32 =
+   ri->src_mask_len;
+   ro->field[RTE_ACL_IPV4VLAN_DST_FIELD].mask_range.u32 = ri->dst_mask_len;
+   ro->field[RTE_ACL_IPV4VLAN_SRCP_FIELD].mask_range.u16 =
+   ri->src_port_high;
+   ro->field[RTE_ACL_IPV4VLAN_DSTP_FIELD].mask_range.u16 =
+   ri->dst_port_high;
+}
+
+/*
+ * Convert IPV4 source and destination from RTE_ACL_FIELD_TYPE_MASK to
+ * RTE_ACL_FIELD_TYPE_BITMASK.
+ */
+static void
+convert_rule_1(const struct rte_acl_ipv4vlan_rule *ri,
+   struct acl_ipv4vlan_rule *ro)
+{
+   uint32_t v;
+
+   convert_rule(ri, ro);
+   v = ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].mask_range.u32;
+   ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].mask_range.u32 =
+   RTE_ACL_MASKLEN_TO_BITMASK(v, sizeof(v));
+   v = ro->field[RTE_ACL_IPV4VLAN_DST_FIELD].mask_range.u32;
+   ro->field[RTE_ACL_IPV4VLAN_DST_FIELD].mask_range.u32 =
+   RTE_ACL_MASKLEN_TO_BITMASK(v, sizeof(v));
+}
+
+/*
+ * Convert IPV4 source and destination from RTE_ACL_FIELD_TYPE_MASK to
+ * RTE_ACL_FIELD_TYPE_RANGE.
+ */
+static void
+convert_rule_2(const struct rte_acl_ipv4vlan_rule *ri,
+   struct acl_ipv4vlan_rule *ro)
+{
+   uint32_t hi, lo, mask;
+
+   convert_rule(ri, ro);
+
+   mask = ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].mask_range.u32;
+   mask = RTE_ACL_MASKLEN_TO_BITMASK(mask, sizeof(mask));
+   lo = ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].value.u32 & mask;
+   hi = lo + ~mask;
+   ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].value.u32 = lo;
+   ro->field[RTE_ACL_IPV4VLAN_SRC_FIELD].mask_range.u32 = hi;
+
+   mask = ro->field[RTE_ACL_IPV4VLAN_DST_FIELD].mask_range.u32;
+   mask = RTE_ACL_MASKLEN_TO_BITMASK(mask, sizeof(mask));
+   lo = 

[dpdk-dev] [PATCH RFC 0/2] vhost: numa aware allocation of virtio_net device and vhost virt queue

2015-06-04 Thread Long, Thomas
Acked-by: Tommy Long 

-Original Message-
From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Huawei Xie
Sent: Thursday, May 28, 2015 3:04 AM
To: dev at dpdk.org
Subject: [dpdk-dev] [PATCH RFC 0/2] vhost: numa aware allocation of virtio_net 
device and vhost virt queue

The virtio_net device and vhost virt queue should be allocated on the same numa 
node as vring descriptors.
When we firstly allocate the virtio_net device and vhost virt queue, we don't 
know the numa node of vring descriptors.
When we receive the VHOST_SET_VRING_ADDR message, we get the numa node of vring 
descriptors, so we will try to reallocate virtio_net and vhost virt queue to 
the same numa node.

Huawei Xie (2):
  use rte_malloc/free for virtio_net and virt_queue memory data allocation/free
  When we get the address of vring descriptor table, will try to reallocate 
virtio_net device and virtqueue to the same numa node.

 config/common_linuxapp|   1 +
 lib/librte_vhost/Makefile |   4 ++
 lib/librte_vhost/virtio-net.c | 112 ++
 mk/rte.app.mk |   3 ++
 4 files changed, 111 insertions(+), 9 deletions(-)

-- 
1.8.1.4



[dpdk-dev] [PATCH 0/6] query hash key size in byte

2015-06-04 Thread Helin Zhang
As different hardware has different hash key size, querying
it (in byte) per port was asked by users. Otherwise there is
no convenient way to know the size of hash key should be prepared.

Helin Zhang (6):
  ethdev: add an field for querying hash key size
  e1000: fill the hash key size
  fm10k: fill the hash key size
  i40e: fill the hash key size
  ixgbe: fill the hash key size
  app/testpmd: show the hash key size

 app/test-pmd/config.c | 2 ++
 drivers/net/e1000/igb_ethdev.c| 3 +++
 drivers/net/fm10k/fm10k_ethdev.c  | 1 +
 drivers/net/i40e/i40e_ethdev.c| 2 ++
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 drivers/net/ixgbe/ixgbe_ethdev.c  | 3 +++
 lib/librte_ether/rte_ethdev.h | 1 +
 7 files changed, 14 insertions(+)

-- 
1.9.3



[dpdk-dev] [PATCH 1/6] ethdev: add an field for querying hash key size

2015-06-04 Thread Helin Zhang
To support querying hash key size per port, an new field of
'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
hash key size in bytes.

Signed-off-by: Helin Zhang 
---
 lib/librte_ether/rte_ethdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..004b05a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -916,6 +916,7 @@ struct rte_eth_dev_info {
uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
+   uint8_t hash_key_size; /**< Hash key size in bytes */
uint16_t reta_size;
/**< Device redirection table size, the total number of entries. */
/** Bit mask of RSS offloads, the bit offset also means flow type */
-- 
1.9.3



[dpdk-dev] [PATCH 2/6] e1000: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/e1000/igb_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index e4b370d..7d388f3 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -68,6 +68,8 @@
 #define IGB_DEFAULT_TX_HTHRESH  0
 #define IGB_DEFAULT_TX_WTHRESH  0

+#define IGB_HKEY_MAX_INDEX 10
+
 /* Bit shift and mask */
 #define IGB_4_BIT_WIDTH  (CHAR_BIT / 2)
 #define IGB_4_BIT_MASK   RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t)
@@ -1377,6 +1379,7 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
/* Should not happen */
break;
}
+   dev_info->hash_key_size = IGB_HKEY_MAX_INDEX * sizeof(uint32_t);
dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
dev_info->flow_type_rss_offloads = IGB_RSS_OFFLOAD_ALL;

-- 
1.9.3



[dpdk-dev] [PATCH 3/6] fm10k: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/fm10k/fm10k_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 87852ed..bd626ce 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -767,6 +767,7 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
DEV_RX_OFFLOAD_UDP_CKSUM  |
DEV_RX_OFFLOAD_TCP_CKSUM;
dev_info->tx_offload_capa= 0;
+   dev_info->hash_key_size = FM10K_RSSRK_SIZE * sizeof(uint32_t);
dev_info->reta_size = FM10K_MAX_RSS_INDICES;

dev_info->default_rxconf = (struct rte_eth_rxconf) {
-- 
1.9.3



[dpdk-dev] [PATCH 4/6] i40e: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/i40e/i40e_ethdev.c| 2 ++
 drivers/net/i40e/i40e_ethdev_vf.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index da6c0b5..c699ddb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1540,6 +1540,8 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
DEV_TX_OFFLOAD_SCTP_CKSUM |
DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
DEV_TX_OFFLOAD_TCP_TSO;
+   dev_info->hash_key_size = (I40E_PFQF_HKEY_MAX_INDEX + 1) *
+   sizeof(uint32_t);
dev_info->reta_size = pf->hash_lut_size;
dev_info->flow_type_rss_offloads = I40E_RSS_OFFLOAD_ALL;

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index 9f92a2f..eb37d3a 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1641,6 +1641,8 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
dev_info->max_tx_queues = vf->vsi_res->num_queue_pairs;
dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
+   dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) *
+   sizeof(uint32_t);
dev_info->reta_size = ETH_RSS_RETA_SIZE_64;
dev_info->flow_type_rss_offloads = I40E_RSS_OFFLOAD_ALL;

-- 
1.9.3



[dpdk-dev] [PATCH 6/6] app/testpmd: show the hash key size

2015-06-04 Thread Helin Zhang
As querying hash key size in byte was supported, it can be shown
in testpmd after getting the device information if not zero.

Signed-off-by: Helin Zhang 
---
 app/test-pmd/config.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f788ed5..800756f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -361,6 +361,8 @@ port_infos_display(portid_t port_id)

memset(_info, 0, sizeof(dev_info));
rte_eth_dev_info_get(port_id, _info);
+   if (dev_info.hash_key_size > 0)
+   printf("Hash key size in bytes: %u\n", dev_info.hash_key_size);
if (dev_info.reta_size > 0)
printf("Redirection table size: %u\n", dev_info.reta_size);
if (!dev_info.flow_type_rss_offloads)
-- 
1.9.3



[dpdk-dev] [PATCH 5/6] ixgbe: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0d9f9b2..588ccc0 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -116,6 +116,8 @@

 #define IXGBE_QUEUE_STAT_COUNTERS (sizeof(hw_stats->qprc) / 
sizeof(hw_stats->qprc[0]))

+#define IXGBE_HKEY_MAX_INDEX 10
+
 static int eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev);
 static int  ixgbe_dev_configure(struct rte_eth_dev *dev);
 static int  ixgbe_dev_start(struct rte_eth_dev *dev);
@@ -2052,6 +2054,7 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
.txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS |
ETH_TXQ_FLAGS_NOOFFLOADS,
};
+   dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t);
dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL;
 }
-- 
1.9.3



[dpdk-dev] rte_eal_alarm_set() is affected by discontinuous jumps in the system time

2015-06-04 Thread Selmon Yang
Hi, Bruce,

Thank you very much for your positive response.
The attachment is the patch.
Please have a look.

Hi, Jay,

Thank you so much for your kindly reminding.
However, due to I really like the alarm functionalities not to be affected.
I guess I need to replace CLOCK_MONOTONIC_RAW with CLOCK_MONOTONIC
and do more tests to see if it also works for me.


2015-06-03 21:07 GMT+08:00 Jay Rolette :
>
> On Wed, Jun 3, 2015 at 7:54 AM, Bruce Richardson
>  wrote:
>>
>> On Wed, Jun 03, 2015 at 02:31:47PM +0800, Selmon Yang wrote:
>> > Hi,
>> >
>> > I found that, in dpdk 2.0, rte_eal_alarm_set() is affected by
>> > discontinuous jumps in the system time because eal_alarm_callback()
>> > and rte_eal_alarm_set() use gettimeofday() to get the current time.
>> >
>> > Here is what I encountered.
>> > I set up a rte eal alarm as below, and I like it to be triggered every
>> > second.
>> > #define USE_PER_S 1000 * 1000
>> > void my_alarm_cb(void *arg)
>> > {
>> > /* send heartbeat signal out, etc. */
>> >
>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL);
>> > return;
>> > }
>> >
>> > int main(void)
>> > {
>> > /* ..., do something */
>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL);
>> > /* ... do something else */
>> > }
>> >
>> > It works fine in most of time.
>> > However, if I change system time manually, it is possible that rte alarm
>> > function works out of my expectation.
>> > Suppose that current time is 11:00:00 AM, and eal_alarm_callback()
>> > is triggered because I executed
>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL) at 10:59:59 AM.
>> > eal_alarm_callback() gets the current time (11:00:00 AM)
>> > and calls my_alarm_cb() as below.
>> > while ((ap = LIST_FIRST(_list)) !=NULL &&
>> >   gettimeofday(, NULL) == 0 &&
>> >   (ap->time.tv_sec < now.tv_sec ||
>> > (ap->time.tv_sec == now.tv_sec &&
>> >   ap->time.tv_usec <=
>> > now.tv_usec))){
>> > ap->executing = 1;
>> > ap->executing_id = pthread_self();
>> > rte_spinlock_unlock(_list_lk);
>> >
>> > ap->cb_fn(ap->cb_arg);
>> >
>> > rte_spinlock_lock(_list_lk);
>> >
>> > LIST_REMOVE(ap, next);
>> > rte_free(ap);
>> > }
>> >
>> > In my_alarm_cb(), rte_eal_alarm_set() is called again.
>> > rte_eall_alarm_set() gets the current time (11:00:00 AM), plus 1 second,
>> > and adds the new alarm entry to alarm_list.
>> > /* use current time to calculate absolute time of alarm */
>> > gettimeofday(, NULL);
>> >
>> > new_alarm->cb_fn = cb_fn;
>> > new_alarm->cb_arg = cb_arg;
>> > new_alarm->time.tv_usec = (now.tv_usec + us) % US_PER_S;
>> > new_alarm->time.tv_sec = now.tv_sec + ((now.tv_usec + us) /
>> > US_PER_S);
>> >
>> > rte_spinlock_lock(_list_lk);
>> > if (!handler_registered) {
>> > ret |= rte_intr_callback_register(_handle,
>> > eal_alarm_callback, NULL);
>> > handler_registered = (ret == 0) ? 1 : 0;
>> > }
>> >
>> > if (LIST_EMPTY(_list))
>> > LIST_INSERT_HEAD(_list, new_alarm, next);
>> > else {
>> > LIST_FOREACH(ap, _list, next) {
>> > if (ap->time.tv_sec > new_alarm->time.tv_sec ||
>> > (ap->time.tv_sec ==
>> > new_alarm->time.tv_sec &&
>> >
>> > ap->time.tv_usec > new_alarm->time.tv_usec)){
>> > LIST_INSERT_BEFORE(ap, new_alarm, next);
>> > break;
>> > }
>> > if (LIST_NEXT(ap, next) == NULL) {
>> > LIST_INSERT_AFTER(ap, new_alarm, next);
>> > break;
>> > }
>> > }
>> > }
>> >
>> > After the new alarm entry is added to alarm_list, if current time is
>> > set to 8:00:00 AM manually, the current time in eal_alarm_callback()
>> > will be updated to 8:00:00 AM, too.
>> > Then the new alarm entry will be triggered after 3 hours and 1 second.
>> >
>> > I think rte alarm should not be affected by discontinuous jumps in
>> > the system time.
>> > I tried to replace gettimeofday() with
>> > clock_gettime(CLOCK_MONOTONIC_RAW, ),
>> > and it looks work fine.
>> > What do you think about this modification?
>> > Will you consider to modify rte_alarm functions to be not affected
>> > by discontinuous jumps in the system time?
>>
>> I agree with you that the alarm functionality should not be affected by
>> jumps
>> in system time. If you have a patch that fixes this bug, it would be great
>> if
>> you could upstream it here.
>>

[dpdk-dev] rte_eal_alarm_set() is affected by discontinuous jumps in the system time

2015-06-04 Thread Selmon Yang
Hi, Bruce, Jay,

I am sorry that the patch in last mail is the wrong one,
please replace it with the attached one in this mail.

2015-06-04 10:09 GMT+08:00 Selmon Yang :
> Hi, Bruce,
>
> Thank you very much for your positive response.
> The attachment is the patch.
> Please have a look.
>
> Hi, Jay,
>
> Thank you so much for your kindly reminding.
> However, due to I really like the alarm functionalities not to be affected.
> I guess I need to replace CLOCK_MONOTONIC_RAW with CLOCK_MONOTONIC
> and do more tests to see if it also works for me.
>
>
> 2015-06-03 21:07 GMT+08:00 Jay Rolette :
>>
>> On Wed, Jun 3, 2015 at 7:54 AM, Bruce Richardson
>>  wrote:
>>>
>>> On Wed, Jun 03, 2015 at 02:31:47PM +0800, Selmon Yang wrote:
>>> > Hi,
>>> >
>>> > I found that, in dpdk 2.0, rte_eal_alarm_set() is affected by
>>> > discontinuous jumps in the system time because eal_alarm_callback()
>>> > and rte_eal_alarm_set() use gettimeofday() to get the current time.
>>> >
>>> > Here is what I encountered.
>>> > I set up a rte eal alarm as below, and I like it to be triggered every
>>> > second.
>>> > #define USE_PER_S 1000 * 1000
>>> > void my_alarm_cb(void *arg)
>>> > {
>>> > /* send heartbeat signal out, etc. */
>>> >
>>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL);
>>> > return;
>>> > }
>>> >
>>> > int main(void)
>>> > {
>>> > /* ..., do something */
>>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL);
>>> > /* ... do something else */
>>> > }
>>> >
>>> > It works fine in most of time.
>>> > However, if I change system time manually, it is possible that rte alarm
>>> > function works out of my expectation.
>>> > Suppose that current time is 11:00:00 AM, and eal_alarm_callback()
>>> > is triggered because I executed
>>> > rte_eal_alarm_set(1 * US_PER_S, my_alarm_cb, NULL) at 10:59:59 AM.
>>> > eal_alarm_callback() gets the current time (11:00:00 AM)
>>> > and calls my_alarm_cb() as below.
>>> > while ((ap = LIST_FIRST(_list)) !=NULL &&
>>> >   gettimeofday(, NULL) == 0 &&
>>> >   (ap->time.tv_sec < now.tv_sec ||
>>> > (ap->time.tv_sec == now.tv_sec &&
>>> >   ap->time.tv_usec <=
>>> > now.tv_usec))){
>>> > ap->executing = 1;
>>> > ap->executing_id = pthread_self();
>>> > rte_spinlock_unlock(_list_lk);
>>> >
>>> > ap->cb_fn(ap->cb_arg);
>>> >
>>> > rte_spinlock_lock(_list_lk);
>>> >
>>> > LIST_REMOVE(ap, next);
>>> > rte_free(ap);
>>> > }
>>> >
>>> > In my_alarm_cb(), rte_eal_alarm_set() is called again.
>>> > rte_eall_alarm_set() gets the current time (11:00:00 AM), plus 1 second,
>>> > and adds the new alarm entry to alarm_list.
>>> > /* use current time to calculate absolute time of alarm */
>>> > gettimeofday(, NULL);
>>> >
>>> > new_alarm->cb_fn = cb_fn;
>>> > new_alarm->cb_arg = cb_arg;
>>> > new_alarm->time.tv_usec = (now.tv_usec + us) % US_PER_S;
>>> > new_alarm->time.tv_sec = now.tv_sec + ((now.tv_usec + us) /
>>> > US_PER_S);
>>> >
>>> > rte_spinlock_lock(_list_lk);
>>> > if (!handler_registered) {
>>> > ret |= rte_intr_callback_register(_handle,
>>> > eal_alarm_callback, NULL);
>>> > handler_registered = (ret == 0) ? 1 : 0;
>>> > }
>>> >
>>> > if (LIST_EMPTY(_list))
>>> > LIST_INSERT_HEAD(_list, new_alarm, next);
>>> > else {
>>> > LIST_FOREACH(ap, _list, next) {
>>> > if (ap->time.tv_sec > new_alarm->time.tv_sec ||
>>> > (ap->time.tv_sec ==
>>> > new_alarm->time.tv_sec &&
>>> >
>>> > ap->time.tv_usec > new_alarm->time.tv_usec)){
>>> > LIST_INSERT_BEFORE(ap, new_alarm, next);
>>> > break;
>>> > }
>>> > if (LIST_NEXT(ap, next) == NULL) {
>>> > LIST_INSERT_AFTER(ap, new_alarm, next);
>>> > break;
>>> > }
>>> > }
>>> > }
>>> >
>>> > After the new alarm entry is added to alarm_list, if current time is
>>> > set to 8:00:00 AM manually, the current time in eal_alarm_callback()
>>> > will be updated to 8:00:00 AM, too.
>>> > Then the new alarm entry will be triggered after 3 hours and 1 second.
>>> >
>>> > I think rte alarm should not be affected by discontinuous jumps in
>>> > the system time.
>>> > I tried to replace gettimeofday() with
>>> > clock_gettime(CLOCK_MONOTONIC_RAW, ),
>>> > and it looks work fine.
>>> > What do you think about this modification?
>>> > Will you consider 

[dpdk-dev] [PATCH v3] i40evf: fix of supporting jumbo frame

2015-06-04 Thread Helin Zhang
It wouldn't check the configured maximum packet length, and then
the scattered receiving function wouldn't be selected at all even
if it wants to receive a jumbo frame. The fix is to select the
correct RX function according to the configurations.

Signed-off-by: Helin Zhang 
---
 drivers/net/i40e/i40e_ethdev.h|  2 +
 drivers/net/i40e/i40e_ethdev_vf.c | 85 ++-
 drivers/net/i40e/i40e_rxtx.c  |  1 -
 3 files changed, 59 insertions(+), 29 deletions(-)

v2 changes:
* Removed maximum packet length check and jumbo frame check in
  i40evf_dev_start(), as the same checks are already in each queue
  initialization.

v3 changes:
* Added code style fixes.

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index baac4a0..587ee71 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -36,6 +36,8 @@

 #include 

+#define I40E_VLAN_TAG_SIZE4
+
 #define I40E_AQ_LEN   32
 #define I40E_AQ_BUF_SZ4096
 /* Number of queues per TC should be one of 1, 2, 4, 8, 16, 32, 64 */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index 9f92a2f..4f4404e 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1417,23 +1417,74 @@ i40evf_vlan_filter_set(struct rte_eth_dev *dev, 
uint16_t vlan_id, int on)
 }

 static int
+i40evf_rxq_init(struct rte_eth_dev *dev, struct i40e_rx_queue *rxq)
+{
+   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   struct rte_eth_dev_data *dev_data = dev->data;
+   struct rte_pktmbuf_pool_private *mbp_priv;
+   uint16_t buf_size, len;
+
+   rxq->qrx_tail = hw->hw_addr + I40E_QRX_TAIL1(rxq->queue_id);
+   I40E_PCI_REG_WRITE(rxq->qrx_tail, rxq->nb_rx_desc - 1);
+   I40EVF_WRITE_FLUSH(hw);
+
+   /* Calculate the maximum packet length allowed */
+   mbp_priv = rte_mempool_get_priv(rxq->mp);
+   buf_size = (uint16_t)(mbp_priv->mbuf_data_room_size -
+   RTE_PKTMBUF_HEADROOM);
+   rxq->hs_mode = i40e_header_split_none;
+   rxq->rx_hdr_len = 0;
+   rxq->rx_buf_len = RTE_ALIGN(buf_size, (1 << I40E_RXQ_CTX_DBUFF_SHIFT));
+   len = rxq->rx_buf_len * I40E_MAX_CHAINED_RX_BUFFERS;
+   rxq->max_pkt_len = RTE_MIN(len,
+   dev_data->dev_conf.rxmode.max_rx_pkt_len);
+
+   /**
+* Check if the jumbo frame and maximum packet length are set correctly
+*/
+   if (dev_data->dev_conf.rxmode.jumbo_frame == 1) {
+   if (rxq->max_pkt_len <= ETHER_MAX_LEN ||
+   rxq->max_pkt_len > I40E_FRAME_SIZE_MAX) {
+   PMD_DRV_LOG(ERR, "maximum packet length must be "
+   "larger than %u and smaller than %u, as jumbo "
+   "frame is enabled", (uint32_t)ETHER_MAX_LEN,
+   (uint32_t)I40E_FRAME_SIZE_MAX);
+   return I40E_ERR_CONFIG;
+   }
+   } else {
+   if (rxq->max_pkt_len < ETHER_MIN_LEN ||
+   rxq->max_pkt_len > ETHER_MAX_LEN) {
+   PMD_DRV_LOG(ERR, "maximum packet length must be "
+   "larger than %u and smaller than %u, as jumbo "
+   "frame is disabled", (uint32_t)ETHER_MIN_LEN,
+   (uint32_t)ETHER_MAX_LEN);
+   return I40E_ERR_CONFIG;
+   }
+   }
+
+   if (dev_data->dev_conf.rxmode.enable_scatter ||
+   (rxq->max_pkt_len + 2 * I40E_VLAN_TAG_SIZE) > buf_size) {
+   dev_data->scattered_rx = 1;
+   dev->rx_pkt_burst = i40e_recv_scattered_pkts;
+   }
+
+   return 0;
+}
+
+static int
 i40evf_rx_init(struct rte_eth_dev *dev)
 {
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
uint16_t i;
struct i40e_rx_queue **rxq =
(struct i40e_rx_queue **)dev->data->rx_queues;
-   struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);

i40evf_config_rss(vf);
for (i = 0; i < dev->data->nb_rx_queues; i++) {
-   rxq[i]->qrx_tail = hw->hw_addr + I40E_QRX_TAIL1(i);
-   I40E_PCI_REG_WRITE(rxq[i]->qrx_tail, rxq[i]->nb_rx_desc - 1);
+   if (i40evf_rxq_init(dev, rxq[i]) < 0)
+   return -EFAULT;
}

-   /* Flush the operation to write registers */
-   I40EVF_WRITE_FLUSH(hw);
-
return 0;
 }

@@ -1474,28 +1525,6 @@ i40evf_dev_start(struct rte_eth_dev *dev)
PMD_INIT_FUNC_TRACE();

vf->max_pkt_len = dev->data->dev_conf.rxmode.max_rx_pkt_len;
-   if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
-   if (vf->max_pkt_len <= ETHER_MAX_LEN ||
-   vf->max_pkt_len > I40E_FRAME_SIZE_MAX) {
-   

[dpdk-dev] [PATCH v2 1/6] ethdev: add an field for querying hash key size

2015-06-04 Thread Helin Zhang
To support querying hash key size per port, an new field of
'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
hash key size in bytes.

Signed-off-by: Helin Zhang 
---
 lib/librte_ether/rte_ethdev.h | 3 +++
 1 file changed, 3 insertions(+)

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..bdebc87 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -916,6 +916,9 @@ struct rte_eth_dev_info {
uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   uint8_t hash_key_size; /**< Hash key size in bytes */
+#endif
uint16_t reta_size;
/**< Device redirection table size, the total number of entries. */
/** Bit mask of RSS offloads, the bit offset also means flow type */
-- 
1.9.3



[dpdk-dev] [PATCH v2 3/6] fm10k: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/fm10k/fm10k_ethdev.c | 3 +++
 1 file changed, 3 insertions(+)

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 87852ed..043c60a 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -767,6 +767,9 @@ fm10k_dev_infos_get(struct rte_eth_dev *dev,
DEV_RX_OFFLOAD_UDP_CKSUM  |
DEV_RX_OFFLOAD_TCP_CKSUM;
dev_info->tx_offload_capa= 0;
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   dev_info->hash_key_size = FM10K_RSSRK_SIZE * sizeof(uint32_t);
+#endif
dev_info->reta_size = FM10K_MAX_RSS_INDICES;

dev_info->default_rxconf = (struct rte_eth_rxconf) {
-- 
1.9.3



[dpdk-dev] [PATCH v2 4/6] i40e: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/i40e/i40e_ethdev.c| 4 
 drivers/net/i40e/i40e_ethdev_vf.c | 4 
 2 files changed, 8 insertions(+)

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index da6c0b5..63c76a6 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1540,6 +1540,10 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
DEV_TX_OFFLOAD_SCTP_CKSUM |
DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
DEV_TX_OFFLOAD_TCP_TSO;
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   dev_info->hash_key_size = (I40E_PFQF_HKEY_MAX_INDEX + 1) *
+   sizeof(uint32_t);
+#endif
dev_info->reta_size = pf->hash_lut_size;
dev_info->flow_type_rss_offloads = I40E_RSS_OFFLOAD_ALL;

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index 9f92a2f..486b394 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1641,6 +1641,10 @@ i40evf_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
dev_info->max_tx_queues = vf->vsi_res->num_queue_pairs;
dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
dev_info->max_rx_pktlen = I40E_FRAME_SIZE_MAX;
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   dev_info->hash_key_size = (I40E_VFQF_HKEY_MAX_INDEX + 1) *
+   sizeof(uint32_t);
+#endif
dev_info->reta_size = ETH_RSS_RETA_SIZE_64;
dev_info->flow_type_rss_offloads = I40E_RSS_OFFLOAD_ALL;

-- 
1.9.3



[dpdk-dev] [PATCH v2 2/6] e1000: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/e1000/igb_ethdev.c | 5 +
 1 file changed, 5 insertions(+)

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index e4b370d..b85b786 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -68,6 +68,8 @@
 #define IGB_DEFAULT_TX_HTHRESH  0
 #define IGB_DEFAULT_TX_WTHRESH  0

+#define IGB_HKEY_MAX_INDEX 10
+
 /* Bit shift and mask */
 #define IGB_4_BIT_WIDTH  (CHAR_BIT / 2)
 #define IGB_4_BIT_MASK   RTE_LEN2MASK(IGB_4_BIT_WIDTH, uint8_t)
@@ -1377,6 +1379,9 @@ eth_igb_infos_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
/* Should not happen */
break;
}
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   dev_info->hash_key_size = IGB_HKEY_MAX_INDEX * sizeof(uint32_t);
+#endif
dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
dev_info->flow_type_rss_offloads = IGB_RSS_OFFLOAD_ALL;

-- 
1.9.3



[dpdk-dev] [PATCH v2 5/6] ixgbe: fill the hash key size

2015-06-04 Thread Helin Zhang
The correct hash key size in bytes should be filled into the
'struct rte_eth_dev_info', to support querying it.

Signed-off-by: Helin Zhang 
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 5 +
 1 file changed, 5 insertions(+)

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0d9f9b2..b6f2574 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -116,6 +116,8 @@

 #define IXGBE_QUEUE_STAT_COUNTERS (sizeof(hw_stats->qprc) / 
sizeof(hw_stats->qprc[0]))

+#define IXGBE_HKEY_MAX_INDEX 10
+
 static int eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev);
 static int  ixgbe_dev_configure(struct rte_eth_dev *dev);
 static int  ixgbe_dev_start(struct rte_eth_dev *dev);
@@ -2052,6 +2054,9 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct 
rte_eth_dev_info *dev_info)
.txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS |
ETH_TXQ_FLAGS_NOOFFLOADS,
};
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   dev_info->hash_key_size = IXGBE_HKEY_MAX_INDEX * sizeof(uint32_t);
+#endif
dev_info->reta_size = ETH_RSS_RETA_SIZE_128;
dev_info->flow_type_rss_offloads = IXGBE_RSS_OFFLOAD_ALL;
 }
-- 
1.9.3



[dpdk-dev] [PATCH v2 6/6] app/testpmd: show the hash key size

2015-06-04 Thread Helin Zhang
As querying hash key size in byte was supported, it can be shown
in testpmd after getting the device information if not zero.

Signed-off-by: Helin Zhang 
---
 app/test-pmd/config.c | 4 
 1 file changed, 4 insertions(+)

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f788ed5..a9ec065 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -361,6 +361,10 @@ port_infos_display(portid_t port_id)

memset(_info, 0, sizeof(dev_info));
rte_eth_dev_info_get(port_id, _info);
+#ifdef RTE_QUERY_HASH_KEY_SIZE
+   if (dev_info.hash_key_size > 0)
+   printf("Hash key size in bytes: %u\n", dev_info.hash_key_size);
+#endif
if (dev_info.reta_size > 0)
printf("Redirection table size: %u\n", dev_info.reta_size);
if (!dev_info.flow_type_rss_offloads)
-- 
1.9.3



[dpdk-dev] [PATCH v2 0/6] query hash key size in byte

2015-06-04 Thread Helin Zhang
As different hardware has different hash key sizes, querying it (in byte)
per port was asked by users. Otherwise there is no convenient way to know
the size of hash key which should be prepared.

v2 changes:
* Disabled the code changes by default, to avoid breaking ABI compatibility.

Helin Zhang (6):
  ethdev: add an field for querying hash key size
  e1000: fill the hash key size
  fm10k: fill the hash key size
  i40e: fill the hash key size
  ixgbe: fill the hash key size
  app/testpmd: show the hash key size

 app/test-pmd/config.c | 4 
 drivers/net/e1000/igb_ethdev.c| 5 +
 drivers/net/fm10k/fm10k_ethdev.c  | 3 +++
 drivers/net/i40e/i40e_ethdev.c| 4 
 drivers/net/i40e/i40e_ethdev_vf.c | 4 
 drivers/net/ixgbe/ixgbe_ethdev.c  | 5 +
 lib/librte_ether/rte_ethdev.h | 3 +++
 7 files changed, 28 insertions(+)

-- 
1.9.3



[dpdk-dev] Any chance someone could fix the SPF records for this mailing list?

2015-06-04 Thread Thomas Monjalon
2015-06-03 20:09, Matthew Hall:
> 2015-06-03 19:54, Alexander Duyck:
> > I have noticed a number of emails from this list are going to spam.  It
> > looks like it might be gmail filtering based on the fact that most of
> > the list has a valid SPF based on an IPv4 address that reports out like
> > below:
[...]
> > Received-SPF: pass (google.com: best guess record for domain of
> > dev-bounces at dpdk.org designates 92.243.14.124 as permitted sender)
[...]
> > However the ones that are going straight into my spam folder list an
> > IPv6 address that is rated neutral by the SPF:
[...]
> > Received-SPF: neutral (google.com: 2001:4b98:dc0:41:216:3eff:fe72:dd13
> > is neither permitted nor denied by best guess record for domain of
> > dev-bounces at dpdk.org) client-ip=2001:4b98:dc0:41:216:3eff:fe72:dd13;
[...]
> > I was just wondering if it would be possible to get the IPv6 address
> > added as a permitted sender for the domain to help reduce the amount of
> > messages that are likely being flagged as spam for myself and likely
> > others.
> 
> https://support.google.com/mail/answer/81126?p=ipv6_authentication_error=1#authentication

Thanks for the information.
Indeed the ip6: field of the SPF was not filled. It should be fixed now.

By the way, it is possible to avoid spam classification in gmail with a
filter to:(dev at dpdk.org).


[dpdk-dev] "Re: [RFC PATCH 0/2] dynamic memzones"

2015-06-04 Thread Gonzalez Monroy, Sergio
On 03/06/2015 20:13, Dax Rawal wrote:
> Hi Sergio,
> >TODOs:
> > - Implement memzone_unreserve, simply call rte_malloc_free.
> > - Implement mempool_delete, simply call rte_memzone_unreserve.
> > - Init heaps with all available memsegs at once.
> > - Review symbols in version map.
> I do not see rte_memzone_unreserve in API documentation. Is this patch 
> already committed so that we have function like rte_mempool_delete()
> Thanks
> Dax
No, it is not merged yet. That was just an RFC, still need to send new 
patch with updates/changes.

The feature is expected for 2.1 release if everything goes according to 
plan.

Sergio


[dpdk-dev] mempool with custom alignment

2015-06-04 Thread Ananyev, Konstantin


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Dax Rawal
> Sent: Wednesday, June 03, 2015 7:30 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] mempool with custom alignment
> 
> Hi,
> I use mempool APIs to allocate DMA-able descriptor ring and buffers so that
> I get both physical and virtual addresses of allocated memory. I buffers I
> get from the mempool APIs are 64 byte aligned. My requirement is 256byte
> alignment. How to achieve this? mempool APIs do not seem to take alignment
> parameters.
> 
> Thanks,
> -Dax

Something like that:
http://patchwork.dpdk.org/ml/archives/dev/2014-October/006595.html
should work, I think.
Konstantin


[dpdk-dev] [PATCH 0/3] get rid of unnecessary memset's

2015-06-04 Thread Thomas Monjalon
2015-06-03 14:13, Stephen Hemminger:
> While looking at code is seems lots of drivers do not know the convention
> that info_get and stats_get both clear the buffer for the caller.
> 
> Stephen Hemminger (3):
>   bonding: remove useless memset
>   ethdev: remove useless memset
>   null: remove unnecessary memset

Acked-by: Thomas Monjalon 

Applied, thanks


[dpdk-dev] [PATCHv2 0/3] ACL: Fix bug in acl_merge_trie() and add a new test-case for it to the UT

2015-06-04 Thread Thomas Monjalon
2015-06-03 18:45, Konstantin Ananyev:
> v2:
> - reorder code a bit to avoid gcc 5.1 warnings.
> 
> Konstantin Ananyev (3):
>   ACL: fix a problem in acl_merge_trie
>   ACL: add new test case for ranges build
>   ACL: remove subtree_id calculations at build stage

Applied, thanks


[dpdk-dev] [PATCH 0/8] ACL: various fixes and cleanups

2015-06-04 Thread Thomas Monjalon
Hi Konstantin,

2015-06-04 00:10, Konstantin Ananyev:
> This patch-set is based on:
> [PATCHv2 0/3] ACL: Fix bug in acl_merge_trie() and add a new test-case for it 
> to the UT.
> 
> Konstantin Ananyev (8):
>  ACL: fix invalid rule wildness calculation for RTE_ACL_FIELD_TYPE_BITMASK
>  ACL: code cleanup - use global RTE_LEN2MASK macro
>  ACL: add function to check rte_acl_build() input parameters
>  ACL: fix rebuilding a trie for subset of rules
>  ACL: introduce RTE_ACL_MASKLEN_TO_BITMASK macro
>  ACL: cleanup remove unused code from acl_bld.c
>  ACL: fix remove ambiguity between rules at UT
>  ACL: add new test-cases into UT

Commit titles would be more useful if they shortly explain the goal instead
of giving some function/macro names. It should be thought as a changelog entry
and reflect behavioral change.
On a side note, the keyword ACL: should be lowercase.

In the case of fixes, adding a Fixes: line may help.
It can be generated with this command:
git log -1 --abbrev=12 --format='Fixes: %h ("%s")' 

Thanks


[dpdk-dev] [PATCH v2] doc: add coding standards documentation

2015-06-04 Thread Bruce Richardson
On Wed, Jun 03, 2015 at 07:35:32PM +0200, Thomas Monjalon wrote:
> 2015-06-03 14:58, Bruce Richardson:
> > Add coding standards document to guides directory. This document
> > codifies the current DPDK C coding conventions, to make it easier for
> > contributors to see the format their code should be in.
> > 
> > Signed-off-by: Siobhan Butler 
> > Signed-off-by: Bruce Richardson 
> > 
> > ---
> > 
> > Updates in V2:
> > * Fixed file creation mode
> > * Removed blank line at end of file
> > * Adjusted coding blocks to be C language, rather than console, so as to
> >   have correct syntax highlighting.
> > * Shortened longer lines by breaking lines at sentence - and
> >   occasionally comma - boundaries.
> > ---
> >  doc/guides/coding_standards/index.rst | 861 
> > ++
> >  doc/guides/index.rst  |   1 +
> 
> This version seems very good.
> However, I have the feeling that coding_standards/ is not a good name:
>   - "standards" word is a bit strong ;
>   - "coding" seems too restrictive, maybe we would like to put
> more guidelines in this directory (thinking about file tree, compat and 
> tests).
> Do you agree I rename it to guidelines/coding_style.rst?

No problem, go with it!


[dpdk-dev] rte_eal_alarm_set() is affected by discontinuous jumps in the system time

2015-06-04 Thread Bruce Richardson
On Thu, Jun 04, 2015 at 10:29:48AM +0800, Selmon Yang wrote:
> Hi, Bruce, Jay,
> 
> I am sorry that the patch in last mail is the wrong one,
> please replace it with the attached one in this mail.
> 

Thanks for the patch. Could you perhaps submit this using git send-email as an 
official 
patch, with appropriate signoff to the mailing list. It will make reviewing
it easier, and allow us to consider it for integration. See http://dpdk.org/dev
for more details on how to submit patches.

Regards,
/Bruce



[dpdk-dev] [PATCH v2] doc: add coding standards documentation

2015-06-04 Thread Thomas Monjalon
2015-06-04 10:33, Bruce Richardson:
> On Wed, Jun 03, 2015 at 07:35:32PM +0200, Thomas Monjalon wrote:
> > 2015-06-03 14:58, Bruce Richardson:
> > > Add coding standards document to guides directory. This document
> > > codifies the current DPDK C coding conventions, to make it easier for
> > > contributors to see the format their code should be in.
> > > 
> > > Signed-off-by: Siobhan Butler 
> > > Signed-off-by: Bruce Richardson 
> > > 
> > > ---
> > > 
> > > Updates in V2:
> > > * Fixed file creation mode
> > > * Removed blank line at end of file
> > > * Adjusted coding blocks to be C language, rather than console, so as to
> > >   have correct syntax highlighting.
> > > * Shortened longer lines by breaking lines at sentence - and
> > >   occasionally comma - boundaries.
> > > ---
> > >  doc/guides/coding_standards/index.rst | 861 
> > > ++
> > >  doc/guides/index.rst  |   1 +
> > 
> > This version seems very good.
> > However, I have the feeling that coding_standards/ is not a good name:
> > - "standards" word is a bit strong ;
> > - "coding" seems too restrictive, maybe we would like to put
> > more guidelines in this directory (thinking about file tree, compat and 
> > tests).
> > Do you agree I rename it to guidelines/coding_style.rst?
> 
> No problem, go with it!

Acked-by: Thomas Monjalon 

Applied, thanks


[dpdk-dev] [PATCH v3] i40evf: fix of supporting jumbo frame

2015-06-04 Thread Thomas Monjalon
2015-06-04 14:54, Helin Zhang:
> It wouldn't check the configured maximum packet length, and then
> the scattered receiving function wouldn't be selected at all even
> if it wants to receive a jumbo frame. The fix is to select the
> correct RX function according to the configurations.
> 
> Signed-off-by: Helin Zhang 
> ---
>  drivers/net/i40e/i40e_ethdev.h|  2 +
>  drivers/net/i40e/i40e_ethdev_vf.c | 85 
> ++-
>  drivers/net/i40e/i40e_rxtx.c  |  1 -
>  3 files changed, 59 insertions(+), 29 deletions(-)
> 
> v2 changes:
> * Removed maximum packet length check and jumbo frame check in
>   i40evf_dev_start(), as the same checks are already in each queue
>   initialization.
> 
> v3 changes:
> * Added code style fixes.

Applied, thanks


[dpdk-dev] [PATCH 0/8] ACL: various fixes and cleanups

2015-06-04 Thread Ananyev, Konstantin
Hi Thomas,

> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, June 04, 2015 10:27 AM
> To: Ananyev, Konstantin
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 0/8] ACL: various fixes and cleanups
> 
> Hi Konstantin,
> 
> 2015-06-04 00:10, Konstantin Ananyev:
> > This patch-set is based on:
> > [PATCHv2 0/3] ACL: Fix bug in acl_merge_trie() and add a new test-case for 
> > it to the UT.
> >
> > Konstantin Ananyev (8):
> >  ACL: fix invalid rule wildness calculation for RTE_ACL_FIELD_TYPE_BITMASK
> >  ACL: code cleanup - use global RTE_LEN2MASK macro
> >  ACL: add function to check rte_acl_build() input parameters
> >  ACL: fix rebuilding a trie for subset of rules
> >  ACL: introduce RTE_ACL_MASKLEN_TO_BITMASK macro
> >  ACL: cleanup remove unused code from acl_bld.c
> >  ACL: fix remove ambiguity between rules at UT
> >  ACL: add new test-cases into UT
> 
> Commit titles would be more useful if they shortly explain the goal instead
> of giving some function/macro names. It should be thought as a changelog entry
> and reflect behavioral change.

There is no changes in behaviour.
Just fixes and clean-ups.

> On a side note, the keyword ACL: should be lowercase.
> 
> In the case of fixes, adding a Fixes: line may help.
> It can be generated with this command:
>   git log -1 --abbrev=12 --format='Fixes: %h ("%s")' 

Ok, will update commit messages in v2.
Konstantin

> 
> Thanks





[dpdk-dev] [PATCH v2 1/6] ethdev: add an field for querying hash key size

2015-06-04 Thread Ananyev, Konstantin
Hi Helin,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Helin Zhang
> Sent: Thursday, June 04, 2015 8:34 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2 1/6] ethdev: add an field for querying hash key 
> size
> 
> To support querying hash key size per port, an new field of
> 'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
> hash key size in bytes.
> 
> Signed-off-by: Helin Zhang 
> ---
>  lib/librte_ether/rte_ethdev.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> v2 changes:
> * Disabled the code changes by default, to avoid breaking ABI compatibility.
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 16dbe00..bdebc87 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -916,6 +916,9 @@ struct rte_eth_dev_info {
>   uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
>   uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
>   uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> +#ifdef RTE_QUERY_HASH_KEY_SIZE
> + uint8_t hash_key_size; /**< Hash key size in bytes */
> +#endif
>   uint16_t reta_size;
>   /**< Device redirection table size, the total number of entries. */
>   /** Bit mask of RSS offloads, the bit offset also means flow type */

Why do you need to introduce an #ifdef RTE_QUERY_HASH_KEY_SIZE
around your code?
Why not to have it always on?
Is it because of not breaking ABI for 2.1?
But here, I suppose there would be no breakage anyway:

struct rte_eth_dev_info {
...
uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
uint16_t reta_size;
/**< Device redirection table size, the total number of entries. */
/** Bit mask of RSS offloads, the bit offset also means flow type */
uint64_t flow_type_rss_offloads;
struct rte_eth_rxconf default_rxconf;


so between 'reta_size' and 'flow_type_rss_offloads', there is a 2 bytes gap.
Wonder, why not put it there?

Konstantin

> --
> 1.9.3



[dpdk-dev] [PATCH] vhost: enable live migration

2015-06-04 Thread Thomas Monjalon
2015-06-01 04:47, Ouyang, Changchun:
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie
> > When we migrate VM, without this feature, qemu will report error:
> > "migrate: Migration disabled: vhost lacks VHOST_F_LOG_ALL feature".
> 
> Is this enough for vhost to support  migrate VM?
> I remember Claire has another patch, possibly need refer to that patch.

Indeed, there were some patches which do not build:
http://dpdk.org/ml/archives/dev/2014-August/005050.html
And there was no answer.

[...]
> > +   (1ULL << VHOST_F_LOG_ALL))

Please check if this line is sufficient.
Thanks


[dpdk-dev] [PATCH 1/6] ethdev: add an field for querying hash key size

2015-06-04 Thread Neil Horman
On Thu, Jun 04, 2015 at 09:00:33AM +0800, Helin Zhang wrote:
> To support querying hash key size per port, an new field of
> 'hash_key_size' was added in 'struct rte_eth_dev_info' for storing
> hash key size in bytes.
> 
> Signed-off-by: Helin Zhang 
> ---
>  lib/librte_ether/rte_ethdev.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 16dbe00..004b05a 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -916,6 +916,7 @@ struct rte_eth_dev_info {
>   uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
>   uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
>   uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> + uint8_t hash_key_size; /**< Hash key size in bytes */
>   uint16_t reta_size;
>   /**< Device redirection table size, the total number of entries. */
>   /** Bit mask of RSS offloads, the bit offset also means flow type */
> -- 
> 1.9.3
> 
> 

You'll need to at least move this to the end of the structure to avoid ABI
breakage, but even then, since the examples statically allocate this struct on
the stack, you need to worry about previously compiled applications not having
enough space allocated.  Is there a hole in the struct that this can fit into to
avoid changing the other member offsets?
Neil



[dpdk-dev] [PATCH v2] doc: add coding standards documentation

2015-06-04 Thread Bruce Richardson
On Thu, Jun 04, 2015 at 11:49:49AM +0200, Thomas Monjalon wrote:
> 2015-06-04 10:33, Bruce Richardson:
> > On Wed, Jun 03, 2015 at 07:35:32PM +0200, Thomas Monjalon wrote:
> > > 2015-06-03 14:58, Bruce Richardson:
> > > > Add coding standards document to guides directory. This document
> > > > codifies the current DPDK C coding conventions, to make it easier for
> > > > contributors to see the format their code should be in.
> > > > 
> > > > Signed-off-by: Siobhan Butler 
> > > > Signed-off-by: Bruce Richardson 
> > > > 
> > > > ---
> > > > 
> > > > Updates in V2:
> > > > * Fixed file creation mode
> > > > * Removed blank line at end of file
> > > > * Adjusted coding blocks to be C language, rather than console, so as to
> > > >   have correct syntax highlighting.
> > > > * Shortened longer lines by breaking lines at sentence - and
> > > >   occasionally comma - boundaries.
> > > > ---
> > > >  doc/guides/coding_standards/index.rst | 861 
> > > > ++
> > > >  doc/guides/index.rst  |   1 +
> > > 
> > > This version seems very good.
> > > However, I have the feeling that coding_standards/ is not a good name:
> > >   - "standards" word is a bit strong ;
> > >   - "coding" seems too restrictive, maybe we would like to put
> > > more guidelines in this directory (thinking about file tree, compat and 
> > > tests).
> > > Do you agree I rename it to guidelines/coding_style.rst?
> > 
> > No problem, go with it!
> 
> Acked-by: Thomas Monjalon 
> 
> Applied, thanks

Great!

General request to mailing list: While I've done my best with this doc to 
cover the current practice of coding in DPDK, I know there are lots of areas of
style and convention I must have missed. Please submit patches to help fill in 
the
gaps in this doc. :-)

Thanks!
/Bruce


[dpdk-dev] [PATCH 1/3] kni: minor opto

2015-06-04 Thread Bruce Richardson
On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> Don't need the 'safe' version of list_for_each_entry() if you aren't deleting 
> from the list as you iterate over it
> 
> Signed-off-by: Jay Rolette 

Acked-by: Bruce Richardson 



[dpdk-dev] [PATCH 1/3] kni: minor opto

2015-06-04 Thread Bruce Richardson
On Thu, Jun 04, 2015 at 02:39:17PM +0100, Bruce Richardson wrote:
> On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> > Don't need the 'safe' version of list_for_each_entry() if you aren't 
> > deleting from the list as you iterate over it
> > 
> > Signed-off-by: Jay Rolette 
> 
> Acked-by: Bruce Richardson 
> 
Forgot to mention though, that the commit title needs to be a little more
descriptive.


[dpdk-dev] [PATCH 3/3] kni: rx loop was using the wrong counter

2015-06-04 Thread Bruce Richardson
On Wed, Jun 03, 2015 at 02:18:57PM -0500, Jay Rolette wrote:
> Loop processing packets dequeued from rx_q was using the number of
> packets requested, not how many it actually received.
> 
> Variable rename to make code a little more clear
> 
> Signed-off-by: Jay Rolette 

s/more clear/clearer/

Acked-by: Bruce Richardson 


[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Neil Horman
On Thu, Jun 04, 2015 at 11:50:33AM +, Wiles, Keith wrote:
> Hi Stephen
> 
> On 6/3/15, 7:12 PM, "Stephen Hemminger"  wrote:
> 
> >On Wed,  3 Jun 2015 13:49:53 -0500
> >Keith Wiles  wrote:
> >
> >> +/* Launch threads, called at application init() and parse app args. */
> >> +int
> >> +rte_eal_init_parse(int argc, char **argv,
> >> +  int (*parse)(int, char **))
> >> +{
> >> +  int ret;
> >> +
> >> +  ret = rte_eal_init(argc, argv);
> >> +  if ((ret >= 0) && (parse != NULL)) {
> >> +  argc -= ret;
> >> +  argv += ret;
> >
> >This won't work C is call by value.
> 
> I tested this routine with Pktgen (again), which has a number of
> application options and it appears to work correctly. Can you explain why
> this will not work?
> 
> Regards,
> ++Keith
> >
> 
> 

Stephen was thinking that your intent was to have argc, and argv modified at the
call site of this function (i.e. if you called rte_eal_init_parse from main(),
then after the call to rte_ela_init_parse, argc would be reduced by ret and argv
would point forward in memory ret bytes in the main function, but they wont.  It
doesn't matter though, because you return ret, so the caller can do that
movement themselves.  As you note, it works.

Note that if it was your intention to have argc and argv modified at the call
site, then Stephen is right and this is broken, you need to modify the prototype
to be:
int rte_eal_init_parse(int *argc, char ***argv)

and do a dereference when modifying the parameters so the change is seen at the
call site.

That said, I'm not sure theres much value in adding this to the API.  For one,
it implies that dpdk arguments need to come first on the command line.  While
all the example applications do that, theres no requirement that they do so, and
this function silently implies that they have to, so any existing applications
in the wild that violate that assumption are enjoined from using this

It also doesn't really save any code.  If we pick an example app (I'll us
l2fwd-jobstats), We currently have this:

/* init EAL */
ret = rte_eal_init(argc, argv);
if (ret < 0)
rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
argc -= ret;
argv += ret;

/* parse application arguments (after the EAL ones) */
ret = l2fwd_parse_args(argc, argv);
if (ret < 0)
rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");

With your new API we would get this:

ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
if (ret < 0)
rte_exit(EXIT_FAILURE, "Invalid arguments - not sure what\n");

Its definately 5 fewer lines of source, but it doesn't save any execution
instructions, and for the effort of that, you loose the ability to determine if
it was a DPDK argument or an application argument that failed.

Its not a bad addition, I'm just not sure its worth having to take on the
additional API surface to include.  I'd be more supportive if you could enhance
the function to allow the previously mentioned before/after flexibiilty.  Then
we could just deprecate rte_eal_init as an API call entirely, and use this
instead.

Neil



[dpdk-dev] [PATCH 2/2] ethtool: add new library to provide ethtool-alike APIs

2015-06-04 Thread O'Driscoll, Tim

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Andrew Harvey (agh)
> Sent: Wednesday, June 3, 2015 3:10 AM
> To: Thomas Monjalon; Wang, Liang-min
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] ethtool: add new library to provide
> ethtool-alike APIs
> 
> I believe that their is value in this interface for software stacks not
> based on Linux being moved toward DPDK that need simple operations like
> getting the mac address.  Some of these stacks have a dearth of
> resources
> available and dedicating a core/thread to KNI to get/set a mac address
> is considered excessive. There are also issues with 32/64 bit kernel
> integration
> using KNI.  If the ethtool interface is not the correct interface then
> please help me
> understand what should/could have been used. If ethtool is considered
> 'old
> and clunky?
> Stephen's and your input would be valuable in designing another
> interface
> with
> similar properties.  The use-case is pretty simple and there is no plans
> for moving
> anything back into the kernel on the contrary its the complete opposite.
> 
> < Andy

Stephen, Thomas: I think it was the two of you that originally questioned the 
justification for including this change. Now that Andy and Dave Harton have 
clarified, does this resolve your concerns or do you still have questions? I 
just want to make sure that we reach a timely conclusion on this.


Tim

> 
> 
> On 6/2/15, 1:37 PM, "Thomas Monjalon"  wrote:
> 
> >I have the feeling we are not progressing in this discussion.
> >Please bring new explanations or I'll give up.
> >David Harton already acked it so maybe he could explain why it is
> useful.
> >
> >Comments below
> >
> >2015-06-02 17:06, Wang, Liang-min:
> >> >2015-06-02 15:47, Wang, Liang-min:
> >> >> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> >> >> > >>I'm curious to understand how renaming rte_eth_dev_set_mtu()
> to
> >> >> > >>rte_ethtool_net_change_mtu() will help anyone.
> >> >> >>
> >> >> >> As described, this interface is designed to provide API closely
> >>to kernel space ethtool ops and net_device_op.
> >> >>
> >> >> >But the application still needs to adapt the code to call rte_*
> >>functions.
> >> >> >So changing to rte_ethtool_net_change_mtu is equivalent to change
> >>to the existing rte_eth_dev_set_mtu. I don't see the benefit.
> >> >>
> >> >> The benefit is single interface for users to access. Instead of
> >>looking into two different interfaces (ethtool, ether).
> >> >
> >> >Sorry it doesn't help to understand.
> >> >Today, there is an ethdev API. Why adding an ethtool-like API would
> >>help?
> >>
> >> Need to understand your specific concern. Do you oppose introducing
> new
> >>API to support ethtool ops and net_device_ops?
> >
> >They are not ethtool/netdev ops but fake ones.
> >In linux:
> > int dev_set_mtu(struct net_device *dev, int new_mtu)
> >In dpdk:
> > int rte_ethtool_net_change_mtu(uint8_t port_id, int mtu);
> >So yes, I'm opposed to add an API which is neither ethdev, neither
> >ethtool.
> >But I may be missing an obvious justification.
> >
> >> Or your concern is on the implementation. If that's latter.
> >> Do you oppose adding a new library to implement ethtool ops and
> >>net_device_ops?
> >
> >Already answered above
> >
> >> If so, are you satisfied with my previous answer on avoiding
> >>polluting ethdev APIs?
> >> If not, do you suggest integrating ethtool APIs into ethdev
> API?
> >
> >No, it's better as a separate library.
> >
> >> If not, is your concern on the implementation of common
> >>functionality between ethtool and ethdev APIs?
> >> If so, as explained, it is designed to provide a unified
> >>interface to assist users to migrate from kernel ethtool/net-device-op
> >>API to DPDK
> >
> >Do you mean it would help migrating some code from kernel space to
> dpdk?
> >How it would help since the functions and data are different from the
> >kernel ones?
> >
> >> BTW, as my reply to Steve's comment, more ops are on their way. This
> >>patch is to open up the interface.
> >
> >Currently they are only some one-liners plus ethtool_drvinfo formatting
> >so there is no real benefit.
> >Why not wait to have more ops implemented?
> >



[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Wiles, Keith
Hi Neil and Stephen,

On 6/4/15, 8:55 AM, "Neil Horman"  wrote:

>On Thu, Jun 04, 2015 at 11:50:33AM +, Wiles, Keith wrote:
>> Hi Stephen
>> 
>> On 6/3/15, 7:12 PM, "Stephen Hemminger" 
>>wrote:
>> 
>> >On Wed,  3 Jun 2015 13:49:53 -0500
>> >Keith Wiles  wrote:
>> >
>> >> +/* Launch threads, called at application init() and parse app args.
>>*/
>> >> +int
>> >> +rte_eal_init_parse(int argc, char **argv,
>> >> + int (*parse)(int, char **))
>> >> +{
>> >> + int ret;
>> >> +
>> >> + ret = rte_eal_init(argc, argv);
>> >> + if ((ret >= 0) && (parse != NULL)) {
>> >> + argc -= ret;
>> >> + argv += ret;
>> >
>> >This won't work C is call by value.
>> 
>> I tested this routine with Pktgen (again), which has a number of
>> application options and it appears to work correctly. Can you explain
>>why
>> this will not work?
>> 
>> Regards,
>> ++Keith
>> >
>> 
>> 
>
>Stephen was thinking that your intent was to have argc, and argv modified
>at the
>call site of this function (i.e. if you called rte_eal_init_parse from
>main(),
>then after the call to rte_ela_init_parse, argc would be reduced by ret
>and argv
>would point forward in memory ret bytes in the main function, but they
>wont.  It
>doesn't matter though, because you return ret, so the caller can do that
>movement themselves.  As you note, it works.
>
>Note that if it was your intention to have argc and argv modified at the
>call
>site, then Stephen is right and this is broken, you need to modify the
>prototype
>to be:
>int rte_eal_init_parse(int *argc, char ***argv)

My intent was not to alter the argc and argv values as that is not a
reasonable use case, correct?

>
>and do a dereference when modifying the parameters so the change is seen
>at the
>call site.
>
>That said, I'm not sure theres much value in adding this to the API.  For
>one,
>it implies that dpdk arguments need to come first on the command line.
>While
>all the example applications do that, theres no requirement that they do
>so, and
>this function silently implies that they have to, so any existing
>applications
>in the wild that violate that assumption are enjoined from using this
>
>It also doesn't really save any code.  If we pick an example app (I'll us
>l2fwd-jobstats), We currently have this:
>
>   /* init EAL */
>ret = rte_eal_init(argc, argv);
>if (ret < 0)
>rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
>argc -= ret;
>argv += ret;
>
>/* parse application arguments (after the EAL ones) */
>ret = l2fwd_parse_args(argc, argv);
>   if (ret < 0)
>rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");
>
>With your new API we would get this:
>
>   ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
>if (ret < 0)
>rte_exit(EXIT_FAILURE, "Invalid arguments - not sure
>what\n");
>
>Its definately 5 fewer lines of source, but it doesn't save any execution
>instructions, and for the effort of that, you loose the ability to
>determine if
>it was a DPDK argument or an application argument that failed.

I agree this is not saving instructions and adding performance, but of
code clutter and providing a layered model for the developer. The
rte_eal_init() routine still exists and I was not trying to remove that
API only layer a convenient API for common constructs.
>
>Its not a bad addition, I'm just not sure its worth having to take on the
>additional API surface to include.  I'd be more supportive if you could
>enhance
>the function to allow the previously mentioned before/after flexibiilty.
>Then
>we could just deprecate rte_eal_init as an API call entirely, and use this
>instead.

I can see we can create an API to add support for doing the applications
args first or after, but would that even be acceptable?

++Keith
>
>Neil
>



[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread David Marchand
On Thu, Jun 4, 2015 at 4:27 PM, Wiles, Keith  wrote:

> Hi Neil and Stephen,
>
> I agree this is not saving instructions and adding performance, but of
> code clutter and providing a layered model for the developer. The
> rte_eal_init() routine still exists and I was not trying to remove that
> API only layer a convenient API for common constructs.
> >
> >Its not a bad addition, I'm just not sure its worth having to take on the
> >additional API surface to include.  I'd be more supportive if you could
> >enhance
> >the function to allow the previously mentioned before/after flexibiilty.
> >Then
> >we could just deprecate rte_eal_init as an API call entirely, and use this
> >instead.
>
> I can see we can create an API to add support for doing the applications
> args first or after, but would that even be acceptable?
>

What's the point ?
Adding stuff just for saving lines ?
Are you serious about this ?


-- 
David Marchand


[dpdk-dev] [PATCH 1/9] kni: fix whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Ran this code base through a script which:
  - removes trailing whitespace
  - removes space before tabs
  - removes blank lines at end of file

Signed-off-by: Stephen Hemminger 
---
 .../linuxapp/kni/ethtool/igb/e1000_api.c   |  1 -
 .../linuxapp/kni/ethtool/igb/e1000_manage.c|  2 -
 .../linuxapp/kni/ethtool/igb/e1000_mbx.c   |  1 -
 .../linuxapp/kni/ethtool/igb/e1000_nvm.c   |  2 -
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h  |  2 +-
 .../linuxapp/kni/ethtool/igb/igb_debugfs.c |  1 -
 .../linuxapp/kni/ethtool/igb/igb_ethtool.c | 26 ++--
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_main.c |  2 +-
 .../linuxapp/kni/ethtool/igb/igb_param.c   |  3 +-
 .../linuxapp/kni/ethtool/igb/igb_procfs.c  | 46 +++---
 .../linuxapp/kni/ethtool/igb/igb_regtest.h |  2 -
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_vmdq.c |  1 -
 lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h  |  4 +-
 .../linuxapp/kni/ethtool/igb/kcompat_ethtool.c | 11 +++---
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c   |  1 -
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_api.c |  1 -
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_common.c  |  1 -
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_main.c|  5 ---
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_sriov.h   |  1 -
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_x540.c|  7 ++--
 .../linuxapp/kni/ethtool/ixgbe/kcompat.h   |  2 +-
 lib/librte_eal/linuxapp/kni/kni_dev.h  |  1 -
 lib/librte_eal/linuxapp/kni/kni_misc.c |  1 -
 lib/librte_eal/linuxapp/kni/kni_vhost.c|  3 +-
 lib/librte_kni/rte_kni.h   |  1 -
 25 files changed, 50 insertions(+), 78 deletions(-)

diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_api.c 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_api.c
index b1d748f..6095d3b 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_api.c
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_api.c
@@ -1157,4 +1157,3 @@ s32 e1000_init_thermal_sensor_thresh(struct e1000_hw *hw)

return E1000_SUCCESS;
 }
-
diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_manage.c 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_manage.c
index e1a2abe..a170039 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_manage.c
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_manage.c
@@ -552,5 +552,3 @@ s32 e1000_load_firmware(struct e1000_hw *hw, u8 *buffer, 
u32 length)

return E1000_SUCCESS;
 }
-
-
diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_mbx.c 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_mbx.c
index 6d004b6..3ef0d98 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_mbx.c
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_mbx.c
@@ -523,4 +523,3 @@ s32 e1000_init_mbx_params_pf(struct e1000_hw *hw)
return E1000_SUCCESS;
}
 }
-
diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_nvm.c 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_nvm.c
index ff42198..6188d00 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_nvm.c
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_nvm.c
@@ -963,5 +963,3 @@ etrack_id:
}
return;
 }
-
-
diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h
index a582f52..e5554ca 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h
@@ -229,7 +229,7 @@ struct igb_lro_stats {
  */
 struct igb_lrohdr {
struct iphdr iph;
-   struct tcphdr th; 
+   struct tcphdr th;
__be32 ts[0];
 };

diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_debugfs.c 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_debugfs.c
index d33c814..c07f9f5 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_debugfs.c
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_debugfs.c
@@ -26,4 +26,3 @@
 
***/

 #include "igb.h"
-
diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
index f3c48b2..af7e68a 100644
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
+++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ethtool.c
@@ -208,7 +208,7 @@ static int igb_get_settings(struct net_device *netdev, 
struct ethtool_cmd *ecmd)

ecmd->port = PORT_FIBRE;
ecmd->transceiver = XCVR_EXTERNAL;
-   } 
+   }

if (hw->mac.autoneg != 1)
ecmd->advertising &= ~(ADVERTISED_Pause |
@@ -1377,7 +1377,7 @@ static int igb_integrated_phy_loopback(struct igb_adapter 
*adapter)
}
} else {
/* enable MII loopback */
-   if (hw->phy.type == e1000_phy_82580) 
+   if (hw->phy.type == e1000_phy_82580)
 

[dpdk-dev] [PATCH 2/9] eal: fix whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Eliminate trailing whitespace, space after tabs, and extra blank lines

Signed-off-by: Stephen Hemminger 
---
 lib/librte_eal/bsdapp/contigmem/contigmem.c|  1 -
 lib/librte_eal/bsdapp/eal/Makefile |  1 -
 lib/librte_eal/bsdapp/eal/eal.c|  1 -
 lib/librte_eal/bsdapp/eal/eal_interrupts.c |  1 -
 lib/librte_eal/common/eal_common_hexdump.c |  3 +--
 lib/librte_eal/common/eal_common_launch.c  |  1 -
 lib/librte_eal/common/eal_common_log.c |  1 -
 lib/librte_eal/common/include/generic/rte_cycles.h |  8 
 lib/librte_eal/common/include/rte_hexdump.h| 20 ++--
 lib/librte_eal/common/include/rte_interrupts.h |  1 -
 lib/librte_eal/common/include/rte_memory.h |  2 +-
 lib/librte_eal/common/include/rte_pci.h| 10 +-
 lib/librte_eal/linuxapp/eal/Makefile   |  1 -
 lib/librte_eal/linuxapp/eal/eal_interrupts.c   |  1 -
 lib/librte_eal/linuxapp/xen_dom0/dom0_mm_misc.c|  2 +-
 15 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/lib/librte_eal/bsdapp/contigmem/contigmem.c 
b/lib/librte_eal/bsdapp/contigmem/contigmem.c
index 6634daa..8ac836d 100644
--- a/lib/librte_eal/bsdapp/contigmem/contigmem.c
+++ b/lib/librte_eal/bsdapp/contigmem/contigmem.c
@@ -230,4 +230,3 @@ contigmem_mmap_single(struct cdev *cdev, vm_ooffset_t 
*offset, vm_size_t size,

return (0);
 }
-
diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index 3d1d9eb..c73ffb6 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -99,4 +99,3 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP)-include/exec-env := \
 DEPDIRS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += lib/librte_eal/common

 include $(RTE_SDK)/mk/rte.lib.mk
-
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 43e8a47..7bd392f 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -569,4 +569,3 @@ rte_eal_process_type(void)
 {
return (rte_config.process_type);
 }
-
diff --git a/lib/librte_eal/bsdapp/eal/eal_interrupts.c 
b/lib/librte_eal/bsdapp/eal/eal_interrupts.c
index cb7d4f1..26a55c7 100644
--- a/lib/librte_eal/bsdapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/bsdapp/eal/eal_interrupts.c
@@ -68,4 +68,3 @@ rte_eal_intr_init(void)
 {
return 0;
 }
-
diff --git a/lib/librte_eal/common/eal_common_hexdump.c 
b/lib/librte_eal/common/eal_common_hexdump.c
index 6135133..d5cbd70 100644
--- a/lib/librte_eal/common/eal_common_hexdump.c
+++ b/lib/librte_eal/common/eal_common_hexdump.c
@@ -103,7 +103,7 @@ rte_memdump(FILE *f, const char * title, const void * buf, 
unsigned int len)

 line[0] = '\0';
 for (i = 0, out = 0; i < len; i++) {
-   // Make sure we do not overrun the line buffer length.
+   // Make sure we do not overrun the line buffer length.
if ( out >= (LINE_LEN - 4) ) {
fprintf(f, "%s", line);
out = 0;
@@ -118,4 +118,3 @@ rte_memdump(FILE *f, const char * title, const void * buf, 
unsigned int len)

 fflush(f);
 }
-
diff --git a/lib/librte_eal/common/eal_common_launch.c 
b/lib/librte_eal/common/eal_common_launch.c
index 152d1c3..229c3a0 100644
--- a/lib/librte_eal/common/eal_common_launch.c
+++ b/lib/librte_eal/common/eal_common_launch.c
@@ -116,4 +116,3 @@ rte_eal_mp_wait_lcore(void)
rte_eal_wait_lcore(lcore_id);
}
 }
-
diff --git a/lib/librte_eal/common/eal_common_log.c 
b/lib/librte_eal/common/eal_common_log.c
index fe3d7d5..c903aa9 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -331,4 +331,3 @@ rte_eal_common_log_init(FILE *default_log)
rte_openlog_stream(default_log);
return 0;
 }
-
diff --git a/lib/librte_eal/common/include/generic/rte_cycles.h 
b/lib/librte_eal/common/include/generic/rte_cycles.h
index 7700f41..8cc21f2 100644
--- a/lib/librte_eal/common/include/generic/rte_cycles.h
+++ b/lib/librte_eal/common/include/generic/rte_cycles.h
@@ -130,12 +130,12 @@ rte_get_hpet_hz(void);
  * then the HPET functions are unavailable and should not be called.
  *
  * @param make_default
- * If set, the hpet timer becomes the default timer whose values are
- * returned by the rte_get_timer_hz/cycles API calls
+ * If set, the hpet timer becomes the default timer whose values are
+ * returned by the rte_get_timer_hz/cycles API calls
  *
  * @return
- * 0 on success,
- * -1 on error, and the make_default parameter is ignored.
+ * 0 on success,
+ * -1 on error, and the make_default parameter is ignored.
  */
 int rte_eal_hpet_init(int make_default);

diff --git a/lib/librte_eal/common/include/rte_hexdump.h 
b/lib/librte_eal/common/include/rte_hexdump.h
index 891c77b..5c18a50 100644
--- a/lib/librte_eal/common/include/rte_hexdump.h
+++ 

[dpdk-dev] [PATCH 3/9] cmdline: fix whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Get rid of trailing whitespace, etc.

Signed-off-by: Stephen Hemminger 
---
 lib/librte_cmdline/cmdline_cirbuf.c | 1 -
 lib/librte_cmdline/cmdline_parse.c  | 1 -
 lib/librte_cmdline/cmdline_rdline.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/lib/librte_cmdline/cmdline_cirbuf.c 
b/lib/librte_cmdline/cmdline_cirbuf.c
index b9f9f4b..f506f88 100644
--- a/lib/librte_cmdline/cmdline_cirbuf.c
+++ b/lib/librte_cmdline/cmdline_cirbuf.c
@@ -464,4 +464,3 @@ cirbuf_get_tail(struct cirbuf * cbuf)
 {
return cbuf->buf[cbuf->end];
 }
-
diff --git a/lib/librte_cmdline/cmdline_parse.c 
b/lib/librte_cmdline/cmdline_parse.c
index dfc885c..24a6ed6 100644
--- a/lib/librte_cmdline/cmdline_parse.c
+++ b/lib/librte_cmdline/cmdline_parse.c
@@ -561,4 +561,3 @@ cmdline_complete(struct cmdline *cl, const char *buf, int 
*state,
}
return 0;
 }
-
diff --git a/lib/librte_cmdline/cmdline_rdline.c 
b/lib/librte_cmdline/cmdline_rdline.c
index f79ebe3..1ef2258 100644
--- a/lib/librte_cmdline/cmdline_rdline.c
+++ b/lib/librte_cmdline/cmdline_rdline.c
@@ -695,4 +695,3 @@ rdline_miniprintf(struct rdline *rdl, const char * buf, 
unsigned int val)
}
}
 }
-
-- 
2.1.4



[dpdk-dev] [PATCH 4/9] vhost: fix trailing whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Signed-off-by: Stephen Hemminger 
---
 lib/librte_vhost/libvirt/qemu-wrap.py | 13 ++---
 lib/librte_vhost/vhost_rxtx.c |  2 +-
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/librte_vhost/libvirt/qemu-wrap.py 
b/lib/librte_vhost/libvirt/qemu-wrap.py
index e2d68a0..5096011 100755
--- a/lib/librte_vhost/libvirt/qemu-wrap.py
+++ b/lib/librte_vhost/libvirt/qemu-wrap.py
@@ -49,17 +49,17 @@
 #
 #3.a) Set the VM to use the launch script
 #
-#  Set the emulator path contained in the
+#  Set the emulator path contained in the
 #   tags
 #
-#  e.g replace /usr/bin/qemu-kvm
+#  e.g replace /usr/bin/qemu-kvm
 #with/usr/bin/qemu-wrap.py
 #
 #   3.b) Set the VM's device's to use vhost-net offload
 #
 #  
-#  
-#  
+#  
+#  
 #  
 #
 # 4. Enable libvirt to access our userpace device file by adding it to
@@ -229,7 +229,7 @@ def get_vhost_fd():
 # flags onto the end
 #
 def modify_netdev_arg(arg):
-   
+
 global fd_list
 vhost_in_use = 0
 s = ''
@@ -259,7 +259,7 @@ def modify_netdev_arg(arg):

 s+=opt

-return s   
+return s


 #
@@ -364,4 +364,3 @@ def main():

 if __name__ == "__main__":
 main()
-
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 4809d32..2da4a02 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -615,7 +615,7 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t 
queue_id,
if (unlikely(m == NULL)) {
RTE_LOG(ERR, VHOST_DATA,
"Failed to allocate memory for mbuf.\n");
-   break;  
+   break;
}
seg_offset = 0;
seg_avail = m->buf_len - RTE_PKTMBUF_HEADROOM;
-- 
2.1.4



[dpdk-dev] [PATCH 6/9] app: fix whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Fix trailing whitespace, space before tab and empty lines at end of file.

Signed-off-by: Stephen Hemminger 
---
 app/cmdline_test/cmdline_test.py  |  3 +--
 app/cmdline_test/cmdline_test_data.py |  1 -
 app/test-pmd/csumonly.c   |  1 -
 app/test-pmd/mempool_anon.c   |  4 ++--
 app/test-pmd/testpmd.c|  8 +++
 app/test/autotest.py  |  1 -
 app/test/autotest_data.py | 28 
 app/test/autotest_runner.py   | 41 +--
 app/test/autotest_test_funcs.py   |  1 -
 app/test/process.h| 12 +-
 app/test/test_acl.h   |  4 ++--
 app/test/test_sched.c |  8 +++
 12 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/app/cmdline_test/cmdline_test.py b/app/cmdline_test/cmdline_test.py
index 5d7c7be..8efc5ea 100755
--- a/app/cmdline_test/cmdline_test.py
+++ b/app/cmdline_test/cmdline_test.py
@@ -67,7 +67,7 @@ def runHistoryTest(child):
while i < history_size / 10:
# add 1 to prevent from parsing as octals
child.send("1" + str(i).zfill(8) + cmdline_test_data.ENTER)
-   # the app will simply print out the number  
+   # the app will simply print out the number
child.expect(str(i + 1), timeout=1)
i += 1
# scroll back history
@@ -111,4 +111,3 @@ except:
sys.exit(1)
 child.close()
 sys.exit(0)
-
diff --git a/app/cmdline_test/cmdline_test_data.py 
b/app/cmdline_test/cmdline_test_data.py
index a5d8dd9..b1945a5 100644
--- a/app/cmdline_test/cmdline_test_data.py
+++ b/app/cmdline_test/cmdline_test_data.py
@@ -309,4 +309,3 @@ tests = [
 "Sequence" : CTRL_D*3,
 "Result" : None},
 ]
-
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index c180ff2..950ea82 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -706,4 +706,3 @@ struct fwd_engine csum_fwd_engine = {
.port_fwd_end   = NULL,
.packet_fwd = pkt_burst_checksum_forward,
 };
-
diff --git a/app/test-pmd/mempool_anon.c b/app/test-pmd/mempool_anon.c
index 559a625..69bbef3 100644
--- a/app/test-pmd/mempool_anon.c
+++ b/app/test-pmd/mempool_anon.c
@@ -130,8 +130,8 @@ mempool_anon_create(const char *name, unsigned elt_num, 
unsigned elt_size,
(rc = get_phys_map(va, pa, pg_num, pg_sz)) == 0) {

/*
-* Check that allocated size is big enough to hold elt_num
-* objects and a calcualte how many bytes are actually required.
+* Check that allocated size is big enough to hold elt_num
+* objects and a calcualte how many bytes are actually required.
 */

if ((usz = rte_mempool_xmem_usage(va, elt_num, total_size, pa,
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 83a3d74..82b465d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1822,10 +1822,10 @@ get_eth_dcb_conf(struct rte_eth_conf *eth_conf, struct 
dcb_config *dcb_conf)
 {
 uint8_t i;

-   /*
-* Builds up the correct configuration for dcb+vt based on the vlan 
tags array
-* given above, and the number of traffic classes available for use.
-*/
+   /*
+* Builds up the correct configuration for dcb+vt based on the vlan 
tags array
+* given above, and the number of traffic classes available for use.
+*/
if (dcb_conf->dcb_mode == DCB_VT_ENABLED) {
struct rte_eth_vmdq_dcb_conf vmdq_rx_conf;
struct rte_eth_vmdq_dcb_tx_conf vmdq_tx_conf;
diff --git a/app/test/autotest.py b/app/test/autotest.py
index a79db10..5d6adb6 100644
--- a/app/test/autotest.py
+++ b/app/test/autotest.py
@@ -73,4 +73,3 @@ for test_group in autotest_data.non_parallel_test_group_list:
runner.add_non_parallel_test_group(test_group)

 runner.run_all_tests()
-
diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 618a946..02921ff 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -56,7 +56,7 @@ parallel_test_group_list = [
 {
"Prefix":   "group_1",
"Memory" :  all_sockets(8),
-   "Tests" :   
+   "Tests" :
[
{
 "Name" :   "Timer autotest",
@@ -69,7 +69,7 @@ parallel_test_group_list = [
 "Command" :"debug_autotest",
 "Func" :   default_autotest,
 "Report" : None,
-   },  
+   },
{
 "Name" :   "Errno autotest",
 "Command" :"errno_autotest",
@@ -87,7 +87,7 @@ parallel_test_group_list = [
 "Command" :"common_autotest",
 "Func" :   default_autotest,
  

[dpdk-dev] [PATCH 7/9] examples: fix whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Signed-off-by: Stephen Hemminger 
---
 examples/cmdline/commands.c  |  1 -
 .../dpdk_qat/config_files/shumway/dh89xxcc_qa_dev0.conf  |  2 --
 .../dpdk_qat/config_files/shumway/dh89xxcc_qa_dev1.conf  |  2 --
 .../dpdk_qat/config_files/stargo/dh89xxcc_qa_dev0.conf   |  2 --
 examples/kni/main.c  |  1 -
 examples/l2fwd/main.c|  1 -
 examples/l3fwd-power/main.c  |  8 
 examples/multi_process/client_server_mp/mp_server/args.c |  1 -
 examples/netmap_compat/lib/compat_netmap.c   |  6 +++---
 examples/qos_sched/app_thread.c  |  2 --
 examples/qos_sched/args.c|  1 -
 examples/qos_sched/cfg_file.c|  2 --
 examples/qos_sched/main.c|  1 -
 examples/qos_sched/main.h|  2 +-
 examples/qos_sched/stats.c   |  1 -
 examples/quota_watermark/qw/init.c   |  2 +-
 examples/quota_watermark/qw/init.h   |  1 -
 examples/vhost/main.c|  9 -
 examples/vhost_xen/main.c| 16 
 examples/vhost_xen/vhost_monitor.c   |  6 ++
 20 files changed, 23 insertions(+), 44 deletions(-)

diff --git a/examples/cmdline/commands.c b/examples/cmdline/commands.c
index 8c6c11b..f3ba247 100644
--- a/examples/cmdline/commands.c
+++ b/examples/cmdline/commands.c
@@ -281,4 +281,3 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)_help,
NULL,
 };
-
diff --git a/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev0.conf 
b/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev0.conf
index 864a36e..9e1c1d1 100644
--- a/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev0.conf
+++ b/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev0.conf
@@ -291,5 +291,3 @@ Cy15CoreAffinity = 23
 NumberCyInstances = 0
 NumberDcInstances = 0
 NumProcesses = 0
-
-
diff --git a/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev1.conf 
b/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev1.conf
index 1783cc8..3e8d8b6 100644
--- a/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev1.conf
+++ b/examples/dpdk_qat/config_files/shumway/dh89xxcc_qa_dev1.conf
@@ -290,5 +290,3 @@ Cy15CoreAffinity = 31
 NumberCyInstances = 0
 NumberDcInstances = 0
 NumProcesses = 0
-
-
diff --git a/examples/dpdk_qat/config_files/stargo/dh89xxcc_qa_dev0.conf 
b/examples/dpdk_qat/config_files/stargo/dh89xxcc_qa_dev0.conf
index 4b09070..c3a85de 100644
--- a/examples/dpdk_qat/config_files/stargo/dh89xxcc_qa_dev0.conf
+++ b/examples/dpdk_qat/config_files/stargo/dh89xxcc_qa_dev0.conf
@@ -233,5 +233,3 @@ Cy7CoreAffinity = 7
 NumberCyInstances = 0
 NumberDcInstances = 0
 NumProcesses = 0
-
-
diff --git a/examples/kni/main.c b/examples/kni/main.c
index 96ca473..6f74d8e 100644
--- a/examples/kni/main.c
+++ b/examples/kni/main.c
@@ -926,4 +926,3 @@ main(int argc, char** argv)

return 0;
 }
-
diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c
index 6ed7690..720fd5a 100644
--- a/examples/l2fwd/main.c
+++ b/examples/l2fwd/main.c
@@ -705,4 +705,3 @@ main(int argc, char **argv)

return 0;
 }
-
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 6ac342b..6057059 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -873,7 +873,7 @@ main_loop(__attribute__((unused)) void *dummy)
rx_queue->freq_up_hint =
power_freq_scaleup_heuristic(lcore_id,
portid, queueid);
-   }
+   }

/* Prefetch first packets */
for (j = 0; j < PREFETCH_OFFSET && j < nb_rx; j++) {
@@ -915,8 +915,8 @@ main_loop(__attribute__((unused)) void *dummy)
/**
 * All Rx queues empty in recent consecutive polls,
 * sleep in a conservative manner, meaning sleep as
-* less as possible.
-*/
+* less as possible.
+*/
for (i = 1, lcore_idle_hint =
qconf->rx_queue_list[0].idle_hint;
i < qconf->n_rx_queue; ++i) {
@@ -929,7 +929,7 @@ main_loop(__attribute__((unused)) void *dummy)
/**
 * execute "pause" instruction to avoid context
 * switch for short sleep.
-*/
+*/

[dpdk-dev] [PATCH 8/9] mk, scripts: remove useless blank lines

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Signed-off-by: Stephen Hemminger 
---
 mk/rte.extapp.mk | 1 -
 mk/rte.extlib.mk | 1 -
 mk/rte.extobj.mk | 1 -
 mk/toolchain/gcc/rte.toolchain-compat.mk | 1 -
 mk/toolchain/icc/rte.toolchain-compat.mk | 1 -
 scripts/gen-config-h.sh  | 1 -
 scripts/validate-abi.sh  | 2 --
 7 files changed, 8 deletions(-)

diff --git a/mk/rte.extapp.mk b/mk/rte.extapp.mk
index 40ff82c..b4d1ef6 100644
--- a/mk/rte.extapp.mk
+++ b/mk/rte.extapp.mk
@@ -50,4 +50,3 @@ all:
 else
 include $(RTE_SDK)/mk/rte.app.mk
 endif
-
diff --git a/mk/rte.extlib.mk b/mk/rte.extlib.mk
index ac5e84f..ba066bc 100644
--- a/mk/rte.extlib.mk
+++ b/mk/rte.extlib.mk
@@ -50,4 +50,3 @@ all:
 else
 include $(RTE_SDK)/mk/rte.lib.mk
 endif
-
diff --git a/mk/rte.extobj.mk b/mk/rte.extobj.mk
index cb2f996..253de28 100644
--- a/mk/rte.extobj.mk
+++ b/mk/rte.extobj.mk
@@ -50,4 +50,3 @@ all:
 else
 include $(RTE_SDK)/mk/rte.obj.mk
 endif
-
diff --git a/mk/toolchain/gcc/rte.toolchain-compat.mk 
b/mk/toolchain/gcc/rte.toolchain-compat.mk
index 05aa37f..61bb5b7 100644
--- a/mk/toolchain/gcc/rte.toolchain-compat.mk
+++ b/mk/toolchain/gcc/rte.toolchain-compat.mk
@@ -84,4 +84,3 @@ else
MACHINE_CFLAGS := $(filter-out -march% -mtune% 
-msse%,$(MACHINE_CFLAGS))
endif
 endif
-
diff --git a/mk/toolchain/icc/rte.toolchain-compat.mk 
b/mk/toolchain/icc/rte.toolchain-compat.mk
index 621afcd..4134466 100644
--- a/mk/toolchain/icc/rte.toolchain-compat.mk
+++ b/mk/toolchain/icc/rte.toolchain-compat.mk
@@ -73,4 +73,3 @@ else
MACHINE_CFLAGS := $(patsubst -march=%,-xSSE3,$(MACHINE_CFLAGS))
endif
 endif
-
diff --git a/scripts/gen-config-h.sh b/scripts/gen-config-h.sh
index d36efd6..1a2436c 100755
--- a/scripts/gen-config-h.sh
+++ b/scripts/gen-config-h.sh
@@ -42,4 +42,3 @@ sed 's,CONFIG_\(.*\)=\(.*\)$,#undef \1\
 #define \1 \2,' |
 sed 's,\# CONFIG_\(.*\) is not set$,#undef \1,'
 echo "#endif /* __RTE_CONFIG_H */"
-
diff --git a/scripts/validate-abi.sh b/scripts/validate-abi.sh
index 369ea8a..1747b8b 100755
--- a/scripts/validate-abi.sh
+++ b/scripts/validate-abi.sh
@@ -241,5 +241,3 @@ done
 git reset --hard
 log "INFO" "ABI CHECK COMPLETE.  REPORTS ARE IN compat_report directory"
 cleanup_and_exit 0
-
-
-- 
2.1.4



[dpdk-dev] [PATCH 9/9] drivers: fix whitespace

2015-06-04 Thread Stephen Hemminger
From: Stephen Hemminger 

Note: ignored whitespace issues in base (external code).

Signed-off-by: Stephen Hemminger 
---
 drivers/net/e1000/em_rxtx.c | 1 -
 drivers/net/e1000/igb_rxtx.c| 1 -
 drivers/net/pcap/rte_eth_pcap.c | 2 +-
 3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 3a79e8b..fdc825f 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -1862,4 +1862,3 @@ eth_em_tx_init(struct rte_eth_dev *dev)
/* This write will effectively turn on the transmit unit. */
E1000_WRITE_REG(hw, E1000_TCTL, tctl);
 }
-
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index f586311..43d6703 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -2394,4 +2394,3 @@ eth_igbvf_tx_init(struct rte_eth_dev *dev)
}

 }
-
diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
index ff9b534..a6ed5bd 100644
--- a/drivers/net/pcap/rte_eth_pcap.c
+++ b/drivers/net/pcap/rte_eth_pcap.c
@@ -760,7 +760,7 @@ rte_pmd_init_internals(const char *name, const unsigned 
nb_rx_queues,

return 0;

-error: 
+error:
rte_free(data);
rte_free(pci_dev);
rte_free(*internals);
-- 
2.1.4



[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Stephen Hemminger
On Thu, 4 Jun 2015 09:55:42 -0400
Neil Horman  wrote:

> That said, I'm not sure theres much value in adding this to the API.  For one,
> it implies that dpdk arguments need to come first on the command line.  While
> all the example applications do that, theres no requirement that they do so, 
> and
> this function silently implies that they have to, so any existing applications
> in the wild that violate that assumption are enjoined from using this

I found that the only way to support daemon command line args is to put local
arguments first, then call EAL to parse it's args.


[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Wiles, Keith


From: David Marchand mailto:david.march...@6wind.com>>
Date: Thursday, June 4, 2015 at 9:43 AM
To: Keith Wiles mailto:keith.wiles at intel.com>>
Cc: Neil Horman mailto:nhorman at tuxdriver.com>>, 
"dev at dpdk.org" mailto:dev at 
dpdk.org>>
Subject: Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at 
rte_eal_init time

On Thu, Jun 4, 2015 at 4:27 PM, Wiles, Keith mailto:keith.wiles at intel.com>> wrote:
Hi Neil and Stephen,

I agree this is not saving instructions and adding performance, but of
code clutter and providing a layered model for the developer. The
rte_eal_init() routine still exists and I was not trying to remove that
API only layer a convenient API for common constructs.
>
>Its not a bad addition, I'm just not sure its worth having to take on the
>additional API surface to include.  I'd be more supportive if you could
>enhance
>the function to allow the previously mentioned before/after flexibiilty.
>Then
>we could just deprecate rte_eal_init as an API call entirely, and use this
>instead.

I can see we can create an API to add support for doing the applications
args first or after, but would that even be acceptable?

What's the point ?
Adding stuff just for saving lines ?
Are you serious about this ?

Wow, OK dropped!


--
David Marchand


[dpdk-dev] [PATCH 1/9] kni: fix whitespace

2015-06-04 Thread Thomas Monjalon
2015-06-04 07:43, Stephen Hemminger:
> From: Stephen Hemminger 
> 
> Ran this code base through a script which:
>   - removes trailing whitespace
>   - removes space before tabs
>   - removes blank lines at end of file
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  .../linuxapp/kni/ethtool/igb/e1000_api.c   |  1 -
>  .../linuxapp/kni/ethtool/igb/e1000_manage.c|  2 -
>  .../linuxapp/kni/ethtool/igb/e1000_mbx.c   |  1 -
>  .../linuxapp/kni/ethtool/igb/e1000_nvm.c   |  2 -
>  lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h  |  2 +-
>  .../linuxapp/kni/ethtool/igb/igb_debugfs.c |  1 -
>  .../linuxapp/kni/ethtool/igb/igb_ethtool.c | 26 ++--
>  lib/librte_eal/linuxapp/kni/ethtool/igb/igb_main.c |  2 +-
>  .../linuxapp/kni/ethtool/igb/igb_param.c   |  3 +-
>  .../linuxapp/kni/ethtool/igb/igb_procfs.c  | 46 
> +++---
>  .../linuxapp/kni/ethtool/igb/igb_regtest.h |  2 -
>  lib/librte_eal/linuxapp/kni/ethtool/igb/igb_vmdq.c |  1 -
>  lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h  |  4 +-
>  .../linuxapp/kni/ethtool/igb/kcompat_ethtool.c | 11 +++---
>  .../linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c   |  1 -
>  .../linuxapp/kni/ethtool/ixgbe/ixgbe_api.c |  1 -
>  .../linuxapp/kni/ethtool/ixgbe/ixgbe_common.c  |  1 -
>  .../linuxapp/kni/ethtool/ixgbe/ixgbe_main.c|  5 ---
>  .../linuxapp/kni/ethtool/ixgbe/ixgbe_sriov.h   |  1 -
>  .../linuxapp/kni/ethtool/ixgbe/ixgbe_x540.c|  7 ++--
>  .../linuxapp/kni/ethtool/ixgbe/kcompat.h   |  2 +-

Helin, do you approve patching kni base driver?


[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Wiles, Keith
Hmmm, replied in HTML.

>On Thu, Jun 4, 2015 at 4:27 PM, Wiles, Keith
> wrote:
>
>Hi Neil and Stephen,
>
>I agree this is not saving instructions and adding performance, but of
>code clutter and providing a layered model for the developer. The
>rte_eal_init() routine still exists and I was not trying to remove that
>API only layer a convenient API for common constructs.
>>
>>Its not a bad addition, I'm just not sure its worth having to take on the
>>additional API surface to include.  I'd be more supportive if you could
>>enhance
>>the function to allow the previously mentioned before/after flexibiilty.
>>Then
>>we could just deprecate rte_eal_init as an API call entirely, and use
>>this
>>instead.
>
>I can see we can create an API to add support for doing the applications
>args first or after, but would that even be acceptable?
>
>
>
>What's the point ?
>Adding stuff just for saving lines ?
>Are you serious about this ?

Wow, OK it is dropped.
>
>
>-- 
>
>David Marchand



[dpdk-dev] [PATCH 2/2] ethtool: add new library to provide ethtool-alike APIs

2015-06-04 Thread Stephen Hemminger
On Wed, 3 Jun 2015 02:09:39 +
"Andrew Harvey (agh)"  wrote:

> I believe that their is value in this interface for software stacks not
> based on Linux being moved toward DPDK that need simple operations like
> getting the mac address.  Some of these stacks have a dearth of resources
> available and dedicating a core/thread to KNI to get/set a mac address
> is considered excessive. There are also issues with 32/64 bit kernel
> integration
> using KNI.  If the ethtool interface is not the correct interface then
> please help me
> understand what should/could have been used. If ethtool is considered 'old
> and clunky?
> Stephen's and your input would be valuable in designing another interface
> with
> similar properties.  The use-case is pretty simple and there is no plans
> for moving
> anything back into the kernel on the contrary its the complete opposite.
> 
> ? Andy

We have DPDK API's to do this, and any added wrappers make it bigger.
I don't see why calling your ethtool API is better than calling
rte_eth* API.

If there is a missing functionality in the rte_ethXXX api's for an
application then add that. For example: rte_eth_mac_addr_get()


[dpdk-dev] [PATCH 1/3] kni: minor opto

2015-06-04 Thread Thomas Monjalon
2015-06-04 14:40, Bruce Richardson:
> On Thu, Jun 04, 2015 at 02:39:17PM +0100, Bruce Richardson wrote:
> > On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> > > Don't need the 'safe' version of list_for_each_entry() if you aren't 
> > > deleting from the list as you iterate over it
> > > 
> > > Signed-off-by: Jay Rolette 
> > 
> > Acked-by: Bruce Richardson 
> > 
> Forgot to mention though, that the commit title needs to be a little more
> descriptive.

So you should not ack this version ;)



[dpdk-dev] [PATCH 1/3] kni: minor opto

2015-06-04 Thread Bruce Richardson
On Thu, Jun 04, 2015 at 05:02:06PM +0200, Thomas Monjalon wrote:
> 2015-06-04 14:40, Bruce Richardson:
> > On Thu, Jun 04, 2015 at 02:39:17PM +0100, Bruce Richardson wrote:
> > > On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> > > > Don't need the 'safe' version of list_for_each_entry() if you aren't 
> > > > deleting from the list as you iterate over it
> > > > 
> > > > Signed-off-by: Jay Rolette 
> > > 
> > > Acked-by: Bruce Richardson 
> > > 
> > Forgot to mention though, that the commit title needs to be a little more
> > descriptive.
> 
> So you should not ack this version ;)
> 
Code and text description looked fine, so I thought an ack otherwise 
appropriate.
However, I will refrain from doing so in future :-)

/Bruce


[dpdk-dev] [RFC PATCH V2 3/4] i40e: increase ASQ_DELAY_MS to 100 in i40evf_wait_cmd_done()

2015-06-04 Thread Bernard Iremonger
Increase delay to avoid i40evf_read_pfmsg() failures.

Signed-off-by: Bernard Iremonger 
---
 drivers/net/i40e/i40e_ethdev_vf.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index 843ab9b..c1add33 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -299,7 +299,7 @@ i40evf_wait_cmd_done(struct rte_eth_dev *dev,
enum i40evf_aq_result ret;

 #define MAX_TRY_TIMES 10
-#define ASQ_DELAY_MS  50
+#define ASQ_DELAY_MS  100
do {
/* Delay some time first */
rte_delay_ms(ASQ_DELAY_MS);
-- 
1.7.4.1



[dpdk-dev] [RFC PATCH V2 1/4] i40e: changes to support PCI Port Hotplug

2015-06-04 Thread Bernard Iremonger
This patch depends on the Port Hotplug Framework.
It implements the eth_dev_uninit functions for rte_i40e_pmd and
rte_i40evf_pmd.

Signed-off-by: Bernard Iremonger 
---
 drivers/net/i40e/i40e_ethdev.c|   79 -
 drivers/net/i40e/i40e_ethdev_vf.c |   45 -
 drivers/net/i40e/i40e_pf.c|   34 
 drivers/net/i40e/i40e_pf.h|1 +
 4 files changed, 157 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index da6c0b5..7bf9532 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -107,6 +107,7 @@
(1UL << RTE_ETH_FLOW_L2_PAYLOAD))

 static int eth_i40e_dev_init(struct rte_eth_dev *eth_dev);
+static int eth_i40e_dev_uninit(struct rte_eth_dev *eth_dev);
 static int i40e_dev_configure(struct rte_eth_dev *dev);
 static int i40e_dev_start(struct rte_eth_dev *dev);
 static void i40e_dev_stop(struct rte_eth_dev *dev);
@@ -268,9 +269,11 @@ static struct eth_driver rte_i40e_pmd = {
{
.name = "rte_i40e_pmd",
.id_table = pci_id_i40e_map,
-   .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
+   .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
+   RTE_PCI_DRV_DETACHABLE,
},
.eth_dev_init = eth_i40e_dev_init,
+   .eth_dev_uninit = eth_i40e_dev_uninit,
.dev_private_size = sizeof(struct i40e_adapter),
 };

@@ -405,6 +408,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
hw->subsystem_device_id = pci_dev->id.subsystem_device_id;
hw->bus.device = pci_dev->addr.devid;
hw->bus.func = pci_dev->addr.function;
+   hw->adapter_stopped = 0;

/* Make sure all is clean before doing PF reset */
i40e_clear_hw(hw);
@@ -584,6 +588,76 @@ err_get_capabilities:
 }

 static int
+eth_i40e_dev_uninit(struct rte_eth_dev *dev)
+{
+   struct rte_pci_device *pci_dev;
+   struct i40e_hw *hw;
+   struct i40e_filter_control_settings settings;
+   int ret;
+   uint8_t aq_fail = 0;
+   unsigned i;
+
+   PMD_INIT_FUNC_TRACE();
+
+   if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+   return 0;
+
+   hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+   pci_dev = dev->pci_dev;
+
+   if (hw->adapter_stopped == 0)
+   i40e_dev_close(dev);
+
+   dev->dev_ops = NULL;
+   dev->rx_pkt_burst = NULL;
+   dev->tx_pkt_burst = NULL;
+
+   /* Disable LLDP */
+   ret = i40e_aq_stop_lldp(hw, true, NULL);
+   if (ret != I40E_SUCCESS) /* Its failure can be ignored */
+   PMD_INIT_LOG(INFO, "Failed to stop lldp");
+
+   /* Clear PXE mode */
+   i40e_clear_pxe_mode(hw);
+
+   /* Unconfigure filter control */
+   memset(, 0, sizeof(settings));
+   ret = i40e_set_filter_control(hw, );
+   if (ret)
+   PMD_INIT_LOG(WARNING, "setup_pf_filter_control failed: %d",
+   ret);
+
+   /* Disable flow control */
+   hw->fc.requested_mode = I40E_FC_NONE;
+   i40e_set_fc(hw, _fail, TRUE);
+
+   /* uninitialize pf host driver */
+   i40e_pf_host_uninit(dev);
+
+   for (i = 0; i < dev->data->nb_rx_queues; i++) {
+   i40e_dev_rx_queue_release(dev->data->rx_queues[i]);
+   dev->data->rx_queues[i] = NULL;
+   }
+
+   for (i = 0; i < dev->data->nb_tx_queues; i++) {
+   i40e_dev_tx_queue_release(dev->data->tx_queues[i]);
+   dev->data->tx_queues[i] = NULL;
+   }
+
+   rte_free(dev->data->mac_addrs);
+   dev->data->mac_addrs = NULL;
+
+   /* disable uio intr before callback unregister */
+   rte_intr_disable(&(pci_dev->intr_handle));
+
+   /* register callback func to eal lib */
+   rte_intr_callback_unregister(&(pci_dev->intr_handle),
+   i40e_dev_interrupt_handler, (void *)dev);
+
+   return 0;
+}
+
+static int
 i40e_dev_configure(struct rte_eth_dev *dev)
 {
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
@@ -858,6 +932,8 @@ i40e_dev_start(struct rte_eth_dev *dev)
struct i40e_vsi *main_vsi = pf->main_vsi;
int ret, i;

+   hw->adapter_stopped = 0;
+
if ((dev->data->dev_conf.link_duplex != ETH_LINK_AUTONEG_DUPLEX) &&
(dev->data->dev_conf.link_duplex != ETH_LINK_FULL_DUPLEX)) {
PMD_INIT_LOG(ERR, "Invalid link_duplex (%hu) for port %hhu",
@@ -965,6 +1041,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
PMD_INIT_FUNC_TRACE();

i40e_dev_stop(dev);
+   hw->adapter_stopped = 1;

/* Disable interrupt */
i40e_pf_disable_irq0(hw);
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index 9f92a2f..843ab9b 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1146,6 

[dpdk-dev] [RFC PATCH V2 2/4] i40e: release vmdq vsi's in dev_close

2015-06-04 Thread Bernard Iremonger

Signed-off-by: Bernard Iremonger 
---
 drivers/net/i40e/i40e_ethdev.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 7bf9532..55fee6b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1037,6 +1037,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
uint32_t reg;
+   int i;

PMD_INIT_FUNC_TRACE();

@@ -1054,6 +1055,14 @@ i40e_dev_close(struct rte_eth_dev *dev)
i40e_fdir_teardown(pf);
i40e_vsi_release(pf->main_vsi);

+   for (i = 0; i < pf->nb_cfg_vmdq_vsi; i++) {
+   i40e_vsi_release(pf->vmdq[i].vsi);
+   pf->vmdq[i].vsi = NULL;
+   }
+
+   rte_free(pf->vmdq);
+   pf->vmdq = NULL;
+
/* shutdown the adminq */
i40e_aq_queue_shutdown(hw, true);
i40e_shutdown_adminq(hw);
-- 
1.7.4.1



[dpdk-dev] [RFC PATCH V2 4/4] i40e: call _clear_cmd() when error occurs

2015-06-04 Thread Bernard Iremonger
_clear_cmd() was not being called in failure situations,
resulting in the next command also failing.
Fix several typos.

Signed-off-by: Bernard Iremonger 
---
 drivers/net/i40e/i40e_ethdev_vf.c |   11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev_vf.c 
b/drivers/net/i40e/i40e_ethdev_vf.c
index c1add33..b783700 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -361,6 +361,7 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct 
vf_cmd_info *args)
 args->in_args, args->in_args_size, NULL);
if (err) {
PMD_DRV_LOG(ERR, "fail to send cmd %d", args->ops);
+   _clear_cmd(vf);
return err;
}

@@ -368,8 +369,10 @@ i40evf_execute_vf_cmd(struct rte_eth_dev *dev, struct 
vf_cmd_info *args)
/* read message and it's expected one */
if (!err && args->ops == info.ops)
_clear_cmd(vf);
-   else if (err)
+   else if (err) {
PMD_DRV_LOG(ERR, "Failed to read message from AdminQ");
+   _clear_cmd(vf);
+   }
else if (args->ops != info.ops)
PMD_DRV_LOG(ERR, "command mismatch, expect %u, get %u",
args->ops, info.ops);
@@ -794,7 +797,7 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
/* Stop TX queues first */
for (i = 0; i < dev->data->nb_tx_queues; i++) {
if (i40evf_dev_tx_queue_stop(dev, i) != 0) {
-   PMD_DRV_LOG(ERR, "Fail to start queue %u", i);
+   PMD_DRV_LOG(ERR, "Fail to stop queue %u", i);
return -1;
}
}
@@ -802,7 +805,7 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
/* Then stop RX queues */
for (i = 0; i < dev->data->nb_rx_queues; i++) {
if (i40evf_dev_rx_queue_stop(dev, i) != 0) {
-   PMD_DRV_LOG(ERR, "Fail to start queue %u", i);
+   PMD_DRV_LOG(ERR, "Fail to stop queue %u", i);
return -1;
}
}
@@ -1431,7 +1434,7 @@ i40evf_dev_tx_queue_stop(struct rte_eth_dev *dev, 
uint16_t tx_queue_id)
err = i40evf_switch_queue(dev, FALSE, tx_queue_id, FALSE);

if (err) {
-   PMD_DRV_LOG(ERR, "Failed to switch TX queue %u of",
+   PMD_DRV_LOG(ERR, "Failed to switch TX queue %u off",
tx_queue_id);
return err;
}
-- 
1.7.4.1



[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Thomas F Herbert


On 6/4/15 9:55 AM, Neil Horman wrote:
> On Thu, Jun 04, 2015 at 11:50:33AM +, Wiles, Keith wrote:
>> Hi Stephen
>>
>> On 6/3/15, 7:12 PM, "Stephen Hemminger"  
>> wrote:
>>
>>> On Wed,  3 Jun 2015 13:49:53 -0500
>>> Keith Wiles  wrote:
>>>
 +/* Launch threads, called at application init() and parse app args. */
 +int
 +rte_eal_init_parse(int argc, char **argv,
 +  int (*parse)(int, char **))
 +{
 +  int ret;
 +
 +  ret = rte_eal_init(argc, argv);
 +  if ((ret >= 0) && (parse != NULL)) {
 +  argc -= ret;
 +  argv += ret;
>>>
>>> This won't work C is call by value.
>>
>> I tested this routine with Pktgen (again), which has a number of
>> application options and it appears to work correctly. Can you explain why
>> this will not work?
>>
>> Regards,
>> ++Keith
>>>
>>
>>
>
> Stephen was thinking that your intent was to have argc, and argv modified at 
> the
> call site of this function (i.e. if you called rte_eal_init_parse from main(),
> then after the call to rte_ela_init_parse, argc would be reduced by ret and 
> argv
> would point forward in memory ret bytes in the main function, but they wont.  
> It
> doesn't matter though, because you return ret, so the caller can do that
> movement themselves.  As you note, it works.
>
> Note that if it was your intention to have argc and argv modified at the call
> site, then Stephen is right and this is broken, you need to modify the 
> prototype
> to be:
> int rte_eal_init_parse(int *argc, char ***argv)
>
> and do a dereference when modifying the parameters so the change is seen at 
> the
> call site.
>
> That said, I'm not sure theres much value in adding this to the API.  For one,
> it implies that dpdk arguments need to come first on the command line.  While
> all the example applications do that, theres no requirement that they do so, 
> and
> this function silently implies that they have to, so any existing applications
> in the wild that violate that assumption are enjoined from using this
>
> It also doesn't really save any code.  If we pick an example app (I'll us
> l2fwd-jobstats), We currently have this:
>
>   /* init EAL */
>  ret = rte_eal_init(argc, argv);
>  if (ret < 0)
>  rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
>  argc -= ret;
>  argv += ret;
>
>  /* parse application arguments (after the EAL ones) */
>  ret = l2fwd_parse_args(argc, argv);
>   if (ret < 0)
>  rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");
>
> With your new API we would get this:
>
>   ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
>  if (ret < 0)
>  rte_exit(EXIT_FAILURE, "Invalid arguments - not sure 
> what\n");
>
> Its definately 5 fewer lines of source, but it doesn't save any execution
> instructions, and for the effort of that, you loose the ability to determine 
> if
> it was a DPDK argument or an application argument that failed.
>
> Its not a bad addition, I'm just not sure its worth having to take on the
> additional API surface to include.  I'd be more supportive if you could 
> enhance
> the function to allow the previously mentioned before/after flexibiilty.  Then
> we could just deprecate rte_eal_init as an API call entirely, and use this
> instead.
+1.

Also, I think rte_set_application_usage_hook() callback could be used by 
app writers for implementing usage() for a conventional  " -h" 
like capability to print all usage including both eal and app specific 
args even if the eal args are not correct. This is an alternative to 
calling eal_init() first and bombing before printing all usage.

--TFH

> Neil
>


[dpdk-dev] mempool with custom alignment

2015-06-04 Thread Dax Rawal
Thanks, Konstantin.

On Thu, Jun 4, 2015 at 1:45 AM, Ananyev, Konstantin <
konstantin.ananyev at intel.com> wrote:

>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Dax Rawal
> > Sent: Wednesday, June 03, 2015 7:30 PM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] mempool with custom alignment
> >
> > Hi,
> > I use mempool APIs to allocate DMA-able descriptor ring and buffers so
> that
> > I get both physical and virtual addresses of allocated memory. I buffers
> I
> > get from the mempool APIs are 64 byte aligned. My requirement is 256byte
> > alignment. How to achieve this? mempool APIs do not seem to take
> alignment
> > parameters.
> >
> > Thanks,
> > -Dax
>
> Something like that:
> http://patchwork.dpdk.org/ml/archives/dev/2014-October/006595.html
> should work, I think.
> Konstantin
>


[dpdk-dev] [PATCH 01/11] ip_pipeline: add parsing for config files with new syntax

2015-06-04 Thread Dumitrescu, Cristian


> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Stephen
> Hemminger
> Sent: Monday, June 1, 2015 2:34 PM
> To: Gajdzica, MaciejX T
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 01/11] ip_pipeline: add parsing for config
> files with new syntax
> 
> On Fri, 29 May 2015 17:43:08 +0200
> Maciej Gajdzica  wrote:
> 
> > +/**
> > + * Find object of name *name* in *obj_array* which is constant size array
> of
> > + * elements that have field *name*.
> > + *
> > + * @param obj_array
> > + *  Constant size array
> > + * @param name
> > + *  name of object to find.
> > + * @return
> > + *  Pointer to object in *obj_array* or NULL if not found.
> > + */
> > +#define APP_PARAM_FIND(obj_array, key)  \
> > +({  \
> > +   ssize_t obj_idx;\
> > +   const ssize_t obj_count = RTE_DIM(obj_array);   \
> > +\
> > +   for (obj_idx = 0; obj_idx < obj_count; obj_idx++) { \
> > +   if (!APP_PARAM_VALID(&((obj_array)[obj_idx])))  \
> > +   continue;   \
> > +   \
> > +   if (strcmp(key, (obj_array)[obj_idx].name) == 0)\
> > +   break;  \
> > +   }   \
> > +   obj_idx < obj_count ? obj_idx : -ENOENT;\
> > +})
> 
> Converting all the functions to macro's is a step backwards in several ways.
>  * macro's are hard to support
>  * macro's lead to lots of programming errors
>  * macro's look ugly
> 
> Why not use real functions, or make the example into C++ if you have
> to do generic programming.

We are using macros here only because C language does not offer us a better 
choice (i.e. support for templates). The alternative would be to write a 
quasi-identical function per each object type, which would lead to unnecessary 
code duplication.

We did our best to keep the number of macros small and to implement each macro 
as straightforward as possible.

All the DPDK sample applications are written in C, so this is the main reason 
we want to keep this application as C code. As people expect C code from DPDK 
sample apps, it is easier for people to reuse parts of this application.




[dpdk-dev] [PATCH 5/5] rte_sched: allow reading without clearing

2015-06-04 Thread Dumitrescu, Cristian


> -Original Message-
> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Wednesday, May 27, 2015 7:10 PM
> To: Dumitrescu, Cristian
> Cc: dev at dpdk.org; Stephen Hemminger
> Subject: [PATCH 5/5] rte_sched: allow reading without clearing
> 
> The rte_sched statistics API should allow reading statistics without
> clearing. Make auto-clear optional. In this version, this is handled
> by deprecating the old API and adding a new one.
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  app/test/test_sched.c  |  4 +--
>  lib/librte_sched/rte_sched.c   | 65 
> +-
>  lib/librte_sched/rte_sched.h   | 37 ++-
>  lib/librte_sched/rte_sched_version.map |  2 ++
>  4 files changed, 74 insertions(+), 34 deletions(-)
> 
> diff --git a/app/test/test_sched.c b/app/test/test_sched.c
> index c7239f8..03f89b4 100644
> --- a/app/test/test_sched.c
> +++ b/app/test/test_sched.c
> @@ -198,13 +198,13 @@ test_sched(void)
> 
>   struct rte_sched_subport_stats subport_stats;
>   uint32_t tc_ov;
> - rte_sched_subport_read_stats(port, SUBPORT, _stats,
> _ov);
> + rte_sched_subport_stats(port, SUBPORT, _stats, _ov,
> 1);
>  #if 0
>   TEST_ASSERT_EQUAL(subport_stats.n_pkts_tc[TC-1], 10, "Wrong
> subport stats\n");
>  #endif
>   struct rte_sched_queue_stats queue_stats;
>   uint16_t qlen;
> - rte_sched_queue_read_stats(port, QUEUE, _stats, );
> + rte_sched_queue_stats(port, QUEUE, _stats, , 1);
>  #if 0
>   TEST_ASSERT_EQUAL(queue_stats.n_pkts, 10, "Wrong queue
> stats\n");
>  #endif
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 9c9419d..b4d7edd 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -965,61 +965,78 @@ rte_sched_port_pkt_read_color(const struct
> rte_mbuf *pkt)
>  }
> 
>  int
> -rte_sched_subport_read_stats(struct rte_sched_port *port,
> - uint32_t subport_id,
> - struct rte_sched_subport_stats *stats,
> - uint32_t *tc_ov)
> +rte_sched_subport_stats(struct rte_sched_port *port, uint32_t
> subport_id,
> + struct rte_sched_subport_stats *stats,
> + uint32_t *tc_ov, int clear)
>  {
>   struct rte_sched_subport *s;
> 
>   /* Check user parameters */
> - if ((port == NULL) ||
> - (subport_id >= port->n_subports_per_port) ||
> - (stats == NULL) ||
> - (tc_ov == NULL)) {
> + if (port == NULL || subport_id >= port->n_subports_per_port)
>   return -1;
> - }
> +
>   s = port->subport + subport_id;
> 
>   /* Copy subport stats and clear */
> - memcpy(stats, >stats, sizeof(struct rte_sched_subport_stats));
> - memset(>stats, 0, sizeof(struct rte_sched_subport_stats));
> + if (stats)
> + *stats = s->stats;
> + if (clear)
> + memset(>stats, 0, sizeof(struct
> rte_sched_subport_stats));
> 
>   /* Subport TC ovesubscription status */
> - *tc_ov = s->tc_ov;
> + if (tc_ov)
> + *tc_ov = s->tc_ov;
> 
>   return 0;
>  }
> 
> +/* Deprecated API, always clears */
>  int
> -rte_sched_queue_read_stats(struct rte_sched_port *port,
> - uint32_t queue_id,
> - struct rte_sched_queue_stats *stats,
> - uint16_t *qlen)
> +rte_sched_subport_read_stats(struct rte_sched_port *port, uint32_t
> subport_id,
> +  struct rte_sched_subport_stats *stats,
> +  uint32_t *tc_ov)
> +{
> + return rte_sched_subport_stats(port, subport_id, stats, tc_ov, 1);
> +}
> +
> +int
> +rte_sched_queue_stats(struct rte_sched_port *port, uint32_t queue_id,
> +   struct rte_sched_queue_stats *stats,
> +   uint16_t *qlen, int clear)
>  {
>   struct rte_sched_queue *q;
>   struct rte_sched_queue_extra *qe;
> 
>   /* Check user parameters */
> - if ((port == NULL) ||
> - (queue_id >= rte_sched_port_queues_per_port(port)) ||
> - (stats == NULL) ||
> - (qlen == NULL)) {
> + if (port == NULL || queue_id >=
> rte_sched_port_queues_per_port(port))
>   return -1;
> - }
> +
>   q = port->queue + queue_id;
>   qe = port->queue_extra + queue_id;
> 
>   /* Copy queue stats and clear */
> - memcpy(stats, >stats, sizeof(struct rte_sched_queue_stats));
> - memset(>stats, 0, sizeof(struct rte_sched_queue_stats));
> + if (stats)
> + *stats = qe->stats;
> + if (clear)
> + memset(>stats, 0, sizeof(struct
> rte_sched_queue_stats));
> 
>   /* Queue length */
> - *qlen = q->qw - q->qr;
> + if (qlen)
> + *qlen = q->qw - q->qr;
> 
>   return 0;
>  }
> 
> +/* Deprecated API, always clears */
> +int
> +rte_sched_queue_read_stats(struct rte_sched_port *port,
> + uint32_t queue_id,
> + struct rte_sched_queue_stats *stats,
> + uint16_t *qlen)
> +{
> + return 

[dpdk-dev] Mac ageing functionality.

2015-06-04 Thread Yeddula, Avinash
Hello All,
Does dpdk provide any kind of ageing functionality ( To be specific, Mac ageing 
functionality is what I'm looking for).

If yes, please provide the pointers.

Thanks
-Avinash


[dpdk-dev] Running testpmd over KNI

2015-06-04 Thread Navneet Rao
Running ---



./testpmd -c7 -n3 --vdev=eth_pcap0,iface=vEth0 --vdev=eth_pcap1,iface=vEth1 -- 
-i --nb-cores=2 --nb-ports=2 --total-num-mbufs=1024



results in a  



EAL: Error - exiting with code: 1

  Cause: Cannot create lock on '/var/run/.rte_config'. Is another primary 
process running?





I don't think I am running another process using testpmd!!!

Any ideas to debug this?



Thanks

-Navneet


[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

2015-06-04 Thread Chilikin, Andrey
> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Neil Horman
> Sent: Thursday, June 4, 2015 2:56 PM
> To: Wiles, Keith
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at
> rte_eal_init time
> 
> On Thu, Jun 04, 2015 at 11:50:33AM +, Wiles, Keith wrote:
> > Hi Stephen
> >
> > On 6/3/15, 7:12 PM, "Stephen Hemminger" 
> wrote:
> >
> > >On Wed,  3 Jun 2015 13:49:53 -0500
> > >Keith Wiles  wrote:
> > >
> > >> +/* Launch threads, called at application init() and parse app
> > >> +args. */ int rte_eal_init_parse(int argc, char **argv,
> > >> +int (*parse)(int, char **))
> > >> +{
> > >> +int ret;
> > >> +
> > >> +ret = rte_eal_init(argc, argv);
> > >> +if ((ret >= 0) && (parse != NULL)) {
> > >> +argc -= ret;
> > >> +argv += ret;
> > >
> > >This won't work C is call by value.
> >
> > I tested this routine with Pktgen (again), which has a number of
> > application options and it appears to work correctly. Can you explain
> > why this will not work?
> >
> > Regards,
> > ++Keith
> > >
> >
> >
> 
> Stephen was thinking that your intent was to have argc, and argv modified at
> the call site of this function (i.e. if you called rte_eal_init_parse from 
> main(),
> then after the call to rte_ela_init_parse, argc would be reduced by ret and 
> argv
> would point forward in memory ret bytes in the main function, but they wont.
> It doesn't matter though, because you return ret, so the caller can do that
> movement themselves.  As you note, it works.
> 
> Note that if it was your intention to have argc and argv modified at the call 
> site,
> then Stephen is right and this is broken, you need to modify the prototype to 
> be:
> int rte_eal_init_parse(int *argc, char ***argv)
> 
> and do a dereference when modifying the parameters so the change is seen at
> the call site.
> 
> That said, I'm not sure theres much value in adding this to the API.  For 
> one, it
> implies that dpdk arguments need to come first on the command line.  While
> all the example applications do that, theres no requirement that they do so,
> and this function silently implies that they have to, so any existing 
> applications
> in the wild that violate that assumption are enjoined from using this
> 
> It also doesn't really save any code.  If we pick an example app (I'll us 
> l2fwd-
> jobstats), We currently have this:
> 
>   /* init EAL */
> ret = rte_eal_init(argc, argv);
> if (ret < 0)
> rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
> argc -= ret;
> argv += ret;
> 
> /* parse application arguments (after the EAL ones) */
> ret = l2fwd_parse_args(argc, argv);
>   if (ret < 0)
> rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");
> 
> With your new API we would get this:
> 
>   ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
> if (ret < 0)
> rte_exit(EXIT_FAILURE, "Invalid arguments - not sure what\n");
> 
> Its definately 5 fewer lines of source, but it doesn't save any execution
> instructions, and for the effort of that, you loose the ability to determine 
> if it
> was a DPDK argument or an application argument that failed.
> 
> Its not a bad addition, I'm just not sure its worth having to take on the
> additional API surface to include.  I'd be more supportive if you could 
> enhance
> the function to allow the previously mentioned before/after flexibiilty.  
> Then we
> could just deprecate rte_eal_init as an API call entirely, and use this 
> instead.

Before/after would be very useful, a lot of applications use only "-c" and "-n" 
EAL command line parameters and "-c" in many cases is redundant as application 
can calculate core mask from its own parameters, and "-n" just a required 
parameter which can be defaulted to a platform specific value. So in addition 
to rte_set_application_usage_hook() it would be nice to have some more general 
way of overwriting eal initialization parameters. 

> 
> Neil



[dpdk-dev] Running testpmd over KNI

2015-06-04 Thread Navneet Rao
Running testpmd as proc-type=auto now results in a SEGMENTATION FAULT!!!
Now trying to debug the source of the memory leak!!!

Actually I want to do this -
Use the testpmd app to setup TX/RX traffic on the 2 NICs that have been now 
configured as Kernel-NICs.
Is there an easier way to accomplish this?

Thanks
-Navneet



-Original Message-
From: Navneet Rao 
Sent: Thursday, June 04, 2015 2:01 PM
To: dev at dpdk.org
Subject: [dpdk-dev] Running testpmd over KNI

Running ---



./testpmd -c7 -n3 --vdev=eth_pcap0,iface=vEth0 --vdev=eth_pcap1,iface=vEth1 -- 
-i --nb-cores=2 --nb-ports=2 --total-num-mbufs=1024



results in a  



EAL: Error - exiting with code: 1

  Cause: Cannot create lock on '/var/run/.rte_config'. Is another primary 
process running?





I don't think I am running another process using testpmd!!!

Any ideas to debug this?



Thanks

-Navneet


[dpdk-dev] [PATCH 2/2] ethtool: add new library to provide ethtool-alike APIs

2015-06-04 Thread Andrew Harvey (agh)
On 6/4/15, 7:58 AM, "Stephen Hemminger"  wrote:

>On Wed, 3 Jun 2015 02:09:39 +
>"Andrew Harvey (agh)"  wrote:
>
>> I believe that their is value in this interface for software stacks not
>> based on Linux being moved toward DPDK that need simple operations like
>> getting the mac address.  Some of these stacks have a dearth of
>>resources
>> available and dedicating a core/thread to KNI to get/set a mac address
>> is considered excessive. There are also issues with 32/64 bit kernel
>> integration
>> using KNI.  If the ethtool interface is not the correct interface then
>> please help me
>> understand what should/could have been used. If ethtool is considered
>>'old
>> and clunky?
>> Stephen's and your input would be valuable in designing another
>>interface
>> with
>> similar properties.  The use-case is pretty simple and there is no plans
>> for moving
>> anything back into the kernel on the contrary its the complete opposite.
>> 
>> ? Andy
>
>We have DPDK API's to do this, and any added wrappers make it bigger.
>I don't see why calling your ethtool API is better than calling
>rte_eth* API.
>
>If there is a missing functionality in the rte_ethXXX api's for an
>application then add that. For example: rte_eth_mac_addr_get()

I am getting somewhat confused by your latest comments.  Your first email
(referenced below) looked really positive and I found your suggestions
useful. Your latest post appears to contradict this and now the interface
was there all the time.  The wrapper fa?ade provided by the ethtool
library provide a clean separation of concerns and will allow people to
migrate from not only KNI but in our case from a legacy system.  If a
software stack has requirements to work with multiple IO abstractions
then the ethtool approach is attractive. I would speculate that many
other stacks moving towards dpdk will have similar issues.

Summarizing, for our use-cases the ethtool interface facilitated our
adoption to dpdk while allowing us to support our legacy IO abstractions.

? Andy

http://dpdk.org/ml/archives/dev/2015-May/018408.html (original email)

http://dpdk.org/ml/archives/dev/2014-June/003005.html (ethtool request)