Re: [PATCH v2 3/3] net/af_packet: get 'framesz' from the iface's MTU

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

On 11/20/2018 10:26 AM, Tiago Lam wrote:
> Use the underlying MTU to calculate the framsize to be used for the mmap
> RINGs. This is to make it more flexible on deployments with different
> MTU requirements, instead of using a pre-defined value of 2048B.

This behavior change should be documented in af_packet documentation which is
missing unfortunately.
Would you able to introduce the initial/basic af_packet doc to at least to
document device argument? If not please let me know, I can work on it.

> 
> If a 'framsz' option is provided, that value is used instead and the MTU
> of the underlying interface is ignored.
> 
> Signed-off-by: Tiago Lam <tiago.lam@intel.com>
> ---
> 
> v2: Fix checkpatches.sh and check-git-log.sh warnings.

<...>

> @@ -877,17 +877,40 @@ rte_eth_from_packet(struct rte_vdev_device *dev,
>  	}
>  
>  	ifnamelen = strlen(ifname);
> -	if (ifnamelen >= sizeof(ifr.ifr_name)) {
> +	if (ifnamelen < sizeof(ifr.ifr_name)) {
> +		memcpy(ifr.ifr_name, ifname, ifnamelen);
> +		ifr.ifr_name[ifnamelen] = '\0';

This can be replaces with strlcpy().

> +	} else {
>  		RTE_LOG(ERR, PMD,
>  			"%s: I/F name too long (%s)\n",
>  			name, ifname);
>  		return -1;
>  	}
>  
> +	/*
> +	 * Base framesize on the MTU of the underlying interface, if no
> +	 * 'framesz' option is given
> +	 */
> +	if (!framesize) {
> +		if (ioctl(*sockfd, SIOCGIFMTU, &ifr) == -1) {
> +			RTE_LOG(ERR, PMD,
> +				"%s: ioctl failed (SIOCGIFMTU)",
> +				name);
> +			framesize = DFLT_FRAME_SIZE;

It may be good to add a log to say default frame size will be used, and perhaps
print out the default value.