Re: [dpdk-dev] [PATCH v2 2/3] net/af_packet: move parse and validation of iface

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 11/20/2018 10:26 AM, Tiago Lam wrote:
> Instead of re-iterating through kvlist just to parse the
> ETH_AF_PACKET_IFACE_ARG argument in rte_pmd_init_internals(), we now use
> the already existing iteration in rte_eth_from_packet() to parse and
> validate the ETH_AF_PACKET_IFACE_ARG argument.

+1

> 
> This will be useful for a later commit, which needs to access the
> interface name to get the underlying configured MTU.
> 
> Signed-off-by: Tiago Lam <tiago.lam@intel.com>
> ---
> 
> v2: Fix checkpatches.sh and check-git-log.sh warnings.
> 
> ---
>  drivers/net/af_packet/rte_eth_af_packet.c | 77 ++++++++++++++++---------------
>  1 file changed, 41 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/af_packet/rte_eth_af_packet.c b/drivers/net/af_packet/rte_eth_af_packet.c
> index 264cfc0..8d749a2 100644
> --- a/drivers/net/af_packet/rte_eth_af_packet.c
> +++ b/drivers/net/af_packet/rte_eth_af_packet.c
> @@ -540,15 +540,12 @@ rte_pmd_init_internals(struct rte_vdev_device *dev,
>  		       unsigned int qdisc_bypass,
>                         struct pmd_internals **internals,
>                         struct rte_eth_dev **eth_dev,
> -                       struct rte_kvargs *kvlist)
> +					   const char *ifname)

Please align other parameter formatting.

<...>

> @@ -877,6 +856,32 @@ rte_eth_from_packet(struct rte_vdev_device *dev,
>  			}
>  			continue;
>  		}
> +		if (strstr(pair->key, ETH_AF_PACKET_IFACE_ARG) != NULL) {
> +			ifname = pair->value;
> +			if (strlen(ifname) == 0) {
> +				RTE_LOG(ERR, PMD,
> +					"%s: invalid iface value\n",
> +					name);
> +				return -1;
> +			}
> +
> +			continue;
> +		}

Indeed instead of accessing kvargs internal pair->values, this should call
rte_kvargs_process() but that is for another patch to fix all occurrences.

> +	}
> +
> +	if (ifname == NULL) {
> +		RTE_LOG(ERR, PMD,
> +			"%s: no interface specified for AF_PACKET ethdev\n",
> +			name);
> +		return -1;
> +	}
> +
> +	ifnamelen = strlen(ifname);

I am aware this is copy-paste but since update, perhaps we can use strnlen()
instead.