On 11/3/2023 7:38 AM, Chengwen Feng wrote:
> This driver could handles both key=value and only-key kvargs, so it
> should use rte_kvargs_process_opt() instead of rte_kvargs_process() to
> parse.
>
> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index b25a52655f..8b35de0a7a 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -2342,7 +2342,7 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
> kvlist = rte_kvargs_parse(params, valid_arguments);
> if (kvlist) {
> if (rte_kvargs_count(kvlist, ETH_TAP_IFACE_ARG) == 1) {
> - ret = rte_kvargs_process(kvlist,
> + ret = rte_kvargs_process_opt(kvlist,
> ETH_TAP_IFACE_ARG,
> &set_interface_name,
> tun_name);
> @@ -2546,28 +2546,28 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
> kvlist = rte_kvargs_parse(params, valid_arguments);
> if (kvlist) {
> if (rte_kvargs_count(kvlist, ETH_TAP_IFACE_ARG) == 1) {
> - ret = rte_kvargs_process(kvlist,
> - ETH_TAP_IFACE_ARG,
> - &set_interface_name,
> - tap_name);
> + ret = rte_kvargs_process_opt(kvlist,
> + ETH_TAP_IFACE_ARG,
> +
> &set_interface_name,
> + tap_name);
> if (ret == -1)
> goto leave;
> }
>
> if (rte_kvargs_count(kvlist, ETH_TAP_REMOTE_ARG) == 1) {
> - ret = rte_kvargs_process(kvlist,
> - ETH_TAP_REMOTE_ARG,
> - &set_remote_iface,
> - remote_iface);
> + ret = rte_kvargs_process_opt(kvlist,
> + ETH_TAP_REMOTE_ARG,
> + &set_remote_iface,
> + remote_iface);
>
As far as I can see, "remote" arg without value is not valid, but driver
handles this case as if "remote" arg is not provided at all. I think it
is reasonable to keep using 'rte_kvargs_process()', and fail if 'value'
is not provided.
> if (ret == -1)
> goto leave;
> }
>
> if (rte_kvargs_count(kvlist, ETH_TAP_MAC_ARG) == 1) {
> - ret = rte_kvargs_process(kvlist,
> - ETH_TAP_MAC_ARG,
> - &set_mac_type,
> - &user_mac);
> + ret = rte_kvargs_process_opt(kvlist,
> + ETH_TAP_MAC_ARG,
> + &set_mac_type,
> + &user_mac);
same here, 'rte_kvargs_process()' can be used, there is no point to give
"mac" keyword without value, that is same as not providing "mac" keyword
at all, so this can fail to notify user either provide a mac or remove
the argument.
I think current logic is to handle "value==null" case, otherwise this is
not a valid usecase.
> if (ret == -1)
> goto leave;
> }
