From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v2 3/3] net/af_packet: get 'framesz' from the
	iface's MTU
Date: Tue, 27 Nov 2018 17:43:00 +0000
Message-ID: <59e1bdba-331b-e337-56e7-dc4f52057d56@intel.com>
References: <1542707697-175836-1-git-send-email-tiago.lam@intel.com>
 <1542709592-215007-1-git-send-email-tiago.lam@intel.com>
 <1542709592-215007-3-git-send-email-tiago.lam@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
To: Tiago Lam <tiago.lam@intel.com>, dev@dpdk.org, linville@tuxdriver.com
Return-path: <dev-bounces@dpdk.org>
Received: from mga12.intel.com (mga12.intel.com [192.55.52.136])
 by dpdk.org (Postfix) with ESMTP id DE8FE1B46D
 for <dev@dpdk.org>; Tue, 27 Nov 2018 18:43:14 +0100 (CET)
In-Reply-To: <1542709592-215007-3-git-send-email-tiago.lam@intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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.