[dpdk-dev] [PATCH 00/11] add rte_kvargs library: a key/value args parser

[dpdk-dev] [PATCH 00/11] add rte_kvargs library: a key/value args parser

[Olivier Matz]

The topic of this patchset is to add a new rte_kvargs library that
can be used as a helper to parse key/value arguments. The code will
be based on rte_eth_pcap_arg_parser and reworked with documentation
and tests. It also fix some minor issues of the original code.

These commits will allow another library (like pmd_ring) to parse
arguments in an easier way without duplicating the code.

Olivier Matz (11):
  kvargs: add a new library to parse key/value arguments
  kvargs: use the new library in pmd_pcap
  kvargs: remove driver name in arguments
  kvargs: remove useless size field
  kvargs: rework API to fix memory leak
  kvargs: simpler parsing and allow duplicated keys
  kvargs: be strict when matching a key
  kvargs: add const attribute in handler parameters
  kvargs: add the key in handler pameters
  kvargs: make the NULL key to match all entries
  kvargs: add test case in app/test

 app/test/Makefile                             |   1 +
 app/test/commands.c                           |   8 +
 app/test/test.h                               |   1 +
 app/test/test_kvargs.c                        | 235 ++++++++++++++++++++++++
 config/defconfig_i686-default-linuxapp-gcc    |   5 +
 config/defconfig_i686-default-linuxapp-icc    |   5 +
 config/defconfig_x86_64-default-linuxapp-gcc  |   5 +
 config/defconfig_x86_64-default-linuxapp-icc  |   5 +
 lib/Makefile                                  |   1 +
 lib/librte_kvargs/Makefile                    |  49 +++++
 lib/librte_kvargs/rte_kvargs.c                | 206 +++++++++++++++++++++
 lib/librte_kvargs/rte_kvargs.h                | 155 ++++++++++++++++
 lib/librte_pmd_pcap/Makefile                  |   8 +-
 lib/librte_pmd_pcap/rte_eth_pcap.c            |  44 ++---
 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c | 255 --------------------------
 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h |  71 -------
 lib/librte_pmd_ring/rte_eth_ring.c            |   2 +
 mk/rte.app.mk                                 |   4 +
 18 files changed, 709 insertions(+), 351 deletions(-)
 create mode 100644 app/test/test_kvargs.c
 create mode 100644 lib/librte_kvargs/Makefile
 create mode 100644 lib/librte_kvargs/rte_kvargs.c
 create mode 100644 lib/librte_kvargs/rte_kvargs.h
 delete mode 100644 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
 delete mode 100644 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h

-- 
1.8.4.rc3

[dpdk-dev] [PATCH 01/11] kvargs: add a new library to parse key/value arguments

[Olivier Matz]

Copy the code from rte_eth_pcap_arg_parser.[ch], without any functional
modifications, only:
- rename functions and structure
- restyle (indentation)
- add comments (doxygen style)
- add "const" or "static" attributes, remove unneeded "inline"

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 config/defconfig_i686-default-linuxapp-gcc   |   5 +
 config/defconfig_i686-default-linuxapp-icc   |   5 +
 config/defconfig_x86_64-default-linuxapp-gcc |   5 +
 config/defconfig_x86_64-default-linuxapp-icc |   5 +
 lib/Makefile                                 |   1 +
 lib/librte_kvargs/Makefile                   |  50 ++++++
 lib/librte_kvargs/rte_kvargs.c               | 248 +++++++++++++++++++++++++++
 lib/librte_kvargs/rte_kvargs.h               | 159 +++++++++++++++++
 mk/rte.app.mk                                |   4 +
 9 files changed, 482 insertions(+)
 create mode 100644 lib/librte_kvargs/Makefile
 create mode 100644 lib/librte_kvargs/rte_kvargs.c
 create mode 100644 lib/librte_kvargs/rte_kvargs.h

diff --git a/config/defconfig_i686-default-linuxapp-gcc b/config/defconfig_i686-default-linuxapp-gcc
index 4f57242..349ca24 100644
--- a/config/defconfig_i686-default-linuxapp-gcc
+++ b/config/defconfig_i686-default-linuxapp-gcc
@@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
 CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
 
 #
+# Compile the argument parser library
+#
+CONFIG_RTE_LIBRTE_KVARGS=y
+
+#
 # Compile generic ethernet library
 #
 CONFIG_RTE_LIBRTE_ETHER=y
diff --git a/config/defconfig_i686-default-linuxapp-icc b/config/defconfig_i686-default-linuxapp-icc
index 90521e1..4e7338f 100644
--- a/config/defconfig_i686-default-linuxapp-icc
+++ b/config/defconfig_i686-default-linuxapp-icc
@@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
 CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
 
 #
+# Compile the argument parser library
+#
+CONFIG_RTE_LIBRTE_KVARGS=y
+
+#
 # Compile generic ethernet library
 #
 CONFIG_RTE_LIBRTE_ETHER=y
diff --git a/config/defconfig_x86_64-default-linuxapp-gcc b/config/defconfig_x86_64-default-linuxapp-gcc
index 4c05ffc..ca85b1a 100644
--- a/config/defconfig_x86_64-default-linuxapp-gcc
+++ b/config/defconfig_x86_64-default-linuxapp-gcc
@@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
 CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
 
 #
+# Compile the argument parser library
+#
+CONFIG_RTE_LIBRTE_KVARGS=y
+
+#
 # Compile generic ethernet library
 #
 CONFIG_RTE_LIBRTE_ETHER=y
diff --git a/config/defconfig_x86_64-default-linuxapp-icc b/config/defconfig_x86_64-default-linuxapp-icc
index c4d5c73..37dbdd0 100644
--- a/config/defconfig_x86_64-default-linuxapp-icc
+++ b/config/defconfig_x86_64-default-linuxapp-icc
@@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
 CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
 
 #
+# Compile the argument parser library
+#
+CONFIG_RTE_LIBRTE_KVARGS=y
+
+#
 # Compile generic ethernet library
 #
 CONFIG_RTE_LIBRTE_ETHER=y
diff --git a/lib/Makefile b/lib/Makefile
index b655d4f..31644f2 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -52,6 +52,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_POWER) += librte_power
 DIRS-$(CONFIG_RTE_LIBRTE_METER) += librte_meter
 DIRS-$(CONFIG_RTE_LIBRTE_SCHED) += librte_sched
 DIRS-$(CONFIG_RTE_LIBRTE_ACL) += librte_acl
+DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs
 
 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_kvargs/Makefile b/lib/librte_kvargs/Makefile
new file mode 100644
index 0000000..c4c2855
--- /dev/null
+++ b/lib/librte_kvargs/Makefile
@@ -0,0 +1,50 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2014 6WIND S.A.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_kvargs.a
+
+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
+
+# all source are stored in SRCS-y
+SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) := rte_kvargs.c
+
+# install includes
+INCS := rte_kvargs.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_KVARGS)-include := $(INCS)
+
+# this lib needs eal and malloc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_malloc
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
new file mode 100644
index 0000000..7a950d6
--- /dev/null
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -0,0 +1,248 @@
+/*-
+ *   BSD LICENSE
+ * 
+ *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ * 
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ * 
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ * 
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include <string.h>
+#include <sys/user.h>
+#include <linux/binfmts.h>
+
+#include <rte_malloc.h>
+#include <rte_log.h>
+#include <rte_string_fns.h>
+
+#include "rte_kvargs.h"
+
+/*
+ * Initialize a rte_kvargs structure to an empty key/value list.
+ */
+int
+rte_kvargs_init(struct rte_kvargs *kvlist)
+{
+	kvlist->count = 0;
+	kvlist->size = RTE_KVARGS_MAX;
+	memset(kvlist->pairs, 0, kvlist->size);
+	return 0;
+}
+
+/*
+ * Add a key-value pair at the end of a given key/value list.
+ * Return an error if the list is full or if the key is duplicated.
+ */
+static int
+rte_kvargs_add_pair(struct rte_kvargs *kvlist, char *key, char *val)
+{
+	unsigned i;
+	struct rte_kvargs_pair* entry;
+
+	/* is the list full? */
+	if (kvlist->count >= kvlist->size) {
+		RTE_LOG(ERR, PMD, "Couldn't add %s, key/value list is full\n", key);
+		return -1;
+	}
+
+	/* Check if the key is duplicated */
+	for (i = 0; i < kvlist->count; i++) {
+		entry = &kvlist->pairs[i];
+		if (strcmp(entry->key, key) == 0) {
+			RTE_LOG(ERR, PMD, "Couldn't add %s, duplicated key\n", key);
+			return -1;
+		}
+	}
+
+	entry = &kvlist->pairs[kvlist->count];
+	entry->key = key;
+	entry->value = val;
+	kvlist->count++;
+	return 0;
+}
+
+/*
+ * Receive a string with a list of arguments following the pattern
+ * key=value;key=value;... and insert them into the list.
+ * strtok() is used so the params string will be copied to be modified.
+ */
+static int
+rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
+	const char *params)
+{
+	unsigned i, count;
+	char *args;
+	char *pairs[RTE_KVARGS_MAX];
+	char *pair[2];
+
+	/* If params are empty, nothing to do */
+	if (params == NULL || params[0] == 0) {
+		RTE_LOG(ERR, PMD, "Couldn't parse %s device, empty arguments\n", name);
+		return -1;
+	}
+
+	/* Copy the const char *params to a modifiable string
+	 * to pass to rte_strsplit
+	 */
+	args = strdup(params);
+	if(args == NULL){
+		RTE_LOG(ERR, PMD, "Couldn't parse %s device \n", name);
+		return -1;
+	}
+
+	count = rte_strsplit(args, strnlen(args, MAX_ARG_STRLEN), pairs,
+			RTE_KVARGS_MAX, RTE_KVARGS_PAIRS_DELIM);
+
+	for (i = 0; i < count; i++) {
+		pair[0] = NULL;
+		pair[1] = NULL;
+
+		rte_strsplit(pairs[i], strnlen(pairs[i], MAX_ARG_STRLEN), pair, 2,
+				RTE_KVARGS_KV_DELIM);
+
+		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
+				|| pair[1][0] == 0) {
+			RTE_LOG(ERR, PMD,
+					"Couldn't parse %s device, wrong key or value \n", name);
+			goto error;
+		}
+
+		if (rte_kvargs_add_pair(kvlist, pair[0], pair[1]) < 0)
+			goto error;
+	}
+	return 0;
+
+error:
+	rte_free(args);
+	return -1;
+}
+
+/*
+ * Determine whether a key is valid or not by looking
+ * into a list of valid keys.
+ */
+static int
+is_valid_key(const char *valid[], const char *key_match)
+{
+	const char **valid_ptr;
+
+	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++)
+		if (strstr(key_match, *valid_ptr) != NULL)
+			return 1;
+	return 0;
+}
+
+/*
+ * Determine whether all keys are valid or not by looking
+ * into a list of valid keys.
+ */
+static int
+check_for_valid_keys(struct rte_kvargs *kvlist,
+		const char *valid[])
+{
+	unsigned i, ret;
+	struct rte_kvargs_pair *pair;
+
+	for (i = 0; i < kvlist->count; i++) {
+		pair = &kvlist->pairs[i];
+		ret = is_valid_key(valid, pair->key);
+		if (!ret) {
+			RTE_LOG(ERR, PMD,
+				"Error parsing device, invalid key <%s>\n",
+				pair->key);
+			return -1;
+		}
+	}
+	return 0;
+}
+
+/*
+ * Return the number of times a given arg_name exists in the key/value list.
+ * E.g. given a list = { rx = 0, rx = 1, tx = 2 } the number of args for
+ * arg "rx" will be 2.
+ */
+unsigned
+rte_kvargs_count(const struct rte_kvargs *kvlist, const char *key_match)
+{
+	const struct rte_kvargs_pair *pair;
+	unsigned i, ret;
+
+	ret = 0;
+	for (i = 0; i < kvlist->count; i++) {
+		pair = &kvlist->pairs[i];
+		if (strcmp(pair->key, key_match) == 0)
+			ret++;
+	}
+
+	return ret;
+}
+
+/*
+ * For each matching key, call the given handler function.
+ */
+int
+rte_kvargs_process(const struct rte_kvargs *kvlist,
+		const char *key_match,
+		arg_handler_t handler,
+		void *opaque_arg)
+{
+	const struct rte_kvargs_pair *pair;
+	unsigned i;
+
+	for (i = 0; i < kvlist->count; i++) {
+		pair = &kvlist->pairs[i];
+		if (strstr(pair->key, key_match) != NULL) {
+			if ((*handler)(pair->value, opaque_arg) < 0)
+				return -1;
+		}
+	}
+	return 0;
+}
+
+/*
+ * Parse the arguments "key=value;key=value;..." string and return
+ * an allocated structure that contains a key/value list. Also
+ * check if only valid keys were used.
+ */
+int
+rte_kvargs_parse(struct rte_kvargs *kvlist,
+		const char *name,
+		const char *args,
+		const char *valid_keys[])
+{
+
+	int ret;
+
+	ret = rte_kvargs_tokenize(kvlist, name, args);
+	if (ret < 0)
+		return ret;
+
+	if (valid_keys == NULL)
+		return 0;
+
+	return check_for_valid_keys(kvlist, valid_keys);
+}
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
new file mode 100644
index 0000000..19485b1
--- /dev/null
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -0,0 +1,159 @@
+/*-
+ *   BSD LICENSE
+ * 
+ *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ * 
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ * 
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ * 
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_KVARGS_H_
+#define _RTE_KVARGS_H_
+
+/**
+ * @file
+ * RTE Argument parsing
+ *
+ * This module can be used to parse arguments whose format is
+ * key1=value1;key2=value2;key3=value3;...
+ *
+ * This file provides some helpers that are especially used by virtual
+ * ethernet devices at initialization for arguments parsing.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** Maximum number of key/value associations */
+#define RTE_KVARGS_MAX 32
+
+/** separator character used between each pair */
+#define RTE_KVARGS_PAIRS_DELIM	';'
+
+/** separator character used between key and value */
+#define RTE_KVARGS_KV_DELIM	'='
+
+/** Type of callback function used by rte_kvargs_process() */
+typedef int (*arg_handler_t)(char *value, void *opaque);
+
+/** A key/value association */
+struct rte_kvargs_pair {
+	char *key;      /**< the name (key) of the association  */
+	char *value;    /**< the value associated to that key */
+};
+
+/** Store a list of key/value associations */
+struct rte_kvargs {
+	unsigned count; /**< number of entries in the list */
+	size_t size;    /**< maximum number of entries */
+	struct rte_kvargs_pair pairs[RTE_KVARGS_MAX]; /**< list of key/values */
+};
+
+/**
+ * Initialize an empty rte_kvargs structure
+ *
+ * Set the content of the rte_kvargs structure given as a parameter
+ * to an empty list of key/value.
+ *
+ * @param kvlist
+ *   The rte_kvargs structure
+ *
+ * @return
+ *   - 0 on success
+ *   - Negative on error
+ */
+int rte_kvargs_init(struct rte_kvargs *kvlist);
+
+/**
+ * Append key/value associations in a rte_kvargs structure from a string
+ *
+ * The function fills a rte_kvargs structure from a string whose format
+ * is key1=value1;key2=value2;...
+ * The key/value associations are appended to those which are already
+ * present in the rte_kvargs structure.
+ *
+ * @param kvlist
+ *   The rte_kvargs structure
+ * @param name
+ *   The name of the driver
+ * @param args
+ *   The input string containing the key/value associations
+ * @param valid_keys
+ *   A list of valid keys (table of const char *, the last must be NULL).
+ *   This argument is ignored if NULL
+ *
+ * @return
+ *   - 0 on success
+ *   - Negative on error
+ */
+int rte_kvargs_parse(struct rte_kvargs *kvlist, const char *name,
+	const char *args, const char *valid_keys[]);
+
+/**
+ * Call a handler function for each key/value matching the key
+ *
+ * For each key/value association that matches the given key, calls the
+ * handler function with the for a given arg_name passing the value on the
+ * dictionary for that key and a given extra argument.
+ *
+ * @param kvlist
+ *   The rte_kvargs structure
+ * @param key_match
+ *   The key on which the handler should be called
+ * @param handler
+ *   The function to call for each matching key
+ * @param opaque_arg
+ *   A pointer passed unchanged to the handler
+ *
+ * @return
+ *   - 0 on success
+ *   - Negative on error
+ */
+int rte_kvargs_process(const struct rte_kvargs *kvlist,
+	const char *key_match, arg_handler_t handler, void *opaque_arg);
+
+/**
+ * Count the number of associations matching the given key
+ *
+ * @param kvlist
+ *   The rte_kvargs structure
+ * @param key_match
+ *   The key that should match
+
+ * @return
+ *   The number of entries
+ */
+unsigned rte_kvargs_count(const struct rte_kvargs *kvlist,
+	const char *key_match);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 786100c..1652029 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -112,6 +112,10 @@ endif
 
 LDLIBS += --start-group
 
+ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
+LDLIBS += -lrte_kvargs
+endif
+
 ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y)
 LDLIBS += -lrte_mbuf
 endif
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 02/11] kvargs: use the new library in pmd_pcap

[Olivier Matz]

The rte_kvargs library is a reworked copy of rte_eth_pcap_arg_parser,
so it provides the same service. Therefore we can use it and remove the
code of rte_eth_pcap_arg_parser.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_pmd_pcap/Makefile                  |   8 +-
 lib/librte_pmd_pcap/rte_eth_pcap.c            |  29 +--
 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c | 255 --------------------------
 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h |  71 -------
 4 files changed, 19 insertions(+), 344 deletions(-)
 delete mode 100644 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
 delete mode 100644 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h

diff --git a/lib/librte_pmd_pcap/Makefile b/lib/librte_pmd_pcap/Makefile
index 33174c0..741dafc 100644
--- a/lib/librte_pmd_pcap/Makefile
+++ b/lib/librte_pmd_pcap/Makefile
@@ -1,6 +1,7 @@
 #   BSD LICENSE
 # 
 #   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
+#   Copyright(c) 2014 6WIND S.A.
 #   All rights reserved.
 # 
 #   Redistribution and use in source and binary forms, with or without
@@ -43,16 +44,15 @@ CFLAGS += $(WERROR_FLAGS)
 # all source are stored in SRCS-y
 #
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += rte_eth_pcap.c
-SRCS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += rte_eth_pcap_arg_parser.c
-
 
 #
 # Export include files
 #
 SYMLINK-y-include += rte_eth_pcap.h
-SYMLINK-y-include += rte_eth_pcap_arg_parser.h
 
 # this lib depends upon:
-DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_mbuf lib/librte_ether
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_mbuf
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_ether
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_kvargs
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 208e316..e47afcb 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -2,6 +2,7 @@
  *   BSD LICENSE
  * 
  *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
  *   All rights reserved.
  * 
  *   Redistribution and use in source and binary forms, with or without
@@ -39,9 +40,9 @@
 #include <rte_string_fns.h>
 #include <rte_cycles.h>
 #include <net/if.h>
+#include <rte_kvargs.h>
 
 #include "rte_eth_pcap.h"
-#include "rte_eth_pcap_arg_parser.h"
 
 #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535
 #define RTE_ETH_PCAP_SNAPLEN 4096
@@ -701,11 +702,11 @@ rte_pmd_pcap_init(const char *name, const char *params)
 {
 	unsigned numa_node, using_dumpers = 0;
 	int ret;
-	struct args_dict dict;
+	struct rte_kvargs kvlist;
 	struct rx_pcaps pcaps;
 	struct tx_pcaps dumpers;
 
-	rte_eth_pcap_init_args_dict(&dict);
+	rte_kvargs_init(&kvlist);
 
 	numa_node = rte_socket_id();
 
@@ -713,16 +714,16 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	start_cycles = rte_get_timer_cycles();
 	hz = rte_get_timer_hz();
 
-	if (rte_eth_pcap_parse_args(&dict, name, params, valid_arguments) < 0)
+	if (rte_kvargs_parse(&kvlist, name, params, valid_arguments) < 0)
 		return -1;
 
 	/*
 	 * If iface argument is passed we open the NICs and use them for
 	 * reading / writing
 	 */
-	if (rte_eth_pcap_num_of_args(&dict, ETH_PCAP_IFACE_ARG) == 1) {
+	if (rte_kvargs_count(&kvlist, ETH_PCAP_IFACE_ARG) == 1) {
 
-		ret = rte_eth_pcap_post_process_arguments(&dict, ETH_PCAP_IFACE_ARG,
+		ret = rte_kvargs_process(&kvlist, ETH_PCAP_IFACE_ARG,
 				&open_rx_tx_iface, &pcaps.pcaps[0]);
 		if (ret < 0)
 			return -1;
@@ -734,13 +735,13 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	 * We check whether we want to open a RX stream from a real NIC or a
 	 * pcap file
 	 */
-	if ((pcaps.num_of_rx = rte_eth_pcap_num_of_args(&dict, ETH_PCAP_RX_PCAP_ARG))) {
-		ret = rte_eth_pcap_post_process_arguments(&dict, ETH_PCAP_RX_PCAP_ARG,
+	if ((pcaps.num_of_rx = rte_kvargs_count(&kvlist, ETH_PCAP_RX_PCAP_ARG))) {
+		ret = rte_kvargs_process(&kvlist, ETH_PCAP_RX_PCAP_ARG,
 				&open_rx_pcap, &pcaps);
 	} else {
-		pcaps.num_of_rx = rte_eth_pcap_num_of_args(&dict,
+		pcaps.num_of_rx = rte_kvargs_count(&kvlist,
 				ETH_PCAP_RX_IFACE_ARG);
-		ret = rte_eth_pcap_post_process_arguments(&dict, ETH_PCAP_RX_IFACE_ARG,
+		ret = rte_kvargs_process(&kvlist, ETH_PCAP_RX_IFACE_ARG,
 				&open_rx_iface, &pcaps);
 	}
 
@@ -751,15 +752,15 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	 * We check whether we want to open a TX stream to a real NIC or a
 	 * pcap file
 	 */
-	if ((dumpers.num_of_tx = rte_eth_pcap_num_of_args(&dict,
+	if ((dumpers.num_of_tx = rte_kvargs_count(&kvlist,
 			ETH_PCAP_TX_PCAP_ARG))) {
-		ret = rte_eth_pcap_post_process_arguments(&dict, ETH_PCAP_TX_PCAP_ARG,
+		ret = rte_kvargs_process(&kvlist, ETH_PCAP_TX_PCAP_ARG,
 				&open_tx_pcap, &dumpers);
 		using_dumpers = 1;
 	} else {
-		dumpers.num_of_tx = rte_eth_pcap_num_of_args(&dict,
+		dumpers.num_of_tx = rte_kvargs_count(&kvlist,
 				ETH_PCAP_TX_IFACE_ARG);
-		ret = rte_eth_pcap_post_process_arguments(&dict, ETH_PCAP_TX_IFACE_ARG,
+		ret = rte_kvargs_process(&kvlist, ETH_PCAP_TX_IFACE_ARG,
 				&open_tx_iface, &dumpers);
 	}
 
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c b/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
deleted file mode 100644
index c881f3b..0000000
--- a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
+++ /dev/null
@@ -1,255 +0,0 @@
-/*-
- *   BSD LICENSE
- * 
- *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
- *   All rights reserved.
- * 
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- * 
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- * 
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-#include <string.h>
-#include <sys/user.h>
-#include <linux/binfmts.h>
-
-#include <rte_malloc.h>
-#include <rte_log.h>
-#include <rte_string_fns.h>
-
-#include "rte_eth_pcap_arg_parser.h"
-
-/*
- * Initializes a non NULL dictionary reference to be used later on.
- */
-inline int
-rte_eth_pcap_init_args_dict(struct args_dict *dict)
-{
-	dict->index = 0;
-	dict->size = RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS;
-	memset(dict->pairs, 0, dict->size);
-	return 0;
-}
-
-/*
- * Adds a key-value pair to a given non-NULL dictionary reference.
- * The final key will be the name+key.
- * Returns error in case the dictionary is full or if the key is duplicated.
- */
-inline int
-rte_eth_pcap_add_pair_to_dict(struct args_dict *dict,
-		char *key,
-		char *val)
-{
-	unsigned i;
-	struct key_value* entry;
-
-	/* is the dictionary full? */
-	if (dict->index >= dict->size) {
-		RTE_LOG(ERR, PMD, "Couldn't add %s, dictionary is full\n", key);
-		return -1;
-	}
-
-	/* Check if the key is duplicated */
-	for (i = 0; i < dict->index; i++) {
-		entry = &dict->pairs[i];
-		if (strcmp(entry->key, key) == 0) {
-			RTE_LOG(ERR, PMD, "Couldn't add %s, duplicated key\n", key);
-			return -1;
-		}
-	}
-
-	entry = &dict->pairs[dict->index];
-	entry->key = key;
-	entry->value = val;
-	dict->index++;
-	return 0;
-
-}
-
-#define RTE_ETH_PCAP_PAIRS_DELIM		';'
-#define RTE_ETH_PCAP_KEY_VALUE_DELIM	'='
-/*
- * Receives a string with a list of arguments following the pattern
- * key=value;key=value;... and inserts them into the non NULL dictionary.
- * strtok is used so the params string will be copied to be modified.
- */
-inline int
-rte_eth_pcap_tokenize_args(struct args_dict *dict,
-		const char *name,
-		const char *params)
-{
-	int i;
-	char *args;
-	char *pairs[RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS];
-	char *pair[2];
-	int num_of_pairs;
-
-	/* If params are empty, nothing to do */
-	if (params == NULL || params[0] == 0) {
-		RTE_LOG(ERR, PMD, "Couldn't parse %s device, empty arguments\n", name);
-		return -1;
-	}
-
-	/* Copy the const char *params to a modifiable string
-	 * to pass to rte_strsplit
-	 */
-	args = strdup(params);
-	if(args == NULL){
-		RTE_LOG(ERR, PMD, "Couldn't parse %s device \n", name);
-		return -1;
-	}
-
-	num_of_pairs = rte_strsplit(args, strnlen(args, MAX_ARG_STRLEN), pairs,
-			RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS, RTE_ETH_PCAP_PAIRS_DELIM);
-
-	for (i = 0; i < num_of_pairs; i++) {
-		pair[0] = NULL;
-		pair[1] = NULL;
-
-		rte_strsplit(pairs[i], strnlen(pairs[i], MAX_ARG_STRLEN), pair, 2,
-				RTE_ETH_PCAP_KEY_VALUE_DELIM);
-
-		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
-				|| pair[1][0] == 0) {
-			RTE_LOG(ERR, PMD,
-					"Couldn't parse %s device, wrong key or value \n", name);
-			goto error;
-		}
-
-		if (rte_eth_pcap_add_pair_to_dict(dict, pair[0], pair[1]) < 0)
-			goto error;
-	}
-	return 0;
-
-error:
-	rte_free(args);
-	return -1;
-}
-
-/*
- * Determines whether a key is valid or not by looking
- * into a list of valid keys.
- */
-static inline int
-is_valid_key(const char *valid[],
-		struct key_value *pair)
-{
-	const char **valid_ptr;
-
-	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++)
-		if (strstr(pair->key, *valid_ptr) != NULL)
-			return 1;
-	return 0;
-}
-
-/*
- * Determines whether all keys are valid or not by looking
- * into a list of valid keys.
- */
-static inline int
-check_for_valid_keys(struct args_dict *dict,
-		const char *valid[])
-{
-	unsigned k_index, ret;
-	struct key_value *pair;
-
-	for (k_index = 0; k_index < dict->index; k_index++) {
-		pair = &dict->pairs[k_index];
-		ret = is_valid_key(valid, pair);
-		if (!ret) {
-			RTE_LOG(ERR, PMD,
-					"Error parsing device, invalid key %s\n", pair->key);
-			return -1;
-		}
-	}
-	return 0;
-}
-
-/*
- * Returns the number of times a given arg_name exists on a dictionary.
- * E.g. given a dict = { rx0 = 0, rx1 = 1, tx0 = 2 } the number of args for
- * arg "rx" will be 2.
- */
-inline unsigned
-rte_eth_pcap_num_of_args(struct args_dict *dict, const char *arg_name)
-{
-	unsigned k_index;
-	struct key_value *pair;
-	unsigned num_of_keys;
-
-	num_of_keys = 0;
-	for (k_index = 0; k_index < dict->index; k_index++) {
-		pair = &dict->pairs[k_index];
-		if (strcmp(pair->key, arg_name) == 0)
-			num_of_keys++;
-	}
-
-	return num_of_keys;
-}
-
-/*
- * Calls the handler function for a given arg_name passing the
- * value on the dictionary for that key and a given extra argument.
- */
-inline int
-rte_eth_pcap_post_process_arguments(struct args_dict *dict,
-		const char *arg_name,
-		arg_handler_t handler,
-		void *extra_args)
-{
-	unsigned k_index;
-	struct key_value *pair;
-
-	for (k_index = 0; k_index < dict->index; k_index++) {
-		pair = &dict->pairs[k_index];
-		if (strstr(pair->key, arg_name) != NULL) {
-			if ((*handler)(pair->value, extra_args) < 0)
-				return -1;
-		}
-	}
-	return 0;
-}
-
-/*
- * Parses the arguments "key=value;key=value;..." string and returns
- * a simple dictionary implementation containing these pairs. It also
- * checks if only valid keys were used.
- */
-inline int
-rte_eth_pcap_parse_args(struct args_dict *dict,
-		const char *name,
-		const char *args,
-		const char *valids[])
-{
-
-	int ret;
-
-	ret = rte_eth_pcap_tokenize_args(dict, name, args);
-	if (ret < 0)
-		return ret;
-
-	return check_for_valid_keys(dict, valids);
-}
-
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h b/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h
deleted file mode 100644
index 47eb3a0..0000000
--- a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h
+++ /dev/null
@@ -1,71 +0,0 @@
-/*-
- *   BSD LICENSE
- * 
- *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
- *   All rights reserved.
- * 
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- * 
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- * 
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef _RTE_ETH_ARG_PARSER_H_
-#define _RTE_ETH_ARG_PARSER_H_
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-#define RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS 32
-
-typedef int (*arg_handler_t)(char*, void*);
-
-struct key_value {
-	char *key;
-	char *value;
-};
-
-struct args_dict {
-	unsigned index;
-	size_t size;
-	struct key_value pairs[RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS];
-};
-
-int rte_eth_pcap_tokenize_args(struct args_dict *dict, const char *name,
-		const char *args);
-int rte_eth_pcap_init_args_dict(struct args_dict *dict);
-int rte_eth_pcap_add_pair_to_dict(struct args_dict *dict, char *key, char *val);
-int rte_eth_pcap_parse_args(struct args_dict *dict, const char* name,
-		const char *args, const char *valids[]);
-int rte_eth_pcap_post_process_arguments(struct args_dict *dict,
-		const char *arg_name, arg_handler_t handler, void *extra_args);
-unsigned rte_eth_pcap_num_of_args(struct args_dict *dict, const char *key);
-void rte_eth_pcap_free_dict(struct args_dict *dict);
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 03/11] kvargs: remove driver name in arguments

[Olivier Matz]

Now that rte_kvargs is a generic library, there is no need to have an argument
for the driver name in rte_kvargs_tokenize() and rte_kvargs_parse()
prototypes. This argument was only used to log the driver name in case of
error. Instead, we can add a log in init function of pmd_pcap and pmd_ring.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.c     | 13 ++++++-------
 lib/librte_kvargs/rte_kvargs.h     |  4 +---
 lib/librte_pmd_pcap/rte_eth_pcap.c |  4 +++-
 lib/librte_pmd_ring/rte_eth_ring.c |  2 ++
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 7a950d6..935698c 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -91,8 +91,7 @@ rte_kvargs_add_pair(struct rte_kvargs *kvlist, char *key, char *val)
  * strtok() is used so the params string will be copied to be modified.
  */
 static int
-rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
-	const char *params)
+rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
 {
 	unsigned i, count;
 	char *args;
@@ -101,7 +100,7 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
 
 	/* If params are empty, nothing to do */
 	if (params == NULL || params[0] == 0) {
-		RTE_LOG(ERR, PMD, "Couldn't parse %s device, empty arguments\n", name);
+		RTE_LOG(ERR, PMD, "Cannot parse empty arguments\n");
 		return -1;
 	}
 
@@ -110,7 +109,7 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
 	 */
 	args = strdup(params);
 	if(args == NULL){
-		RTE_LOG(ERR, PMD, "Couldn't parse %s device \n", name);
+		RTE_LOG(ERR, PMD, "Cannot parse arguments: not enough memory\n");
 		return -1;
 	}
 
@@ -127,7 +126,8 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
 		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
 				|| pair[1][0] == 0) {
 			RTE_LOG(ERR, PMD,
-					"Couldn't parse %s device, wrong key or value \n", name);
+				"Cannot parse arguments: wrong key or value\n"
+				"params=<%s>\n", params);
 			goto error;
 		}
 
@@ -230,14 +230,13 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
  */
 int
 rte_kvargs_parse(struct rte_kvargs *kvlist,
-		const char *name,
 		const char *args,
 		const char *valid_keys[])
 {
 
 	int ret;
 
-	ret = rte_kvargs_tokenize(kvlist, name, args);
+	ret = rte_kvargs_tokenize(kvlist, args);
 	if (ret < 0)
 		return ret;
 
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 19485b1..804ea1d 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -100,8 +100,6 @@ int rte_kvargs_init(struct rte_kvargs *kvlist);
  *
  * @param kvlist
  *   The rte_kvargs structure
- * @param name
- *   The name of the driver
  * @param args
  *   The input string containing the key/value associations
  * @param valid_keys
@@ -112,7 +110,7 @@ int rte_kvargs_init(struct rte_kvargs *kvlist);
  *   - 0 on success
  *   - Negative on error
  */
-int rte_kvargs_parse(struct rte_kvargs *kvlist, const char *name,
+int rte_kvargs_parse(struct rte_kvargs *kvlist,
 	const char *args, const char *valid_keys[]);
 
 /**
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index e47afcb..2006b35 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -706,6 +706,8 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	struct rx_pcaps pcaps;
 	struct tx_pcaps dumpers;
 
+	RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
+
 	rte_kvargs_init(&kvlist);
 
 	numa_node = rte_socket_id();
@@ -714,7 +716,7 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	start_cycles = rte_get_timer_cycles();
 	hz = rte_get_timer_hz();
 
-	if (rte_kvargs_parse(&kvlist, name, params, valid_arguments) < 0)
+	if (rte_kvargs_parse(&kvlist, params, valid_arguments) < 0)
 		return -1;
 
 	/*
diff --git a/lib/librte_pmd_ring/rte_eth_ring.c b/lib/librte_pmd_ring/rte_eth_ring.c
index fa3ff72..abef2e8 100644
--- a/lib/librte_pmd_ring/rte_eth_ring.c
+++ b/lib/librte_pmd_ring/rte_eth_ring.c
@@ -384,6 +384,8 @@ rte_eth_ring_pair_attach(const char *name, const unsigned numa_node)
 int
 rte_pmd_ring_init(const char *name, const char *params)
 {
+	RTE_LOG(INFO, PMD, "Initializing pmd_ring for %s\n", name);
+
 	if (params == NULL)
 		eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE);
 	else {
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 04/11] kvargs: remove useless size field

[Olivier Matz]

This value was not very useful as the size of the table is fixed (equals
RTE_KVARGS_MAX).

By the way, the memset in the initialization function was wrong (size
too short). Even if it was not really an issue since we rely on the
"count" field, it is now fixed by this patch.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.c | 5 ++---
 lib/librte_kvargs/rte_kvargs.h | 1 -
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 935698c..0bf46fe 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -48,8 +48,7 @@ int
 rte_kvargs_init(struct rte_kvargs *kvlist)
 {
 	kvlist->count = 0;
-	kvlist->size = RTE_KVARGS_MAX;
-	memset(kvlist->pairs, 0, kvlist->size);
+	memset(kvlist->pairs, 0, sizeof(kvlist->pairs));
 	return 0;
 }
 
@@ -64,7 +63,7 @@ rte_kvargs_add_pair(struct rte_kvargs *kvlist, char *key, char *val)
 	struct rte_kvargs_pair* entry;
 
 	/* is the list full? */
-	if (kvlist->count >= kvlist->size) {
+	if (kvlist->count >= RTE_KVARGS_MAX) {
 		RTE_LOG(ERR, PMD, "Couldn't add %s, key/value list is full\n", key);
 		return -1;
 	}
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 804ea1d..577f00b 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -71,7 +71,6 @@ struct rte_kvargs_pair {
 /** Store a list of key/value associations */
 struct rte_kvargs {
 	unsigned count; /**< number of entries in the list */
-	size_t size;    /**< maximum number of entries */
 	struct rte_kvargs_pair pairs[RTE_KVARGS_MAX]; /**< list of key/values */
 };
 
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 05/11] kvargs: rework API to fix memory leak

[Olivier Matz]

Before the patch, a call to rte_kvargs_tokenize() resulted in a call to
strdup() to allocate a modifiable copy of the argument string. This
string was never freed, excepted in the error cases of
rte_kvargs_tokenize() where rte_free() was wrongly called instead of
free(). In other cases, freeing this string was impossible as the
pointer not saved.

This patch introduces rte_kvargs_free() in order to free the structure
properly. The pointer to the duplicated string is now kept in the
rte_kvargs structure. A call to rte_kvargs_parse() directly allocates
the structure, making rte_kvargs_init() useless.

The only drawback of this API change is that a key/value associations
cannot be added to an existing kvlist. But it's not used today, and
there is not obvious use case for that.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/Makefile         |  3 +-
 lib/librte_kvargs/rte_kvargs.c     | 64 ++++++++++++++++++--------------------
 lib/librte_kvargs/rte_kvargs.h     | 42 +++++++++++--------------
 lib/librte_pmd_pcap/rte_eth_pcap.c | 27 ++++++++--------
 4 files changed, 63 insertions(+), 73 deletions(-)

diff --git a/lib/librte_kvargs/Makefile b/lib/librte_kvargs/Makefile
index c4c2855..69fd414 100644
--- a/lib/librte_kvargs/Makefile
+++ b/lib/librte_kvargs/Makefile
@@ -43,8 +43,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) := rte_kvargs.c
 INCS := rte_kvargs.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_KVARGS)-include := $(INCS)
 
-# this lib needs eal and malloc
+# this lib needs eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_eal
-DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_malloc
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 0bf46fe..9c52ce4 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -34,25 +34,14 @@
 #include <string.h>
 #include <sys/user.h>
 #include <linux/binfmts.h>
+#include <stdlib.h>
 
-#include <rte_malloc.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
 
 #include "rte_kvargs.h"
 
 /*
- * Initialize a rte_kvargs structure to an empty key/value list.
- */
-int
-rte_kvargs_init(struct rte_kvargs *kvlist)
-{
-	kvlist->count = 0;
-	memset(kvlist->pairs, 0, sizeof(kvlist->pairs));
-	return 0;
-}
-
-/*
  * Add a key-value pair at the end of a given key/value list.
  * Return an error if the list is full or if the key is duplicated.
  */
@@ -93,7 +82,6 @@ static int
 rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
 {
 	unsigned i, count;
-	char *args;
 	char *pairs[RTE_KVARGS_MAX];
 	char *pair[2];
 
@@ -106,13 +94,13 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
 	/* Copy the const char *params to a modifiable string
 	 * to pass to rte_strsplit
 	 */
-	args = strdup(params);
-	if(args == NULL){
+	kvlist->str = strdup(params);
+	if (kvlist->str == NULL) {
 		RTE_LOG(ERR, PMD, "Cannot parse arguments: not enough memory\n");
 		return -1;
 	}
 
-	count = rte_strsplit(args, strnlen(args, MAX_ARG_STRLEN), pairs,
+	count = rte_strsplit(kvlist->str, strnlen(kvlist->str, MAX_ARG_STRLEN), pairs,
 			RTE_KVARGS_MAX, RTE_KVARGS_PAIRS_DELIM);
 
 	for (i = 0; i < count; i++) {
@@ -127,17 +115,13 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
 			RTE_LOG(ERR, PMD,
 				"Cannot parse arguments: wrong key or value\n"
 				"params=<%s>\n", params);
-			goto error;
+			return -1;
 		}
 
 		if (rte_kvargs_add_pair(kvlist, pair[0], pair[1]) < 0)
-			goto error;
+			return -1;
 	}
 	return 0;
-
-error:
-	rte_free(args);
-	return -1;
 }
 
 /*
@@ -222,25 +206,39 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
 	return 0;
 }
 
+/* free the rte_kvargs structure */
+void
+rte_kvargs_free(struct rte_kvargs *kvlist)
+{
+	if (kvlist->str != NULL)
+		free(kvlist->str);
+	free(kvlist);
+}
+
 /*
  * Parse the arguments "key=value;key=value;..." string and return
  * an allocated structure that contains a key/value list. Also
  * check if only valid keys were used.
  */
-int
-rte_kvargs_parse(struct rte_kvargs *kvlist,
-		const char *args,
-		const char *valid_keys[])
+struct rte_kvargs *
+rte_kvargs_parse(const char *args, const char *valid_keys[])
 {
+	struct rte_kvargs *kvlist;
 
-	int ret;
+	kvlist = malloc(sizeof(*kvlist));
+	if (kvlist == NULL)
+		return NULL;
+	memset(kvlist, 0, sizeof(*kvlist));
 
-	ret = rte_kvargs_tokenize(kvlist, args);
-	if (ret < 0)
-		return ret;
+	if (rte_kvargs_tokenize(kvlist, args) < 0) {
+		rte_kvargs_free(kvlist);
+		return NULL;
+	}
 
-	if (valid_keys == NULL)
-		return 0;
+	if (valid_keys != NULL && check_for_valid_keys(kvlist, valid_keys) < 0) {
+		rte_kvargs_free(kvlist);
+		return NULL;
+	}
 
-	return check_for_valid_keys(kvlist, valid_keys);
+	return kvlist;
 }
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 577f00b..47d78af 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -70,47 +70,41 @@ struct rte_kvargs_pair {
 
 /** Store a list of key/value associations */
 struct rte_kvargs {
+	char *str;      /**< copy of the argument string */
 	unsigned count; /**< number of entries in the list */
 	struct rte_kvargs_pair pairs[RTE_KVARGS_MAX]; /**< list of key/values */
 };
 
 /**
- * Initialize an empty rte_kvargs structure
+ * Allocate a rte_kvargs and store key/value associations from a string
  *
- * Set the content of the rte_kvargs structure given as a parameter
- * to an empty list of key/value.
+ * The function allocates and fills a rte_kvargs structure from a given
+ * string whose format is key1=value1;key2=value2;...
  *
- * @param kvlist
- *   The rte_kvargs structure
+ * The structure can be freed with rte_kvargs_free().
+ *
+ * @param args
+ *   The input string containing the key/value associations
+ * @param valid_keys
+ *   A list of valid keys (table of const char *, the last must be NULL).
+ *   This argument is ignored if NULL
  *
  * @return
- *   - 0 on success
- *   - Negative on error
+ *   - A pointer to an allocated rte_kvargs structure on success
+ *   - NULL on error
  */
-int rte_kvargs_init(struct rte_kvargs *kvlist);
+struct rte_kvargs *rte_kvargs_parse(const char *args, const char *valid_keys[]);
 
 /**
- * Append key/value associations in a rte_kvargs structure from a string
+ * Free a rte_kvargs structure
  *
- * The function fills a rte_kvargs structure from a string whose format
- * is key1=value1;key2=value2;...
- * The key/value associations are appended to those which are already
- * present in the rte_kvargs structure.
+ * Free a rte_kvargs structure previously allocated with
+ * rte_kvargs_parse().
  *
  * @param kvlist
  *   The rte_kvargs structure
- * @param args
- *   The input string containing the key/value associations
- * @param valid_keys
- *   A list of valid keys (table of const char *, the last must be NULL).
- *   This argument is ignored if NULL
- *
- * @return
- *   - 0 on success
- *   - Negative on error
  */
-int rte_kvargs_parse(struct rte_kvargs *kvlist,
-	const char *args, const char *valid_keys[]);
+void rte_kvargs_free(struct rte_kvargs *kvlist);
 
 /**
  * Call a handler function for each key/value matching the key
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 2006b35..1f46cc9 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -702,30 +702,29 @@ rte_pmd_pcap_init(const char *name, const char *params)
 {
 	unsigned numa_node, using_dumpers = 0;
 	int ret;
-	struct rte_kvargs kvlist;
+	struct rte_kvargs *kvlist;
 	struct rx_pcaps pcaps;
 	struct tx_pcaps dumpers;
 
 	RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
 
-	rte_kvargs_init(&kvlist);
-
 	numa_node = rte_socket_id();
 
 	gettimeofday(&start_time, NULL);
 	start_cycles = rte_get_timer_cycles();
 	hz = rte_get_timer_hz();
 
-	if (rte_kvargs_parse(&kvlist, params, valid_arguments) < 0)
+	kvlist = rte_kvargs_parse(params, valid_arguments);
+	if (kvlist == NULL)
 		return -1;
 
 	/*
 	 * If iface argument is passed we open the NICs and use them for
 	 * reading / writing
 	 */
-	if (rte_kvargs_count(&kvlist, ETH_PCAP_IFACE_ARG) == 1) {
+	if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) == 1) {
 
-		ret = rte_kvargs_process(&kvlist, ETH_PCAP_IFACE_ARG,
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
 				&open_rx_tx_iface, &pcaps.pcaps[0]);
 		if (ret < 0)
 			return -1;
@@ -737,13 +736,13 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	 * We check whether we want to open a RX stream from a real NIC or a
 	 * pcap file
 	 */
-	if ((pcaps.num_of_rx = rte_kvargs_count(&kvlist, ETH_PCAP_RX_PCAP_ARG))) {
-		ret = rte_kvargs_process(&kvlist, ETH_PCAP_RX_PCAP_ARG,
+	if ((pcaps.num_of_rx = rte_kvargs_count(kvlist, ETH_PCAP_RX_PCAP_ARG))) {
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
 				&open_rx_pcap, &pcaps);
 	} else {
-		pcaps.num_of_rx = rte_kvargs_count(&kvlist,
+		pcaps.num_of_rx = rte_kvargs_count(kvlist,
 				ETH_PCAP_RX_IFACE_ARG);
-		ret = rte_kvargs_process(&kvlist, ETH_PCAP_RX_IFACE_ARG,
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
 				&open_rx_iface, &pcaps);
 	}
 
@@ -754,15 +753,15 @@ rte_pmd_pcap_init(const char *name, const char *params)
 	 * We check whether we want to open a TX stream to a real NIC or a
 	 * pcap file
 	 */
-	if ((dumpers.num_of_tx = rte_kvargs_count(&kvlist,
+	if ((dumpers.num_of_tx = rte_kvargs_count(kvlist,
 			ETH_PCAP_TX_PCAP_ARG))) {
-		ret = rte_kvargs_process(&kvlist, ETH_PCAP_TX_PCAP_ARG,
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_PCAP_ARG,
 				&open_tx_pcap, &dumpers);
 		using_dumpers = 1;
 	} else {
-		dumpers.num_of_tx = rte_kvargs_count(&kvlist,
+		dumpers.num_of_tx = rte_kvargs_count(kvlist,
 				ETH_PCAP_TX_IFACE_ARG);
-		ret = rte_kvargs_process(&kvlist, ETH_PCAP_TX_IFACE_ARG,
+		ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_IFACE_ARG,
 				&open_tx_iface, &dumpers);
 	}
 
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 06/11] kvargs: simpler parsing and allow duplicated keys

[Olivier Matz]

Remove the rte_kvargs_add_pair() function whose only role was to check
if a key is duplicated. Having duplicated keys is now allowed by kvargs
API.

Also replace rte_strsplit() by more a standard function strtok_r() that
is easier to understand for people already knowing the libc. It also
avoids useless calls to strnlen(). The delimiters macros become strings
instead of chars due to the strtok_r() API.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.c | 71 ++++++++++--------------------------------
 lib/librte_kvargs/rte_kvargs.h |  8 +++--
 2 files changed, 22 insertions(+), 57 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 9c52ce4..6aaa316 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -32,8 +32,6 @@
  *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 #include <string.h>
-#include <sys/user.h>
-#include <linux/binfmts.h>
 #include <stdlib.h>
 
 #include <rte_log.h>
@@ -42,38 +40,6 @@
 #include "rte_kvargs.h"
 
 /*
- * Add a key-value pair at the end of a given key/value list.
- * Return an error if the list is full or if the key is duplicated.
- */
-static int
-rte_kvargs_add_pair(struct rte_kvargs *kvlist, char *key, char *val)
-{
-	unsigned i;
-	struct rte_kvargs_pair* entry;
-
-	/* is the list full? */
-	if (kvlist->count >= RTE_KVARGS_MAX) {
-		RTE_LOG(ERR, PMD, "Couldn't add %s, key/value list is full\n", key);
-		return -1;
-	}
-
-	/* Check if the key is duplicated */
-	for (i = 0; i < kvlist->count; i++) {
-		entry = &kvlist->pairs[i];
-		if (strcmp(entry->key, key) == 0) {
-			RTE_LOG(ERR, PMD, "Couldn't add %s, duplicated key\n", key);
-			return -1;
-		}
-	}
-
-	entry = &kvlist->pairs[kvlist->count];
-	entry->key = key;
-	entry->value = val;
-	kvlist->count++;
-	return 0;
-}
-
-/*
  * Receive a string with a list of arguments following the pattern
  * key=value;key=value;... and insert them into the list.
  * strtok() is used so the params string will be copied to be modified.
@@ -81,15 +47,8 @@ rte_kvargs_add_pair(struct rte_kvargs *kvlist, char *key, char *val)
 static int
 rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
 {
-	unsigned i, count;
-	char *pairs[RTE_KVARGS_MAX];
-	char *pair[2];
-
-	/* If params are empty, nothing to do */
-	if (params == NULL || params[0] == 0) {
-		RTE_LOG(ERR, PMD, "Cannot parse empty arguments\n");
-		return -1;
-	}
+	unsigned i;
+	char *str, *ctx1, *ctx2;
 
 	/* Copy the const char *params to a modifiable string
 	 * to pass to rte_strsplit
@@ -100,27 +59,29 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
 		return -1;
 	}
 
-	count = rte_strsplit(kvlist->str, strnlen(kvlist->str, MAX_ARG_STRLEN), pairs,
-			RTE_KVARGS_MAX, RTE_KVARGS_PAIRS_DELIM);
-
-	for (i = 0; i < count; i++) {
-		pair[0] = NULL;
-		pair[1] = NULL;
+	/* browse each key/value pair and add it in kvlist */
+	str = kvlist->str;
+	while ((str = strtok_r(str, RTE_KVARGS_PAIRS_DELIM, &ctx1)) != NULL) {
 
-		rte_strsplit(pairs[i], strnlen(pairs[i], MAX_ARG_STRLEN), pair, 2,
-				RTE_KVARGS_KV_DELIM);
+		i = kvlist->count;
+		if (i >= RTE_KVARGS_MAX) {
+			RTE_LOG(ERR, PMD, "Cannot parse arguments: list full\n");
+			return -1;
+		}
 
-		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
-				|| pair[1][0] == 0) {
+		kvlist->pairs[i].key = strtok_r(str, RTE_KVARGS_KV_DELIM, &ctx2);
+		kvlist->pairs[i].value = strtok_r(NULL, RTE_KVARGS_KV_DELIM, &ctx2);
+		if (kvlist->pairs[i].key == NULL || kvlist->pairs[i].value == NULL) {
 			RTE_LOG(ERR, PMD,
 				"Cannot parse arguments: wrong key or value\n"
 				"params=<%s>\n", params);
 			return -1;
 		}
 
-		if (rte_kvargs_add_pair(kvlist, pair[0], pair[1]) < 0)
-			return -1;
+		kvlist->count++;
+		str = NULL;
 	}
+
 	return 0;
 }
 
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 47d78af..352ae9a 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -42,6 +42,10 @@
  * This module can be used to parse arguments whose format is
  * key1=value1;key2=value2;key3=value3;...
  *
+ * The same key can appear several times with the same or a different
+ * value. Indeed, the arguments are stored as a list of key/values
+ * associations and not as a dictionary.
+ *
  * This file provides some helpers that are especially used by virtual
  * ethernet devices at initialization for arguments parsing.
  */
@@ -54,10 +58,10 @@ extern "C" {
 #define RTE_KVARGS_MAX 32
 
 /** separator character used between each pair */
-#define RTE_KVARGS_PAIRS_DELIM	';'
+#define RTE_KVARGS_PAIRS_DELIM	";"
 
 /** separator character used between key and value */
-#define RTE_KVARGS_KV_DELIM	'='
+#define RTE_KVARGS_KV_DELIM	"="
 
 /** Type of callback function used by rte_kvargs_process() */
 typedef int (*arg_handler_t)(char *value, void *opaque);
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 07/11] kvargs: be strict when matching a key

[Olivier Matz]

When we match a key in is_valid_key() and rte_kvargs_process(), do a
strict comparison (strcmp()) instead of using strstr(s1, s2) which tries
a find s1 in s2. This old behavior could lead to unexpected match, for
instance "cola" match "chocolate".

Surprisingly, no patch was needed on rte_kvargs_count() as it already
used strcmp().

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 6aaa316..73034fc 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -94,9 +94,10 @@ is_valid_key(const char *valid[], const char *key_match)
 {
 	const char **valid_ptr;
 
-	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++)
-		if (strstr(key_match, *valid_ptr) != NULL)
+	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++) {
+		if (strcmp(key_match, *valid_ptr) == 0)
 			return 1;
+	}
 	return 0;
 }
 
@@ -159,7 +160,7 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
 
 	for (i = 0; i < kvlist->count; i++) {
 		pair = &kvlist->pairs[i];
-		if (strstr(pair->key, key_match) != NULL) {
+		if (strcmp(pair->key, key_match) == 0) {
 			if ((*handler)(pair->value, opaque_arg) < 0)
 				return -1;
 		}
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 08/11] kvargs: add const attribute in handler parameters

[Olivier Matz]

The "value" argument is read-only and should be const.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.h     |  2 +-
 lib/librte_pmd_pcap/rte_eth_pcap.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 352ae9a..35572ba 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -64,7 +64,7 @@ extern "C" {
 #define RTE_KVARGS_KV_DELIM	"="
 
 /** Type of callback function used by rte_kvargs_process() */
-typedef int (*arg_handler_t)(char *value, void *opaque);
+typedef int (*arg_handler_t)(const char *value, void *opaque);
 
 /** A key/value association */
 struct rte_kvargs_pair {
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 1f46cc9..8484497 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -419,10 +419,10 @@ static struct eth_dev_ops ops = {
  * reference of it for use it later on.
  */
 static int
-open_rx_pcap(char *value, void *extra_args)
+open_rx_pcap(const char *value, void *extra_args)
 {
 	unsigned i;
-	char *pcap_filename = value;
+	const char *pcap_filename = value;
 	struct rx_pcaps *pcaps = extra_args;
 	pcap_t *rx_pcap;
 
@@ -442,10 +442,10 @@ open_rx_pcap(char *value, void *extra_args)
  * for use it later on.
  */
 static int
-open_tx_pcap(char *value, void *extra_args)
+open_tx_pcap(const char *value, void *extra_args)
 {
 	unsigned i;
-	char *pcap_filename = value;
+	const char *pcap_filename = value;
 	struct tx_pcaps *dumpers = extra_args;
 	pcap_t *tx_pcap;
 	pcap_dumper_t *dumper;
@@ -492,7 +492,7 @@ open_iface_live(const char *iface, pcap_t **pcap) {
  * Opens an interface for reading and writing
  */
 static inline int
-open_rx_tx_iface(char *value, void *extra_args)
+open_rx_tx_iface(const char *value, void *extra_args)
 {
 	const char *iface = value;
 	pcap_t **pcap = extra_args;
@@ -506,7 +506,7 @@ open_rx_tx_iface(char *value, void *extra_args)
  * Opens a NIC for reading packets from it
  */
 static inline int
-open_rx_iface(char *value, void *extra_args)
+open_rx_iface(const char *value, void *extra_args)
 {
 	unsigned i;
 	const char *iface = value;
@@ -526,7 +526,7 @@ open_rx_iface(char *value, void *extra_args)
  * Opens a NIC for writing packets to it
  */
 static inline int
-open_tx_iface(char *value, void *extra_args)
+open_tx_iface(const char *value, void *extra_args)
 {
 	unsigned i;
 	const char *iface = value;
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 09/11] kvargs: add the key in handler pameters

[Olivier Matz]

This argument can be useful when rte_kvargs_process() is called with
key=NULL, in this case the handler is invoked for all entries of the
kvlist.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.c     |  2 +-
 lib/librte_kvargs/rte_kvargs.h     |  2 +-
 lib/librte_pmd_pcap/rte_eth_pcap.c | 10 +++++-----
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 73034fc..1ff7056 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -161,7 +161,7 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
 	for (i = 0; i < kvlist->count; i++) {
 		pair = &kvlist->pairs[i];
 		if (strcmp(pair->key, key_match) == 0) {
-			if ((*handler)(pair->value, opaque_arg) < 0)
+			if ((*handler)(pair->key, pair->value, opaque_arg) < 0)
 				return -1;
 		}
 	}
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 35572ba..c080c06 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -64,7 +64,7 @@ extern "C" {
 #define RTE_KVARGS_KV_DELIM	"="
 
 /** Type of callback function used by rte_kvargs_process() */
-typedef int (*arg_handler_t)(const char *value, void *opaque);
+typedef int (*arg_handler_t)(const char *key, const char *value, void *opaque);
 
 /** A key/value association */
 struct rte_kvargs_pair {
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 8484497..6649f0f 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -419,7 +419,7 @@ static struct eth_dev_ops ops = {
  * reference of it for use it later on.
  */
 static int
-open_rx_pcap(const char *value, void *extra_args)
+open_rx_pcap(const char *key __rte_unused, const char *value, void *extra_args)
 {
 	unsigned i;
 	const char *pcap_filename = value;
@@ -442,7 +442,7 @@ open_rx_pcap(const char *value, void *extra_args)
  * for use it later on.
  */
 static int
-open_tx_pcap(const char *value, void *extra_args)
+open_tx_pcap(const char *key __rte_unused, const char *value, void *extra_args)
 {
 	unsigned i;
 	const char *pcap_filename = value;
@@ -492,7 +492,7 @@ open_iface_live(const char *iface, pcap_t **pcap) {
  * Opens an interface for reading and writing
  */
 static inline int
-open_rx_tx_iface(const char *value, void *extra_args)
+open_rx_tx_iface(const char *key __rte_unused, const char *value, void *extra_args)
 {
 	const char *iface = value;
 	pcap_t **pcap = extra_args;
@@ -506,7 +506,7 @@ open_rx_tx_iface(const char *value, void *extra_args)
  * Opens a NIC for reading packets from it
  */
 static inline int
-open_rx_iface(const char *value, void *extra_args)
+open_rx_iface(const char *key __rte_unused, const char *value, void *extra_args)
 {
 	unsigned i;
 	const char *iface = value;
@@ -526,7 +526,7 @@ open_rx_iface(const char *value, void *extra_args)
  * Opens a NIC for writing packets to it
  */
 static inline int
-open_tx_iface(const char *value, void *extra_args)
+open_tx_iface(const char *key __rte_unused, const char *value, void *extra_args)
 {
 	unsigned i;
 	const char *iface = value;
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 10/11] kvargs: make the NULL key to match all entries

[Olivier Matz]

In rte_kvargs_process() and rte_kvargs_count(), if the key_match
argument is NULL, process all entries.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_kvargs/rte_kvargs.c | 4 ++--
 lib/librte_kvargs/rte_kvargs.h | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index 1ff7056..3d65437 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -139,7 +139,7 @@ rte_kvargs_count(const struct rte_kvargs *kvlist, const char *key_match)
 	ret = 0;
 	for (i = 0; i < kvlist->count; i++) {
 		pair = &kvlist->pairs[i];
-		if (strcmp(pair->key, key_match) == 0)
+		if (key_match == NULL || strcmp(pair->key, key_match) == 0)
 			ret++;
 	}
 
@@ -160,7 +160,7 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
 
 	for (i = 0; i < kvlist->count; i++) {
 		pair = &kvlist->pairs[i];
-		if (strcmp(pair->key, key_match) == 0) {
+		if (key_match == NULL || strcmp(pair->key, key_match) == 0) {
 			if ((*handler)(pair->key, pair->value, opaque_arg) < 0)
 				return -1;
 		}
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index c080c06..e375547 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -120,7 +120,8 @@ void rte_kvargs_free(struct rte_kvargs *kvlist);
  * @param kvlist
  *   The rte_kvargs structure
  * @param key_match
- *   The key on which the handler should be called
+ *   The key on which the handler should be called, or NULL to process handler
+ *   on all associations
  * @param handler
  *   The function to call for each matching key
  * @param opaque_arg
@@ -139,7 +140,7 @@ int rte_kvargs_process(const struct rte_kvargs *kvlist,
  * @param kvlist
  *   The rte_kvargs structure
  * @param key_match
- *   The key that should match
+ *   The key that should match, or NULL to count all associations
 
  * @return
  *   The number of entries
-- 
1.8.4.rc3

[dpdk-dev] [PATCH 11/11] kvargs: add test case in app/test

[Olivier Matz]

Add a file app/test/test_kvargs.c that checks the rte_kvargs API.
The test passes:

  RTE>>kvargs
  == test valid case ==
  == test invalid case ==
  PMD: Error parsing device, invalid key <wrong-key>
  Test OK

I also tested that rte_eth_pcap runs with the following arguments:

  ./app/testpmd -c 0x15 -n 3 --proc-type=primary --huge-dir=/mnt/huge \
    --use-device="eth_pcap0;iface=ixgbe0" \
    -- -i --port-topology=chained

  ./app/testpmd -c 0x15 -n 3 --proc-type=primary --huge-dir=/mnt/huge \
    --use-device="eth_pcap0;rx_iface=ixgbe0;rx_iface=ixgbe1;tx_iface=ixgbe0" \
    -- -i --port-topology=chained

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test/Makefile      |   1 +
 app/test/commands.c    |   8 ++
 app/test/test.h        |   1 +
 app/test/test_kvargs.c | 235 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 245 insertions(+)
 create mode 100644 app/test/test_kvargs.c

diff --git a/app/test/Makefile b/app/test/Makefile
index bb2e907..b022705 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -96,6 +96,7 @@ SRCS-$(CONFIG_RTE_APP_TEST) += test_timer_perf.c
 ifeq ($(CONFIG_RTE_APP_TEST),y)
 SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_RING) += test_pmd_ring.c
+SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) += test_kvargs.c
 endif
 
 CFLAGS += -O3
diff --git a/app/test/commands.c b/app/test/commands.c
index 83f737f..9f42d36 100644
--- a/app/test/commands.c
+++ b/app/test/commands.c
@@ -2,6 +2,7 @@
  *   BSD LICENSE
  * 
  *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2014 6WIND S.A.
  *   All rights reserved.
  * 
  *   Redistribution and use in source and binary forms, with or without
@@ -191,6 +192,10 @@ static void cmd_autotest_parsed(void *parsed_result,
 	if (all || !strcmp(res->autotest, "acl_autotest"))
 		ret |= test_acl();
 #endif /* RTE_LIBRTE_ACL */
+#ifdef RTE_LIBRTE_KVARGS
+	if (all || !strcmp(res->autotest, "kvargs_autotest"))
+		ret |= test_kvargs();
+#endif /* RTE_LIBRTE_KVARGS */
 
 	if (ret == 0)
 		printf("Test OK\n");
@@ -231,6 +236,9 @@ cmdline_parse_token_string_t cmd_autotest_autotest =
 #ifdef RTE_LIBRTE_PMD_RING
 			"ring_pmd_autotest#"
 #endif
+#ifdef RTE_LIBRTE_KVARGS
+			"kvargs_autotest#"
+#endif
 			"common_autotest#all_autotests");
 
 cmdline_parse_inst_t cmd_autotest = {
diff --git a/app/test/test.h b/app/test/test.h
index bb349b2..bb77594 100644
--- a/app/test/test.h
+++ b/app/test/test.h
@@ -91,6 +91,7 @@ int test_kni(void);
 int test_power(void);
 int test_common(void);
 int test_pmd_ring(void);
+int test_kvargs(void);
 
 int test_pci_run;
 
diff --git a/app/test/test_kvargs.c b/app/test/test_kvargs.c
new file mode 100644
index 0000000..371d36d
--- /dev/null
+++ b/app/test/test_kvargs.c
@@ -0,0 +1,235 @@
+/*-
+ *   BSD LICENSE
+ * 
+ *   Copyright(c) 2014 6WIND S.A.
+ *   All rights reserved.
+ * 
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ * 
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ * 
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <rte_common.h>
+#include <rte_kvargs.h>
+
+#include "test.h"
+
+/* incrementd in handler, to check it is properly called once per
+ * key/value association */
+static unsigned count;
+
+/* this handler increment the "count" variable at each call and check
+ * that the key is "check" and the value is "value%d" */
+static int check_handler(const char *key, const char *value,
+	__rte_unused void *opaque)
+{
+	char buf[16];
+
+	/* we check that the value is "check" */
+	if (strcmp(key, "check"))
+		return -1;
+
+	/* we check that the value is "value$(count)" */
+	snprintf(buf, sizeof(buf), "value%d", count);
+	if (strncmp(buf, value, sizeof(buf)))
+		return -1;
+
+	count ++;
+	return 0;
+}
+
+/* test a valid case */
+static int test_valid_kvargs(void)
+{
+	struct rte_kvargs *kvlist;
+	const char *args;
+	const char *valid_keys_list[] = { "foo", "check", NULL };
+	const char **valid_keys;
+
+	/* empty args is valid */
+	args = "";
+	valid_keys = NULL;
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error");
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	rte_kvargs_free(kvlist);
+
+	/* first test without valid_keys */
+	args = "foo=1234;check=value0;check=value1";
+	valid_keys = NULL;
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error");
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	/* call check_handler() for all entries with key="check" */
+	count = 0;
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process() error\n");
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	if (count != 2) {
+		printf("invalid count value %d after rte_kvargs_process(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	count = 0;
+	/* call check_handler() for all entries with key="unexistant_key" */
+	if (rte_kvargs_process(kvlist, "unexistant_key", check_handler, NULL) < 0) {
+		printf("rte_kvargs_process() error\n");
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_process(unexistant_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	/* count all entries with key="foo" */
+	count = rte_kvargs_count(kvlist, "foo");
+	if (count != 1) {
+		printf("invalid count value %d after rte_kvargs_count(foo)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	/* count all entries */
+	count = rte_kvargs_count(kvlist, NULL);
+	if (count != 3) {
+		printf("invalid count value %d after rte_kvargs_count(NULL)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	/* count all entries with key="unexistant_key" */
+	count = rte_kvargs_count(kvlist, "unexistant_key");
+	if (count != 0) {
+		printf("invalid count value %d after rte_kvargs_count(unexistant_key)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	rte_kvargs_free(kvlist);
+
+	/* second test using valid_keys */
+	args = "foo=droids;check=value0;check=value1;check=wrong_value";
+	valid_keys = valid_keys_list;
+	kvlist = rte_kvargs_parse(args, valid_keys);
+	if (kvlist == NULL) {
+		printf("rte_kvargs_parse() error");
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	/* call check_handler() on all entries with key="check", it
+	 * should fail as the value is not recognized by the handler */
+	if (rte_kvargs_process(kvlist, "check", check_handler, NULL) == 0) {
+		printf("rte_kvargs_process() is success bu should not\n");
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	count = rte_kvargs_count(kvlist, "check");
+	if (count != 3) {
+		printf("invalid count value %d after rte_kvargs_count(check)\n",
+			count);
+		rte_kvargs_free(kvlist);
+		goto fail;
+	}
+	rte_kvargs_free(kvlist);
+
+	return 0;
+
+ fail:
+	printf("while processing <%s>", args);
+	if (valid_keys != NULL && *valid_keys != NULL) {
+		printf(" using valid_keys=<%s", *valid_keys);
+		while (*(++valid_keys) != NULL)
+			printf(",%s", *valid_keys);
+		printf(">");
+	}
+	printf("\n");
+	return -1;
+}
+
+/* test several error cases */
+static int test_invalid_kvargs(void)
+{
+	struct rte_kvargs *kvlist;
+	/* list of argument that should fail */
+	const char *args_list[] = {
+		"wrong-key=x",     /* key not in valid_keys_list */
+		"foo=1;foo=",      /* empty value */
+		"foo=1;;foo=2",    /* empty key/value */
+		"foo=1;foo",       /* no value */
+		"foo=1;=2",        /* no key */
+		";=",              /* also test with a smiley */
+		NULL };
+	const char **args;
+	const char *valid_keys_list[] = { "foo", "check", NULL };
+	const char **valid_keys = valid_keys_list;
+
+	for (args = args_list; *args != NULL; args++) {
+
+		kvlist = rte_kvargs_parse(*args, valid_keys);
+		if (kvlist != NULL) {
+			printf("rte_kvargs_parse() returned 0 (but should not)\n");
+			rte_kvargs_free(kvlist);
+			goto fail;
+		}
+		return 0;
+	}
+
+ fail:
+	printf("while processing <%s>", *args);
+	if (valid_keys != NULL && *valid_keys != NULL) {
+		printf(" using valid_keys=<%s", *valid_keys);
+		while (*(++valid_keys) != NULL)
+			printf(",%s", *valid_keys);
+		printf(">");
+	}
+	printf("\n");
+	return -1;
+}
+
+int test_kvargs(void)
+{
+	printf("== test valid case ==\n");
+	if (test_valid_kvargs() < 0)
+		return -1;
+	printf("== test invalid case ==\n");
+	if (test_invalid_kvargs() < 0)
+		return -1;
+	return 0;
+}
-- 
1.8.4.rc3

Re: [dpdk-dev] [PATCH 01/11] kvargs: add a new library to parse key/value arguments

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 01/11] kvargs: add a new library to parse
> key/value arguments
> 
> Copy the code from rte_eth_pcap_arg_parser.[ch], without any functional
> modifications, only:
> - rename functions and structure
> - restyle (indentation)
> - add comments (doxygen style)
> - add "const" or "static" attributes, remove unneeded "inline"
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  config/defconfig_i686-default-linuxapp-gcc   |   5 +
>  config/defconfig_i686-default-linuxapp-icc   |   5 +
>  config/defconfig_x86_64-default-linuxapp-gcc |   5 +
>  config/defconfig_x86_64-default-linuxapp-icc |   5 +
>  lib/Makefile                                 |   1 +
>  lib/librte_kvargs/Makefile                   |  50 ++++++
>  lib/librte_kvargs/rte_kvargs.c               | 248
> +++++++++++++++++++++++++++
>  lib/librte_kvargs/rte_kvargs.h               | 159 +++++++++++++++++
>  mk/rte.app.mk                                |   4 +
>  9 files changed, 482 insertions(+)
>  create mode 100644 lib/librte_kvargs/Makefile  create mode 100644
> lib/librte_kvargs/rte_kvargs.c  create mode 100644
> lib/librte_kvargs/rte_kvargs.h
> 
> diff --git a/config/defconfig_i686-default-linuxapp-gcc
> b/config/defconfig_i686-default-linuxapp-gcc
> index 4f57242..349ca24 100644
> --- a/config/defconfig_i686-default-linuxapp-gcc
> +++ b/config/defconfig_i686-default-linuxapp-gcc
> @@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
> 
>  #
> +# Compile the argument parser library
> +#
> +CONFIG_RTE_LIBRTE_KVARGS=y
> +
> +#
>  # Compile generic ethernet library
>  #
>  CONFIG_RTE_LIBRTE_ETHER=y
> diff --git a/config/defconfig_i686-default-linuxapp-icc
> b/config/defconfig_i686-default-linuxapp-icc
> index 90521e1..4e7338f 100644
> --- a/config/defconfig_i686-default-linuxapp-icc
> +++ b/config/defconfig_i686-default-linuxapp-icc
> @@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
> 
>  #
> +# Compile the argument parser library
> +#
> +CONFIG_RTE_LIBRTE_KVARGS=y
> +
> +#
>  # Compile generic ethernet library
>  #
>  CONFIG_RTE_LIBRTE_ETHER=y
> diff --git a/config/defconfig_x86_64-default-linuxapp-gcc
> b/config/defconfig_x86_64-default-linuxapp-gcc
> index 4c05ffc..ca85b1a 100644
> --- a/config/defconfig_x86_64-default-linuxapp-gcc
> +++ b/config/defconfig_x86_64-default-linuxapp-gcc
> @@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
> 
>  #
> +# Compile the argument parser library
> +#
> +CONFIG_RTE_LIBRTE_KVARGS=y
> +
> +#
>  # Compile generic ethernet library
>  #
>  CONFIG_RTE_LIBRTE_ETHER=y
> diff --git a/config/defconfig_x86_64-default-linuxapp-icc
> b/config/defconfig_x86_64-default-linuxapp-icc
> index c4d5c73..37dbdd0 100644
> --- a/config/defconfig_x86_64-default-linuxapp-icc
> +++ b/config/defconfig_x86_64-default-linuxapp-icc
> @@ -141,6 +141,11 @@ CONFIG_RTE_LIBRTE_EAL_BAREMETAL=n
> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=y
> 
>  #
> +# Compile the argument parser library
> +#
> +CONFIG_RTE_LIBRTE_KVARGS=y
> +
> +#
>  # Compile generic ethernet library
>  #
>  CONFIG_RTE_LIBRTE_ETHER=y
> diff --git a/lib/Makefile b/lib/Makefile index b655d4f..31644f2 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -52,6 +52,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_POWER) += librte_power
>  DIRS-$(CONFIG_RTE_LIBRTE_METER) += librte_meter
>  DIRS-$(CONFIG_RTE_LIBRTE_SCHED) += librte_sched
>  DIRS-$(CONFIG_RTE_LIBRTE_ACL) += librte_acl
> +DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs
> 
>  ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
>  DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni diff --git
> a/lib/librte_kvargs/Makefile b/lib/librte_kvargs/Makefile new file mode
> 100644 index 0000000..c4c2855
> --- /dev/null
> +++ b/lib/librte_kvargs/Makefile
> @@ -0,0 +1,50 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2014 6WIND S.A.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#     * Neither the name of Intel Corporation nor the names of its
> +#       contributors may be used to endorse or promote products derived
> +#       from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +# library name
> +LIB = librte_kvargs.a
> +
> +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
> +
> +# all source are stored in SRCS-y
> +SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) := rte_kvargs.c
> +
> +# install includes
> +INCS := rte_kvargs.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_KVARGS)-include := $(INCS)
> +
> +# this lib needs eal and malloc
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_eal
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_malloc
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
> new file mode 100644 index 0000000..7a950d6
> --- /dev/null
> +++ b/lib/librte_kvargs/rte_kvargs.c
> @@ -0,0 +1,248 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2014 6WIND S.A.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +#include <string.h>
> +#include <sys/user.h>
> +#include <linux/binfmts.h>
> +
> +#include <rte_malloc.h>
> +#include <rte_log.h>
> +#include <rte_string_fns.h>
> +
> +#include "rte_kvargs.h"
> +
> +/*
> + * Initialize a rte_kvargs structure to an empty key/value list.
> + */
> +int
> +rte_kvargs_init(struct rte_kvargs *kvlist) {
> +	kvlist->count = 0;
> +	kvlist->size = RTE_KVARGS_MAX;
> +	memset(kvlist->pairs, 0, kvlist->size);
> +	return 0;
> +}
> +
> +/*
> + * Add a key-value pair at the end of a given key/value list.
> + * Return an error if the list is full or if the key is duplicated.
> + */
> +static int
> +rte_kvargs_add_pair(struct rte_kvargs *kvlist, char *key, char *val) {
> +	unsigned i;
> +	struct rte_kvargs_pair* entry;
> +
> +	/* is the list full? */
> +	if (kvlist->count >= kvlist->size) {
> +		RTE_LOG(ERR, PMD, "Couldn't add %s, key/value list is
> full\n", key);
> +		return -1;
> +	}
> +
> +	/* Check if the key is duplicated */
> +	for (i = 0; i < kvlist->count; i++) {
> +		entry = &kvlist->pairs[i];
> +		if (strcmp(entry->key, key) == 0) {
> +			RTE_LOG(ERR, PMD, "Couldn't add %s, duplicated
> key\n", key);
> +			return -1;
> +		}
> +	}
> +
> +	entry = &kvlist->pairs[kvlist->count];
> +	entry->key = key;
> +	entry->value = val;
> +	kvlist->count++;
> +	return 0;
> +}
> +
> +/*
> + * Receive a string with a list of arguments following the pattern
> + * key=value;key=value;... and insert them into the list.
> + * strtok() is used so the params string will be copied to be modified.
> + */
> +static int
> +rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
> +	const char *params)
> +{
> +	unsigned i, count;
> +	char *args;
> +	char *pairs[RTE_KVARGS_MAX];
> +	char *pair[2];
> +
> +	/* If params are empty, nothing to do */
> +	if (params == NULL || params[0] == 0) {
> +		RTE_LOG(ERR, PMD, "Couldn't parse %s device, empty
> arguments\n", name);
> +		return -1;
> +	}
> +
> +	/* Copy the const char *params to a modifiable string
> +	 * to pass to rte_strsplit
> +	 */
> +	args = strdup(params);
> +	if(args == NULL){
> +		RTE_LOG(ERR, PMD, "Couldn't parse %s device \n", name);
> +		return -1;
> +	}
> +
> +	count = rte_strsplit(args, strnlen(args, MAX_ARG_STRLEN), pairs,
> +			RTE_KVARGS_MAX, RTE_KVARGS_PAIRS_DELIM);
> +
> +	for (i = 0; i < count; i++) {
> +		pair[0] = NULL;
> +		pair[1] = NULL;
> +
> +		rte_strsplit(pairs[i], strnlen(pairs[i], MAX_ARG_STRLEN),
> pair, 2,
> +				RTE_KVARGS_KV_DELIM);
> +
> +		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
> +				|| pair[1][0] == 0) {
> +			RTE_LOG(ERR, PMD,
> +					"Couldn't parse %s device, wrong key
> or value \n", name);
> +			goto error;
> +		}
> +
> +		if (rte_kvargs_add_pair(kvlist, pair[0], pair[1]) < 0)
> +			goto error;
> +	}
> +	return 0;
> +
> +error:
> +	rte_free(args);
> +	return -1;
> +}
> +
> +/*
> + * Determine whether a key is valid or not by looking
> + * into a list of valid keys.
> + */
> +static int
> +is_valid_key(const char *valid[], const char *key_match) {
> +	const char **valid_ptr;
> +
> +	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++)
> +		if (strstr(key_match, *valid_ptr) != NULL)
> +			return 1;
> +	return 0;
> +}
> +
> +/*
> + * Determine whether all keys are valid or not by looking
> + * into a list of valid keys.
> + */
> +static int
> +check_for_valid_keys(struct rte_kvargs *kvlist,
> +		const char *valid[])
> +{
> +	unsigned i, ret;
> +	struct rte_kvargs_pair *pair;
> +
> +	for (i = 0; i < kvlist->count; i++) {
> +		pair = &kvlist->pairs[i];
> +		ret = is_valid_key(valid, pair->key);
> +		if (!ret) {
> +			RTE_LOG(ERR, PMD,
> +				"Error parsing device, invalid key <%s>\n",
> +				pair->key);
> +			return -1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/*
> + * Return the number of times a given arg_name exists in the key/value
> list.
> + * E.g. given a list = { rx = 0, rx = 1, tx = 2 } the number of args
> +for
> + * arg "rx" will be 2.
> + */
> +unsigned
> +rte_kvargs_count(const struct rte_kvargs *kvlist, const char
> +*key_match) {
> +	const struct rte_kvargs_pair *pair;
> +	unsigned i, ret;
> +
> +	ret = 0;
> +	for (i = 0; i < kvlist->count; i++) {
> +		pair = &kvlist->pairs[i];
> +		if (strcmp(pair->key, key_match) == 0)
> +			ret++;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * For each matching key, call the given handler function.
> + */
> +int
> +rte_kvargs_process(const struct rte_kvargs *kvlist,
> +		const char *key_match,
> +		arg_handler_t handler,
> +		void *opaque_arg)
> +{
> +	const struct rte_kvargs_pair *pair;
> +	unsigned i;
> +
> +	for (i = 0; i < kvlist->count; i++) {
> +		pair = &kvlist->pairs[i];
> +		if (strstr(pair->key, key_match) != NULL) {
> +			if ((*handler)(pair->value, opaque_arg) < 0)
> +				return -1;
> +		}
> +	}
> +	return 0;
> +}
> +
> +/*
> + * Parse the arguments "key=value;key=value;..." string and return
> + * an allocated structure that contains a key/value list. Also
> + * check if only valid keys were used.
> + */
> +int
> +rte_kvargs_parse(struct rte_kvargs *kvlist,
> +		const char *name,
> +		const char *args,
> +		const char *valid_keys[])
> +{
> +
> +	int ret;
> +
> +	ret = rte_kvargs_tokenize(kvlist, name, args);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (valid_keys == NULL)
> +		return 0;
> +
> +	return check_for_valid_keys(kvlist, valid_keys); }
> diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
> new file mode 100644 index 0000000..19485b1
> --- /dev/null
> +++ b/lib/librte_kvargs/rte_kvargs.h
> @@ -0,0 +1,159 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2014 6WIND S.A.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +#ifndef _RTE_KVARGS_H_
> +#define _RTE_KVARGS_H_
> +
> +/**
> + * @file
> + * RTE Argument parsing
> + *
> + * This module can be used to parse arguments whose format is
> + * key1=value1;key2=value2;key3=value3;...
> + *
> + * This file provides some helpers that are especially used by virtual
> + * ethernet devices at initialization for arguments parsing.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/** Maximum number of key/value associations */ #define
> RTE_KVARGS_MAX
> +32
> +
> +/** separator character used between each pair */
> +#define RTE_KVARGS_PAIRS_DELIM	';'
> +
> +/** separator character used between key and value */
> +#define RTE_KVARGS_KV_DELIM	'='
> +
> +/** Type of callback function used by rte_kvargs_process() */ typedef
> +int (*arg_handler_t)(char *value, void *opaque);
> +
> +/** A key/value association */
> +struct rte_kvargs_pair {
> +	char *key;      /**< the name (key) of the association  */
> +	char *value;    /**< the value associated to that key */
> +};
> +
> +/** Store a list of key/value associations */ struct rte_kvargs {
> +	unsigned count; /**< number of entries in the list */
> +	size_t size;    /**< maximum number of entries */
> +	struct rte_kvargs_pair pairs[RTE_KVARGS_MAX]; /**< list of
> key/values
> +*/ };
> +
> +/**
> + * Initialize an empty rte_kvargs structure
> + *
> + * Set the content of the rte_kvargs structure given as a parameter
> + * to an empty list of key/value.
> + *
> + * @param kvlist
> + *   The rte_kvargs structure
> + *
> + * @return
> + *   - 0 on success
> + *   - Negative on error
> + */
> +int rte_kvargs_init(struct rte_kvargs *kvlist);
> +
> +/**
> + * Append key/value associations in a rte_kvargs structure from a
> +string
> + *
> + * The function fills a rte_kvargs structure from a string whose format
> + * is key1=value1;key2=value2;...
> + * The key/value associations are appended to those which are already
> + * present in the rte_kvargs structure.
> + *
> + * @param kvlist
> + *   The rte_kvargs structure
> + * @param name
> + *   The name of the driver
> + * @param args
> + *   The input string containing the key/value associations
> + * @param valid_keys
> + *   A list of valid keys (table of const char *, the last must be NULL).
> + *   This argument is ignored if NULL
> + *
> + * @return
> + *   - 0 on success
> + *   - Negative on error
> + */
> +int rte_kvargs_parse(struct rte_kvargs *kvlist, const char *name,
> +	const char *args, const char *valid_keys[]);
> +
> +/**
> + * Call a handler function for each key/value matching the key
> + *
> + * For each key/value association that matches the given key, calls the
> + * handler function with the for a given arg_name passing the value on
> +the
> + * dictionary for that key and a given extra argument.
> + *
> + * @param kvlist
> + *   The rte_kvargs structure
> + * @param key_match
> + *   The key on which the handler should be called
> + * @param handler
> + *   The function to call for each matching key
> + * @param opaque_arg
> + *   A pointer passed unchanged to the handler
> + *
> + * @return
> + *   - 0 on success
> + *   - Negative on error
> + */
> +int rte_kvargs_process(const struct rte_kvargs *kvlist,
> +	const char *key_match, arg_handler_t handler, void *opaque_arg);
> +
> +/**
> + * Count the number of associations matching the given key
> + *
> + * @param kvlist
> + *   The rte_kvargs structure
> + * @param key_match
> + *   The key that should match
> +
> + * @return
> + *   The number of entries
> + */
> +unsigned rte_kvargs_count(const struct rte_kvargs *kvlist,
> +	const char *key_match);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 786100c..1652029
> 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -112,6 +112,10 @@ endif
> 
>  LDLIBS += --start-group
> 
> +ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> +LDLIBS += -lrte_kvargs
> +endif
> +
>  ifeq ($(CONFIG_RTE_LIBRTE_MBUF),y)
>  LDLIBS += -lrte_mbuf
>  endif
> --
> 1.8.4.rc3

Acked-by: Bruce Richardson<bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 02/11] kvargs: use the new library in pmd_pcap

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 02/11] kvargs: use the new library in pmd_pcap
> 
> The rte_kvargs library is a reworked copy of rte_eth_pcap_arg_parser, so it
> provides the same service. Therefore we can use it and remove the code of
> rte_eth_pcap_arg_parser.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_pmd_pcap/Makefile                  |   8 +-
>  lib/librte_pmd_pcap/rte_eth_pcap.c            |  29 +--
>  lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c | 255 --------------------------
> lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h |  71 -------
>  4 files changed, 19 insertions(+), 344 deletions(-)  delete mode 100644
> lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
>  delete mode 100644 lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h
> 
> diff --git a/lib/librte_pmd_pcap/Makefile b/lib/librte_pmd_pcap/Makefile
> index 33174c0..741dafc 100644
> --- a/lib/librte_pmd_pcap/Makefile
> +++ b/lib/librte_pmd_pcap/Makefile
> @@ -1,6 +1,7 @@
>  #   BSD LICENSE
>  #
>  #   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> +#   Copyright(c) 2014 6WIND S.A.
>  #   All rights reserved.
>  #
>  #   Redistribution and use in source and binary forms, with or without
> @@ -43,16 +44,15 @@ CFLAGS += $(WERROR_FLAGS)  # all source are
> stored in SRCS-y  #
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += rte_eth_pcap.c
> -SRCS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += rte_eth_pcap_arg_parser.c
> -
> 
>  #
>  # Export include files
>  #
>  SYMLINK-y-include += rte_eth_pcap.h
> -SYMLINK-y-include += rte_eth_pcap_arg_parser.h
> 
>  # this lib depends upon:
> -DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_mbuf
> lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_mbuf
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_kvargs
> 
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c
> b/lib/librte_pmd_pcap/rte_eth_pcap.c
> index 208e316..e47afcb 100644
> --- a/lib/librte_pmd_pcap/rte_eth_pcap.c
> +++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
> @@ -2,6 +2,7 @@
>   *   BSD LICENSE
>   *
>   *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2014 6WIND S.A.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -39,9 +40,9 @@
>  #include <rte_string_fns.h>
>  #include <rte_cycles.h>
>  #include <net/if.h>
> +#include <rte_kvargs.h>
> 
>  #include "rte_eth_pcap.h"
> -#include "rte_eth_pcap_arg_parser.h"
> 
>  #define RTE_ETH_PCAP_SNAPSHOT_LEN 65535  #define
> RTE_ETH_PCAP_SNAPLEN 4096 @@ -701,11 +702,11 @@
> rte_pmd_pcap_init(const char *name, const char *params)  {
>  	unsigned numa_node, using_dumpers = 0;
>  	int ret;
> -	struct args_dict dict;
> +	struct rte_kvargs kvlist;
>  	struct rx_pcaps pcaps;
>  	struct tx_pcaps dumpers;
> 
> -	rte_eth_pcap_init_args_dict(&dict);
> +	rte_kvargs_init(&kvlist);
> 
>  	numa_node = rte_socket_id();
> 
> @@ -713,16 +714,16 @@ rte_pmd_pcap_init(const char *name, const
> char *params)
>  	start_cycles = rte_get_timer_cycles();
>  	hz = rte_get_timer_hz();
> 
> -	if (rte_eth_pcap_parse_args(&dict, name, params,
> valid_arguments) < 0)
> +	if (rte_kvargs_parse(&kvlist, name, params, valid_arguments) < 0)
>  		return -1;
> 
>  	/*
>  	 * If iface argument is passed we open the NICs and use them for
>  	 * reading / writing
>  	 */
> -	if (rte_eth_pcap_num_of_args(&dict, ETH_PCAP_IFACE_ARG) == 1)
> {
> +	if (rte_kvargs_count(&kvlist, ETH_PCAP_IFACE_ARG) == 1) {
> 
> -		ret = rte_eth_pcap_post_process_arguments(&dict,
> ETH_PCAP_IFACE_ARG,
> +		ret = rte_kvargs_process(&kvlist, ETH_PCAP_IFACE_ARG,
>  				&open_rx_tx_iface, &pcaps.pcaps[0]);
>  		if (ret < 0)
>  			return -1;
> @@ -734,13 +735,13 @@ rte_pmd_pcap_init(const char *name, const
> char *params)
>  	 * We check whether we want to open a RX stream from a real NIC
> or a
>  	 * pcap file
>  	 */
> -	if ((pcaps.num_of_rx = rte_eth_pcap_num_of_args(&dict,
> ETH_PCAP_RX_PCAP_ARG))) {
> -		ret = rte_eth_pcap_post_process_arguments(&dict,
> ETH_PCAP_RX_PCAP_ARG,
> +	if ((pcaps.num_of_rx = rte_kvargs_count(&kvlist,
> ETH_PCAP_RX_PCAP_ARG))) {
> +		ret = rte_kvargs_process(&kvlist,
> ETH_PCAP_RX_PCAP_ARG,
>  				&open_rx_pcap, &pcaps);
>  	} else {
> -		pcaps.num_of_rx = rte_eth_pcap_num_of_args(&dict,
> +		pcaps.num_of_rx = rte_kvargs_count(&kvlist,
>  				ETH_PCAP_RX_IFACE_ARG);
> -		ret = rte_eth_pcap_post_process_arguments(&dict,
> ETH_PCAP_RX_IFACE_ARG,
> +		ret = rte_kvargs_process(&kvlist,
> ETH_PCAP_RX_IFACE_ARG,
>  				&open_rx_iface, &pcaps);
>  	}
> 
> @@ -751,15 +752,15 @@ rte_pmd_pcap_init(const char *name, const
> char *params)
>  	 * We check whether we want to open a TX stream to a real NIC or a
>  	 * pcap file
>  	 */
> -	if ((dumpers.num_of_tx = rte_eth_pcap_num_of_args(&dict,
> +	if ((dumpers.num_of_tx = rte_kvargs_count(&kvlist,
>  			ETH_PCAP_TX_PCAP_ARG))) {
> -		ret = rte_eth_pcap_post_process_arguments(&dict,
> ETH_PCAP_TX_PCAP_ARG,
> +		ret = rte_kvargs_process(&kvlist,
> ETH_PCAP_TX_PCAP_ARG,
>  				&open_tx_pcap, &dumpers);
>  		using_dumpers = 1;
>  	} else {
> -		dumpers.num_of_tx = rte_eth_pcap_num_of_args(&dict,
> +		dumpers.num_of_tx = rte_kvargs_count(&kvlist,
>  				ETH_PCAP_TX_IFACE_ARG);
> -		ret = rte_eth_pcap_post_process_arguments(&dict,
> ETH_PCAP_TX_IFACE_ARG,
> +		ret = rte_kvargs_process(&kvlist,
> ETH_PCAP_TX_IFACE_ARG,
>  				&open_tx_iface, &dumpers);
>  	}
> 
> diff --git a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
> b/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
> deleted file mode 100644
> index c881f3b..0000000
> --- a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.c
> +++ /dev/null
> @@ -1,255 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> - */
> -#include <string.h>
> -#include <sys/user.h>
> -#include <linux/binfmts.h>
> -
> -#include <rte_malloc.h>
> -#include <rte_log.h>
> -#include <rte_string_fns.h>
> -
> -#include "rte_eth_pcap_arg_parser.h"
> -
> -/*
> - * Initializes a non NULL dictionary reference to be used later on.
> - */
> -inline int
> -rte_eth_pcap_init_args_dict(struct args_dict *dict) -{
> -	dict->index = 0;
> -	dict->size = RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS;
> -	memset(dict->pairs, 0, dict->size);
> -	return 0;
> -}
> -
> -/*
> - * Adds a key-value pair to a given non-NULL dictionary reference.
> - * The final key will be the name+key.
> - * Returns error in case the dictionary is full or if the key is duplicated.
> - */
> -inline int
> -rte_eth_pcap_add_pair_to_dict(struct args_dict *dict,
> -		char *key,
> -		char *val)
> -{
> -	unsigned i;
> -	struct key_value* entry;
> -
> -	/* is the dictionary full? */
> -	if (dict->index >= dict->size) {
> -		RTE_LOG(ERR, PMD, "Couldn't add %s, dictionary is full\n",
> key);
> -		return -1;
> -	}
> -
> -	/* Check if the key is duplicated */
> -	for (i = 0; i < dict->index; i++) {
> -		entry = &dict->pairs[i];
> -		if (strcmp(entry->key, key) == 0) {
> -			RTE_LOG(ERR, PMD, "Couldn't add %s, duplicated
> key\n", key);
> -			return -1;
> -		}
> -	}
> -
> -	entry = &dict->pairs[dict->index];
> -	entry->key = key;
> -	entry->value = val;
> -	dict->index++;
> -	return 0;
> -
> -}
> -
> -#define RTE_ETH_PCAP_PAIRS_DELIM		';'
> -#define RTE_ETH_PCAP_KEY_VALUE_DELIM	'='
> -/*
> - * Receives a string with a list of arguments following the pattern
> - * key=value;key=value;... and inserts them into the non NULL dictionary.
> - * strtok is used so the params string will be copied to be modified.
> - */
> -inline int
> -rte_eth_pcap_tokenize_args(struct args_dict *dict,
> -		const char *name,
> -		const char *params)
> -{
> -	int i;
> -	char *args;
> -	char *pairs[RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS];
> -	char *pair[2];
> -	int num_of_pairs;
> -
> -	/* If params are empty, nothing to do */
> -	if (params == NULL || params[0] == 0) {
> -		RTE_LOG(ERR, PMD, "Couldn't parse %s device, empty
> arguments\n", name);
> -		return -1;
> -	}
> -
> -	/* Copy the const char *params to a modifiable string
> -	 * to pass to rte_strsplit
> -	 */
> -	args = strdup(params);
> -	if(args == NULL){
> -		RTE_LOG(ERR, PMD, "Couldn't parse %s device \n", name);
> -		return -1;
> -	}
> -
> -	num_of_pairs = rte_strsplit(args, strnlen(args, MAX_ARG_STRLEN),
> pairs,
> -			RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS,
> RTE_ETH_PCAP_PAIRS_DELIM);
> -
> -	for (i = 0; i < num_of_pairs; i++) {
> -		pair[0] = NULL;
> -		pair[1] = NULL;
> -
> -		rte_strsplit(pairs[i], strnlen(pairs[i], MAX_ARG_STRLEN),
> pair, 2,
> -				RTE_ETH_PCAP_KEY_VALUE_DELIM);
> -
> -		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
> -				|| pair[1][0] == 0) {
> -			RTE_LOG(ERR, PMD,
> -					"Couldn't parse %s device, wrong key
> or value \n", name);
> -			goto error;
> -		}
> -
> -		if (rte_eth_pcap_add_pair_to_dict(dict, pair[0], pair[1]) < 0)
> -			goto error;
> -	}
> -	return 0;
> -
> -error:
> -	rte_free(args);
> -	return -1;
> -}
> -
> -/*
> - * Determines whether a key is valid or not by looking
> - * into a list of valid keys.
> - */
> -static inline int
> -is_valid_key(const char *valid[],
> -		struct key_value *pair)
> -{
> -	const char **valid_ptr;
> -
> -	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++)
> -		if (strstr(pair->key, *valid_ptr) != NULL)
> -			return 1;
> -	return 0;
> -}
> -
> -/*
> - * Determines whether all keys are valid or not by looking
> - * into a list of valid keys.
> - */
> -static inline int
> -check_for_valid_keys(struct args_dict *dict,
> -		const char *valid[])
> -{
> -	unsigned k_index, ret;
> -	struct key_value *pair;
> -
> -	for (k_index = 0; k_index < dict->index; k_index++) {
> -		pair = &dict->pairs[k_index];
> -		ret = is_valid_key(valid, pair);
> -		if (!ret) {
> -			RTE_LOG(ERR, PMD,
> -					"Error parsing device, invalid key
> %s\n", pair->key);
> -			return -1;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * Returns the number of times a given arg_name exists on a dictionary.
> - * E.g. given a dict = { rx0 = 0, rx1 = 1, tx0 = 2 } the number of args for
> - * arg "rx" will be 2.
> - */
> -inline unsigned
> -rte_eth_pcap_num_of_args(struct args_dict *dict, const char *arg_name)
> -{
> -	unsigned k_index;
> -	struct key_value *pair;
> -	unsigned num_of_keys;
> -
> -	num_of_keys = 0;
> -	for (k_index = 0; k_index < dict->index; k_index++) {
> -		pair = &dict->pairs[k_index];
> -		if (strcmp(pair->key, arg_name) == 0)
> -			num_of_keys++;
> -	}
> -
> -	return num_of_keys;
> -}
> -
> -/*
> - * Calls the handler function for a given arg_name passing the
> - * value on the dictionary for that key and a given extra argument.
> - */
> -inline int
> -rte_eth_pcap_post_process_arguments(struct args_dict *dict,
> -		const char *arg_name,
> -		arg_handler_t handler,
> -		void *extra_args)
> -{
> -	unsigned k_index;
> -	struct key_value *pair;
> -
> -	for (k_index = 0; k_index < dict->index; k_index++) {
> -		pair = &dict->pairs[k_index];
> -		if (strstr(pair->key, arg_name) != NULL) {
> -			if ((*handler)(pair->value, extra_args) < 0)
> -				return -1;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/*
> - * Parses the arguments "key=value;key=value;..." string and returns
> - * a simple dictionary implementation containing these pairs. It also
> - * checks if only valid keys were used.
> - */
> -inline int
> -rte_eth_pcap_parse_args(struct args_dict *dict,
> -		const char *name,
> -		const char *args,
> -		const char *valids[])
> -{
> -
> -	int ret;
> -
> -	ret = rte_eth_pcap_tokenize_args(dict, name, args);
> -	if (ret < 0)
> -		return ret;
> -
> -	return check_for_valid_keys(dict, valids);
> -}
> -
> diff --git a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h
> b/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h
> deleted file mode 100644
> index 47eb3a0..0000000
> --- a/lib/librte_pmd_pcap/rte_eth_pcap_arg_parser.h
> +++ /dev/null
> @@ -1,71 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> - */
> -
> -#ifndef _RTE_ETH_ARG_PARSER_H_
> -#define _RTE_ETH_ARG_PARSER_H_
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -#define RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS 32
> -
> -typedef int (*arg_handler_t)(char*, void*);
> -
> -struct key_value {
> -	char *key;
> -	char *value;
> -};
> -
> -struct args_dict {
> -	unsigned index;
> -	size_t size;
> -	struct key_value pairs[RTE_ETH_PCAP_ARG_PARSER_MAX_ARGS];
> -};
> -
> -int rte_eth_pcap_tokenize_args(struct args_dict *dict, const char *name,
> -		const char *args);
> -int rte_eth_pcap_init_args_dict(struct args_dict *dict); -int
> rte_eth_pcap_add_pair_to_dict(struct args_dict *dict, char *key, char
> *val); -int rte_eth_pcap_parse_args(struct args_dict *dict, const char*
> name,
> -		const char *args, const char *valids[]);
> -int rte_eth_pcap_post_process_arguments(struct args_dict *dict,
> -		const char *arg_name, arg_handler_t handler, void
> *extra_args);
> -unsigned rte_eth_pcap_num_of_args(struct args_dict *dict, const char
> *key); -void rte_eth_pcap_free_dict(struct args_dict *dict);
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -
> -#endif
> --
> 1.8.4.rc3

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 03/11] kvargs: remove driver name in arguments

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 03/11] kvargs: remove driver name in
> arguments
> 
> Now that rte_kvargs is a generic library, there is no need to have an
> argument for the driver name in rte_kvargs_tokenize() and
> rte_kvargs_parse() prototypes. This argument was only used to log the
> driver name in case of error. Instead, we can add a log in init function of
> pmd_pcap and pmd_ring.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/rte_kvargs.c     | 13 ++++++-------
>  lib/librte_kvargs/rte_kvargs.h     |  4 +---
>  lib/librte_pmd_pcap/rte_eth_pcap.c |  4 +++-
> lib/librte_pmd_ring/rte_eth_ring.c |  2 ++
>  4 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
> index 7a950d6..935698c 100644
> --- a/lib/librte_kvargs/rte_kvargs.c
> +++ b/lib/librte_kvargs/rte_kvargs.c
> @@ -91,8 +91,7 @@ rte_kvargs_add_pair(struct rte_kvargs *kvlist, char
> *key, char *val)
>   * strtok() is used so the params string will be copied to be modified.
>   */
>  static int
> -rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *name,
> -	const char *params)
> +rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
>  {
>  	unsigned i, count;
>  	char *args;
> @@ -101,7 +100,7 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist,
> const char *name,
> 
>  	/* If params are empty, nothing to do */
>  	if (params == NULL || params[0] == 0) {
> -		RTE_LOG(ERR, PMD, "Couldn't parse %s device, empty
> arguments\n", name);
> +		RTE_LOG(ERR, PMD, "Cannot parse empty arguments\n");
>  		return -1;
>  	}
> 
> @@ -110,7 +109,7 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist,
> const char *name,
>  	 */
>  	args = strdup(params);
>  	if(args == NULL){
> -		RTE_LOG(ERR, PMD, "Couldn't parse %s device \n", name);
> +		RTE_LOG(ERR, PMD, "Cannot parse arguments: not enough
> memory\n");
>  		return -1;
>  	}
> 
> @@ -127,7 +126,8 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist,
> const char *name,
>  		if (pair[0] == NULL || pair[1] == NULL || pair[0][0] == 0
>  				|| pair[1][0] == 0) {
>  			RTE_LOG(ERR, PMD,
> -					"Couldn't parse %s device, wrong key
> or value \n", name);
> +				"Cannot parse arguments: wrong key or
> value\n"
> +				"params=<%s>\n", params);
>  			goto error;
>  		}
> 
> @@ -230,14 +230,13 @@ rte_kvargs_process(const struct rte_kvargs
> *kvlist,
>   */
>  int
>  rte_kvargs_parse(struct rte_kvargs *kvlist,
> -		const char *name,
>  		const char *args,
>  		const char *valid_keys[])
>  {
> 
>  	int ret;
> 
> -	ret = rte_kvargs_tokenize(kvlist, name, args);
> +	ret = rte_kvargs_tokenize(kvlist, args);
>  	if (ret < 0)
>  		return ret;
> 
> diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
> index 19485b1..804ea1d 100644
> --- a/lib/librte_kvargs/rte_kvargs.h
> +++ b/lib/librte_kvargs/rte_kvargs.h
> @@ -100,8 +100,6 @@ int rte_kvargs_init(struct rte_kvargs *kvlist);
>   *
>   * @param kvlist
>   *   The rte_kvargs structure
> - * @param name
> - *   The name of the driver
>   * @param args
>   *   The input string containing the key/value associations
>   * @param valid_keys
> @@ -112,7 +110,7 @@ int rte_kvargs_init(struct rte_kvargs *kvlist);
>   *   - 0 on success
>   *   - Negative on error
>   */
> -int rte_kvargs_parse(struct rte_kvargs *kvlist, const char *name,
> +int rte_kvargs_parse(struct rte_kvargs *kvlist,
>  	const char *args, const char *valid_keys[]);
> 
>  /**
> diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c
> b/lib/librte_pmd_pcap/rte_eth_pcap.c
> index e47afcb..2006b35 100644
> --- a/lib/librte_pmd_pcap/rte_eth_pcap.c
> +++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
> @@ -706,6 +706,8 @@ rte_pmd_pcap_init(const char *name, const char
> *params)
>  	struct rx_pcaps pcaps;
>  	struct tx_pcaps dumpers;
> 
> +	RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
> +
>  	rte_kvargs_init(&kvlist);
> 
>  	numa_node = rte_socket_id();
> @@ -714,7 +716,7 @@ rte_pmd_pcap_init(const char *name, const char
> *params)
>  	start_cycles = rte_get_timer_cycles();
>  	hz = rte_get_timer_hz();
> 
> -	if (rte_kvargs_parse(&kvlist, name, params, valid_arguments) < 0)
> +	if (rte_kvargs_parse(&kvlist, params, valid_arguments) < 0)
>  		return -1;
> 
>  	/*
> diff --git a/lib/librte_pmd_ring/rte_eth_ring.c
> b/lib/librte_pmd_ring/rte_eth_ring.c
> index fa3ff72..abef2e8 100644
> --- a/lib/librte_pmd_ring/rte_eth_ring.c
> +++ b/lib/librte_pmd_ring/rte_eth_ring.c
> @@ -384,6 +384,8 @@ rte_eth_ring_pair_attach(const char *name, const
> unsigned numa_node)  int  rte_pmd_ring_init(const char *name, const char
> *params)  {
> +	RTE_LOG(INFO, PMD, "Initializing pmd_ring for %s\n", name);
> +
>  	if (params == NULL)
>  		eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE);
>  	else {
> --
> 1.8.4.rc3

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 04/11] kvargs: remove useless size field

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 04/11] kvargs: remove useless size field
> 
> This value was not very useful as the size of the table is fixed (equals
> RTE_KVARGS_MAX).
> 
> By the way, the memset in the initialization function was wrong (size too
> short). Even if it was not really an issue since we rely on the "count" field, it
> is now fixed by this patch.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 06/11] kvargs: simpler parsing and allow duplicated keys

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 06/11] kvargs: simpler parsing and allow
> duplicated keys
> 
> Remove the rte_kvargs_add_pair() function whose only role was to check if
> a key is duplicated. Having duplicated keys is now allowed by kvargs API.
> 
> Also replace rte_strsplit() by more a standard function strtok_r() that is
> easier to understand for people already knowing the libc. It also avoids
> useless calls to strnlen(). The delimiters macros become strings instead of
> chars due to the strtok_r() API.
> 

As a general rule, we try to use only string functions which track the buffer length they are working with, which is why the function rte_strsplit() is used. While strtok_r() is indeed a standard C function, why not use the code as originally written?

Re: [dpdk-dev] [PATCH 06/11] kvargs: simpler parsing and allow duplicated keys

[Olivier MATZ]

Hi Bruce,

>> Also replace rte_strsplit() by more a standard function strtok_r() that is
>> easier to understand for people already knowing the libc. It also avoids
>> useless calls to strnlen(). The delimiters macros become strings instead of
>> chars due to the strtok_r() API.
>>
> 
> As a general rule, we try to use only string functions which
> track the buffer length they are working with, which is why the
> function rte_strsplit() is used. While strtok_r() is indeed a
> standard C function, why not use the code as originally written?

Thank you for your comment.

Another reason for not using rte_strsplit() is that the buffer length
argument that was given was not relevant. Indeed, the string we want to
parse initially comes from the command line arguments, which is
duplicated with strdup(). It is not stored in a fixed size buffer.

In my opinion, giving strnlen(pairs[i], MAX_ARG_STRLEN) as the size
to rte_strsplit() would not prevent the program to a segfault or to
read a corrupted string if it is not properly null-terminated.

Does it makes sense ?

Regards
Olivier

Re: [dpdk-dev] [PATCH 05/11] kvargs: rework API to fix memory leak

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 05/11] kvargs: rework API to fix memory leak
> 
> Before the patch, a call to rte_kvargs_tokenize() resulted in a call to
> strdup() to allocate a modifiable copy of the argument string. This string
> was never freed, excepted in the error cases of
> rte_kvargs_tokenize() where rte_free() was wrongly called instead of free().
> In other cases, freeing this string was impossible as the pointer not saved.
> 
> This patch introduces rte_kvargs_free() in order to free the structure
> properly. The pointer to the duplicated string is now kept in the rte_kvargs
> structure. A call to rte_kvargs_parse() directly allocates the structure,
> making rte_kvargs_init() useless.
> 
> The only drawback of this API change is that a key/value associations
> cannot be added to an existing kvlist. But it's not used today, and there is
> not obvious use case for that.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/Makefile         |  3 +-
>  lib/librte_kvargs/rte_kvargs.c     | 64 ++++++++++++++++++--------------------
>  lib/librte_kvargs/rte_kvargs.h     | 42 +++++++++++--------------
>  lib/librte_pmd_pcap/rte_eth_pcap.c | 27 ++++++++--------
>  4 files changed, 63 insertions(+), 73 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 06/11] kvargs: simpler parsing and allow duplicated keys

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 06/11] kvargs: simpler parsing and allow
> duplicated keys
> 
> Remove the rte_kvargs_add_pair() function whose only role was to check if
> a key is duplicated. Having duplicated keys is now allowed by kvargs API.
> 
> Also replace rte_strsplit() by more a standard function strtok_r() that is
> easier to understand for people already knowing the libc. It also avoids
> useless calls to strnlen(). The delimiters macros become strings instead of
> chars due to the strtok_r() API.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/rte_kvargs.c | 71 ++++++++++--------------------------------
>  lib/librte_kvargs/rte_kvargs.h |  8 +++--
>  2 files changed, 22 insertions(+), 57 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 07/11] kvargs: be strict when matching a key

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 07/11] kvargs: be strict when matching a key
> 
> When we match a key in is_valid_key() and rte_kvargs_process(), do a strict
> comparison (strcmp()) instead of using strstr(s1, s2) which tries a find s1 in
> s2. This old behavior could lead to unexpected match, for instance "cola"
> match "chocolate".
> 
> Surprisingly, no patch was needed on rte_kvargs_count() as it already used
> strcmp().
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/rte_kvargs.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
> index 6aaa316..73034fc 100644
> --- a/lib/librte_kvargs/rte_kvargs.c
> +++ b/lib/librte_kvargs/rte_kvargs.c
> @@ -94,9 +94,10 @@ is_valid_key(const char *valid[], const char
> *key_match)  {
>  	const char **valid_ptr;
> 
> -	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++)
> -		if (strstr(key_match, *valid_ptr) != NULL)
> +	for (valid_ptr = valid; *valid_ptr != NULL; valid_ptr++) {
> +		if (strcmp(key_match, *valid_ptr) == 0)
>  			return 1;
> +	}
>  	return 0;
>  }
> 
> @@ -159,7 +160,7 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
> 
>  	for (i = 0; i < kvlist->count; i++) {
>  		pair = &kvlist->pairs[i];
> -		if (strstr(pair->key, key_match) != NULL) {
> +		if (strcmp(pair->key, key_match) == 0) {
>  			if ((*handler)(pair->value, opaque_arg) < 0)
>  				return -1;
>  		}
> --
> 1.8.4.rc3

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 08/11] kvargs: add const attribute in handler parameters

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 08/11] kvargs: add const attribute in handler
> parameters
> 
> The "value" argument is read-only and should be const.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/rte_kvargs.h     |  2 +-
>  lib/librte_pmd_pcap/rte_eth_pcap.c | 14 +++++++-------
>  2 files changed, 8 insertions(+), 8 deletions(-)

 Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 09/11] kvargs: add the key in handler pameters

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 09/11] kvargs: add the key in handler pameters
> 
> This argument can be useful when rte_kvargs_process() is called with
> key=NULL, in this case the handler is invoked for all entries of the kvlist.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/rte_kvargs.c     |  2 +-
>  lib/librte_kvargs/rte_kvargs.h     |  2 +-
>  lib/librte_pmd_pcap/rte_eth_pcap.c | 10 +++++-----
>  3 files changed, 7 insertions(+), 7 deletions(-)

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 10/11] kvargs: make the NULL key to match all entries

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 10/11] kvargs: make the NULL key to match all
> entries
> 
> In rte_kvargs_process() and rte_kvargs_count(), if the key_match argument
> is NULL, process all entries.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_kvargs/rte_kvargs.c | 4 ++--  lib/librte_kvargs/rte_kvargs.h | 5
> +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 11/11] kvargs: add test case in app/test

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, January 28, 2014 4:07 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 11/11] kvargs: add test case in app/test
> 
> Add a file app/test/test_kvargs.c that checks the rte_kvargs API.
> The test passes:
> 
>   RTE>>kvargs
>   == test valid case ==
>   == test invalid case ==
>   PMD: Error parsing device, invalid key <wrong-key>
>   Test OK
> 
> I also tested that rte_eth_pcap runs with the following arguments:
> 
>   ./app/testpmd -c 0x15 -n 3 --proc-type=primary --huge-dir=/mnt/huge \
>     --use-device="eth_pcap0;iface=ixgbe0" \
>     -- -i --port-topology=chained
> 
>   ./app/testpmd -c 0x15 -n 3 --proc-type=primary --huge-dir=/mnt/huge \
>     --use-
> device="eth_pcap0;rx_iface=ixgbe0;rx_iface=ixgbe1;tx_iface=ixgbe0" \
>     -- -i --port-topology=chained
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  app/test/Makefile      |   1 +
>  app/test/commands.c    |   8 ++
>  app/test/test.h        |   1 +
>  app/test/test_kvargs.c | 235
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 245 insertions(+)
>  create mode 100644 app/test/test_kvargs.c

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH 00/11] add rte_kvargs library: a key/value args parser

[Thomas Monjalon]

28/01/2014 17:06, Olivier Matz:
> The topic of this patchset is to add a new rte_kvargs library that
> can be used as a helper to parse key/value arguments. The code will
> be based on rte_eth_pcap_arg_parser and reworked with documentation
> and tests. It also fix some minor issues of the original code.
> 
> These commits will allow another library (like pmd_ring) to parse
> arguments in an easier way without duplicating the code.
> 
> Olivier Matz (11):
>   kvargs: add a new library to parse key/value arguments
>   kvargs: use the new library in pmd_pcap
>   kvargs: remove driver name in arguments
>   kvargs: remove useless size field
>   kvargs: rework API to fix memory leak
>   kvargs: simpler parsing and allow duplicated keys
>   kvargs: be strict when matching a key
>   kvargs: add const attribute in handler parameters
>   kvargs: add the key in handler pameters
>   kvargs: make the NULL key to match all entries
>   kvargs: add test case in app/test

All is applied. Thanks for this cleanup effort.

-- 
Thomas