parse_u16() ignores the end pointer, so "8junk" is accepted, and assigns a u64 into a u16 without a range check, so "65536" wraps to zero. Zero is still rejected, as it is the unset marker for quanta_size.
parse_bool() checks neither errno nor the end pointer. iavf_parse_watchdog_period() uses atoi(), which cannot report an error at all, so a malformed value becomes zero and the negative check never fires. no-poll-on-link-down, auto_reset and enable_lldp are booleans, so make the fields bool and use rte_kvargs_handle_bool(). That is every caller of the local parse_bool(), so it goes away too. A bare key now enables the option, and the usual spellings are accepted. Signed-off-by: Stephen Hemminger <[email protected]> --- drivers/net/intel/iavf/iavf.h | 6 +-- drivers/net/intel/iavf/iavf_ethdev.c | 55 +++++++++------------------- 2 files changed, 20 insertions(+), 41 deletions(-) diff --git a/drivers/net/intel/iavf/iavf.h b/drivers/net/intel/iavf/iavf.h index eb7cf2b58d..8f31fbabbf 100644 --- a/drivers/net/intel/iavf/iavf.h +++ b/drivers/net/intel/iavf/iavf.h @@ -324,10 +324,10 @@ struct iavf_devargs { uint8_t proto_xtr[IAVF_MAX_QUEUE_NUM]; uint16_t quanta_size; uint32_t watchdog_period; - int auto_reset; - int no_poll_on_link_down; + bool auto_reset; + bool no_poll_on_link_down; uint64_t mbuf_check; - int enable_lldp; + bool enable_lldp; }; struct iavf_security_ctx; diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c index a8abe0154c..27d6f73b94 100644 --- a/drivers/net/intel/iavf/iavf_ethdev.c +++ b/drivers/net/intel/iavf/iavf_ethdev.c @@ -3,6 +3,7 @@ */ #include <ctype.h> +#include <limits.h> #include <sys/queue.h> #include <stdalign.h> #include <stdio.h> @@ -13,6 +14,7 @@ #include <stdarg.h> #include <inttypes.h> #include <rte_byteorder.h> +#include <rte_kvargs.h> #include <rte_common.h> #include <rte_os_shim.h> @@ -2401,11 +2403,10 @@ static int parse_u16(__rte_unused const char *key, const char *value, void *args) { u16 *num = (u16 *)args; - u16 tmp; + uint64_t tmp; - errno = 0; - tmp = strtoull(value, NULL, 10); - if (errno || !tmp) { + /* zero is not accepted, it is the "unset" marker for this argument */ + if (rte_kvargs_to_uint(value, 1, UINT16_MAX, &tmp) < 0) { PMD_DRV_LOG(WARNING, "%s: \"%s\" is not a valid u16", key, value); return -1; @@ -2416,35 +2417,13 @@ parse_u16(__rte_unused const char *key, const char *value, void *args) return 0; } -static int -parse_bool(const char *key, const char *value, void *args) -{ - int *i = (int *)args; - char *end; - int num; - - num = strtoul(value, &end, 10); - - if (num != 0 && num != 1) { - PMD_DRV_LOG(WARNING, "invalid value:\"%s\" for key:\"%s\", " - "value must be 0 or 1", - value, key); - return -1; - } - - *i = num; - return 0; -} - static int iavf_parse_watchdog_period(__rte_unused const char *key, const char *value, void *args) { int *num = (int *)args; - int tmp; + uint64_t tmp; - errno = 0; - tmp = atoi(value); - if (tmp < 0) { + if (rte_kvargs_to_uint(value, 0, INT_MAX, &tmp) < 0) { PMD_DRV_LOG(WARNING, "%s: \"%s\" is not greater than or equal to zero", key, value); return -1; @@ -2515,8 +2494,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev) int ret; int watchdog_period = -1; - ad->devargs.auto_reset = 1; - ad->devargs.no_poll_on_link_down = 1; + ad->devargs.auto_reset = true; + ad->devargs.no_poll_on_link_down = true; if (!devargs) return 0; @@ -2550,8 +2529,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev) else ad->devargs.watchdog_period = watchdog_period; - ret = rte_kvargs_process(kvlist, IAVF_NO_POLL_ON_LINK_DOWN_ARG, - &parse_bool, &ad->devargs.no_poll_on_link_down); + ret = rte_kvargs_process_opt(kvlist, IAVF_NO_POLL_ON_LINK_DOWN_ARG, + rte_kvargs_handle_bool, &ad->devargs.no_poll_on_link_down); if (ret) goto bail; @@ -2568,15 +2547,15 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev) if (ret) goto bail; - ret = rte_kvargs_process(kvlist, IAVF_ENABLE_AUTO_RESET_ARG, - &parse_bool, &ad->devargs.auto_reset); + ret = rte_kvargs_process_opt(kvlist, IAVF_ENABLE_AUTO_RESET_ARG, + rte_kvargs_handle_bool, &ad->devargs.auto_reset); if (ret) goto bail; - if (ad->devargs.auto_reset != 0 && ad->devargs.no_poll_on_link_down == 0) { + if (ad->devargs.auto_reset && !ad->devargs.no_poll_on_link_down) { PMD_INIT_LOG(WARNING, "no-poll-on-link-down=0 is incompatible with auto_reset=1, ignoring"); - ad->devargs.no_poll_on_link_down = 1; + ad->devargs.no_poll_on_link_down = true; } if (rte_kvargs_count(kvlist, IAVF_ENABLE_PTYPE_LLDP_ARG) > 0) { @@ -2586,8 +2565,8 @@ static int iavf_parse_devargs(struct rte_eth_dev *dev) goto bail; } - ret = rte_kvargs_process(kvlist, IAVF_ENABLE_LLDP_ARG, - &parse_bool, &ad->devargs.enable_lldp); + ret = rte_kvargs_process_opt(kvlist, IAVF_ENABLE_LLDP_ARG, + rte_kvargs_handle_bool, &ad->devargs.enable_lldp); if (ret) goto bail; -- 2.53.0
