From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tiwei.bie@intel.com>
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
 by dpdk.org (Postfix) with ESMTP id CEBB0200
 for <dev@dpdk.org>; Mon, 18 Dec 2017 07:19:37 +0100 (CET)
X-Amp-Result: UNKNOWN
X-Amp-Original-Verdict: FILE UNKNOWN
X-Amp-File-Uploaded: False
Received: from fmsmga005.fm.intel.com ([10.253.24.32])
 by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 17 Dec 2017 22:19:36 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.45,420,1508828400"; d="scan'208";a="187653697"
Received: from deepin-15.sh.intel.com (HELO debian-xvivbkq) ([10.67.104.165])
 by fmsmga005.fm.intel.com with ESMTP; 17 Dec 2017 22:19:35 -0800
Date: Mon, 18 Dec 2017 14:19:07 +0800
From: Tiwei Bie <tiwei.bie@intel.com>
To: Ning Li <muziding001@163.com>
Cc: Yuanhan Liu <yliu@fridaylinux.org>,
 Maxime Coquelin <maxime.coquelin@redhat.com>, dev@dpdk.org
Message-ID: <20171218061907.fo3ausw5msfhic6q@debian-xvivbkq>
References: <1513251494-9980-1-git-send-email-muziding001@163.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1513251494-9980-1-git-send-email-muziding001@163.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Subject: Re: [dpdk-dev] [PATCH] net/virtio-user: specify MAC address for tap
 port
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 18 Dec 2017 06:19:38 -0000

Hi Ning,

On Thu, Dec 14, 2017 at 07:38:14PM +0800, Ning Li wrote:
> When use virtio_user as exception path, we need to specify a MAC
> address for the tap port.

Is this a fix? Did you meet any issue? If so, please describe
the issue and add a fixline.

> 
> Signed-off-by: Ning Li <muziding001@163.com>
> ---
>  drivers/net/virtio/virtio_user/vhost_kernel.c     |  3 ++-
>  drivers/net/virtio/virtio_user/vhost_kernel_tap.c |  8 ++++++++
>  drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 12 +++++++++++-
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index 68d28b1..dd24b6b 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -380,7 +380,8 @@ struct vhost_memory_kernel {
>  	else
>  		hdr_size = sizeof(struct virtio_net_hdr);
>  
> -	tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq);
> +	tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq,
> +			 (char *)dev->mac_addr);
>  	if (tapfd < 0) {
>  		PMD_DRV_LOG(ERR, "fail to open tap for vhost kernel");
>  		return -1;
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
> index 689a5cf..756fde2 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
> @@ -123,6 +123,14 @@

You forgot to add the mac param for vhost_kernel_open_tap().
The patch isn't buildable on my machine.

>  		PMD_DRV_LOG(ERR, "TUNSETOFFLOAD ioctl() failed: %s",
>  			   strerror(errno));
>  
> +	memset(&ifr, 0, sizeof(ifr));
> +	ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER;

ARPHRD_ETHER? Please explain.

> +	memcpy(ifr.ifr_hwaddr.sa_data, mac, ETH_ALEN);

You can use ETHER_ADDR_LEN.
There is no need to define ETH_ALEN.

> +	if (ioctl(tapfd, SIOCSIFHWADDR, (void *)&ifr) == -1) {
> +		PMD_DRV_LOG(ERR, "SIOCSIFHWADDR failed: %s", strerror(errno));
> +		goto error;
> +	}
> +
>  	if (!(*p_ifname))
>  		*p_ifname = strdup(ifr.ifr_name);
>  
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
> index eae340c..f59b1ac 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
> @@ -64,4 +64,14 @@
>  /* Constants */
>  #define PATH_NET_TUN	"/dev/net/tun"
>  
> -int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq);
> +/* Socket configuration controls. */
> +#define SIOCSIFHWADDR   0x8924          /* set hardware address */

There is no need to define this.

> +
> +/* ARP protocol HARDWARE identifiers. */
> +#define ARPHRD_ETHER    1               /* Ethernet             */
> +
> +/* Length of Ethernet address. */
> +#define ETH_ALEN  6
> +

There is no need to define this.

Thanks,
Tiwei

> +int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
> +			 const char *mac);
> -- 
> 1.8.3.1
> 
>