get_integer_arg() checks only errno, so "queues=foo" was silently taken as zero, and since the store is skipped on failure but zero is returned, the argument was accepted while leaving the default in place. It also uses base 0, so a leading zero selects octal.
link_speed_handler() checks neither errno nor the end pointer before validating the result against the speed capabilities. The boolean arguments become bool and use rte_kvargs_handle_bool(): server, cq, packed_vq, vectorized, mrg_rxbuf and in_order for virtio_user, and vectorized for virtio. They are passed by value into virtio_user_dev_init(), whose parameters stay int. vectorized_check_handler() treated anything other than "1" as false, so "vectorized=y" silently disabled it and a typo did the same. It is now a boolean like the rest: the usual spellings work, a bare key enables the option, and anything else is rejected instead of being read as false. Signed-off-by: Stephen Hemminger <[email protected]> --- drivers/net/virtio/virtio_ethdev.c | 36 ++++++--------- drivers/net/virtio/virtio_user_ethdev.c | 59 +++++++++---------------- 2 files changed, 34 insertions(+), 61 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 9fd746b80f..83ea6745ce 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -47,7 +47,7 @@ static int virtio_dev_allmulticast_disable(struct rte_eth_dev *dev); static uint32_t virtio_dev_speed_capa_get(uint32_t speed); static int virtio_dev_devargs_parse(struct rte_devargs *devargs, uint32_t *speed, - int *vectorized); + bool *vectorized); static int virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info); static int virtio_dev_link_update(struct rte_eth_dev *dev, @@ -2039,7 +2039,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) { struct virtio_hw *hw = eth_dev->data->dev_private; uint32_t speed = RTE_ETH_SPEED_NUM_UNKNOWN; - int vectorized = 0; + bool vectorized = false; int ret; if (sizeof(struct virtio_net_hdr_hash_report) > RTE_PKTMBUF_HEADROOM) { @@ -2138,20 +2138,6 @@ virtio_dev_speed_capa_get(uint32_t speed) } } -static int vectorized_check_handler(__rte_unused const char *key, - const char *value, void *ret_val) -{ - if (value == NULL || ret_val == NULL) - return -EINVAL; - - if (strcmp(value, "1") == 0) - *(int *)ret_val = 1; - else - *(int *)ret_val = 0; - - return 0; -} - #define VIRTIO_ARG_SPEED "speed" #define VIRTIO_ARG_VECTORIZED "vectorized" @@ -2159,21 +2145,25 @@ static int link_speed_handler(const char *key __rte_unused, const char *value, void *ret_val) { - uint32_t val; - if (!value || !ret_val) + uint64_t val; + + if (ret_val == NULL) + return -EINVAL; + + if (rte_kvargs_to_uint(value, 0, UINT32_MAX, &val) < 0) return -EINVAL; - val = strtoul(value, NULL, 0); + /* validate input */ if (virtio_dev_speed_capa_get(val) == 0) return -EINVAL; - *(uint32_t *)ret_val = val; + *(uint32_t *)ret_val = val; return 0; } static int -virtio_dev_devargs_parse(struct rte_devargs *devargs, uint32_t *speed, int *vectorized) +virtio_dev_devargs_parse(struct rte_devargs *devargs, uint32_t *speed, bool *vectorized) { struct rte_kvargs *kvlist; int ret = 0; @@ -2200,9 +2190,9 @@ virtio_dev_devargs_parse(struct rte_devargs *devargs, uint32_t *speed, int *vect if (vectorized && rte_kvargs_count(kvlist, VIRTIO_ARG_VECTORIZED) == 1) { - ret = rte_kvargs_process(kvlist, + ret = rte_kvargs_process_opt(kvlist, VIRTIO_ARG_VECTORIZED, - vectorized_check_handler, vectorized); + rte_kvargs_handle_bool, vectorized); if (ret < 0) { PMD_INIT_LOG(ERR, "Failed to parse %s", VIRTIO_ARG_VECTORIZED); diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 747dddeb2e..4925924211 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -387,23 +387,6 @@ get_string_arg(const char *key __rte_unused, return 0; } -static int -get_integer_arg(const char *key __rte_unused, - const char *value, void *extra_args) -{ - uint64_t integer = 0; - if (!value || !extra_args) - return -EINVAL; - errno = 0; - integer = strtoull(value, NULL, 0); - /* extra_args keeps default value, it should be replaced - * only in case of successful parsing of the 'value' arg - */ - if (errno == 0) - *(uint64_t *)extra_args = integer; - return -errno; -} - static uint32_t vdpa_dynamic_major_num(void) { @@ -508,13 +491,13 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) struct virtio_user_dev *dev; enum virtio_user_backend_type backend_type = VIRTIO_USER_BACKEND_UNKNOWN; uint64_t queues = VIRTIO_USER_DEF_Q_NUM; - uint64_t cq = VIRTIO_USER_DEF_CQ_EN; + bool cq = VIRTIO_USER_DEF_CQ_EN; uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ; - uint64_t server_mode = VIRTIO_USER_DEF_SERVER_MODE; - uint64_t mrg_rxbuf = 1; - uint64_t in_order = 1; - uint64_t packed_vq = 0; - uint64_t vectorized = 0; + bool server_mode = VIRTIO_USER_DEF_SERVER_MODE; + bool mrg_rxbuf = true; + bool in_order = true; + bool packed_vq = false; + bool vectorized = false; char *path = NULL; char *ifname = NULL; char *mac_addr = NULL; @@ -602,7 +585,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE) == 1) { if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE, - &get_integer_arg, &queue_size) < 0) { + rte_kvargs_handle_u64, &queue_size) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_QUEUE_SIZE); goto end; @@ -611,7 +594,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUES_NUM) == 1) { if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUES_NUM, - &get_integer_arg, &queues) < 0) { + rte_kvargs_handle_u64, &queues) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_QUEUES_NUM); goto end; @@ -619,8 +602,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_SERVER_MODE) == 1) { - if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_SERVER_MODE, - &get_integer_arg, &server_mode) < 0) { + if (rte_kvargs_process_opt(kvlist, VIRTIO_USER_ARG_SERVER_MODE, + rte_kvargs_handle_bool, &server_mode) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_SERVER_MODE); goto end; @@ -628,8 +611,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) { - if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, - &get_integer_arg, &cq) < 0) { + if (rte_kvargs_process_opt(kvlist, VIRTIO_USER_ARG_CQ_NUM, + rte_kvargs_handle_bool, &cq) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_CQ_NUM); goto end; @@ -637,8 +620,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PACKED_VQ) == 1) { - if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PACKED_VQ, - &get_integer_arg, &packed_vq) < 0) { + if (rte_kvargs_process_opt(kvlist, VIRTIO_USER_ARG_PACKED_VQ, + rte_kvargs_handle_bool, &packed_vq) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_PACKED_VQ); goto end; @@ -646,8 +629,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_VECTORIZED) == 1) { - if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_VECTORIZED, - &get_integer_arg, &vectorized) < 0) { + if (rte_kvargs_process_opt(kvlist, VIRTIO_USER_ARG_VECTORIZED, + rte_kvargs_handle_bool, &vectorized) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_VECTORIZED); goto end; @@ -655,8 +638,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_MRG_RXBUF) == 1) { - if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_MRG_RXBUF, - &get_integer_arg, &mrg_rxbuf) < 0) { + if (rte_kvargs_process_opt(kvlist, VIRTIO_USER_ARG_MRG_RXBUF, + rte_kvargs_handle_bool, &mrg_rxbuf) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_MRG_RXBUF); goto end; @@ -664,8 +647,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_IN_ORDER) == 1) { - if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_IN_ORDER, - &get_integer_arg, &in_order) < 0) { + if (rte_kvargs_process_opt(kvlist, VIRTIO_USER_ARG_IN_ORDER, + rte_kvargs_handle_bool, &in_order) < 0) { PMD_INIT_LOG(ERR, "error to parse %s", VIRTIO_USER_ARG_IN_ORDER); goto end; @@ -816,7 +799,7 @@ RTE_PMD_REGISTER_ALIAS(net_virtio_user, virtio_user); RTE_PMD_REGISTER_PARAM_STRING(net_virtio_user, "path=<path> " "mac=<mac addr> " - "cq=<int> " + "cq=<0|1> " "queue_size=<int> " "queues=<int> " "iface=<string> " -- 2.53.0
