From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <muziding001@163.com>
Received: from m12-11.163.com (m12-11.163.com [220.181.12.11])
 by dpdk.org (Postfix) with ESMTP id B46311B00C
 for <dev@dpdk.org>; Tue, 19 Dec 2017 10:27:17 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com;
 s=s110527; h=From:Subject:Date:Message-ID:MIME-Version; bh=EiqgO
 XE/lgUN32AXBjeOCmgkOiDBWtHkllo2LlYYqxM=; b=d9SMQmwnbGrvJJK1Tz8+P
 1v/2+2hwsegKThtzB3B5H0BWyBiBHHuo6lFXWMxM0UcsSnXJqi74TPua7NWClI3g
 zTZLptP+xnyfOWHYy1OjtgF/fhZg6M5a09QnMc33/Rf9xT5lbyyFZA8ZutGlUqBZ
 /qTdqy8ize+oEYxA0aRpPY=
Received: from NJYJYLINING71 (unknown [221.226.39.82])
 by smtp7 (Coremail) with SMTP id C8CowAAXzhpz2zhapUKaCg--.54213S2;
 Tue, 19 Dec 2017 17:27:16 +0800 (CST)
From: "Ning Li" <muziding001@163.com>
To: "'Tiwei Bie'" <tiwei.bie@intel.com>
Cc: "'Yuanhan Liu'" <yliu@fridaylinux.org>,
 "'Maxime Coquelin'" <maxime.coquelin@redhat.com>, <dev@dpdk.org>
References: <1513251494-9980-1-git-send-email-muziding001@163.com>
 <20171218061907.fo3ausw5msfhic6q@debian-xvivbkq>
In-Reply-To: <20171218061907.fo3ausw5msfhic6q@debian-xvivbkq>
Date: Tue, 19 Dec 2017 17:27:17 +0800
Message-ID: <002001d378ab$90968280$b1c38780$@163.com>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="utf-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Microsoft Outlook 16.0
Thread-Index: AQLQGv2xlZ+oT4lS7InBsImmTxtopwGBP3IwoUUWn9A=
Content-Language: zh-cn
X-CM-TRANSID: C8CowAAXzhpz2zhapUKaCg--.54213S2
X-Coremail-Antispam: 1Uf129KBjvJXoWxJw1DXFW8GryrXF47Gr4rXwb_yoWrGF1Dpw
 4DGFsxZr13tr97Ca1Sqa15ur98Z3Z3Ar17Kr93Xw1rurn0qFnrCFyUu3WYgr17AFWUGF4I
 yF409FnIg3s8Z3DanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2
 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jzg4DUUUUU=
X-Originating-IP: [221.226.39.82]
X-CM-SenderInfo: ppx2xvhlqjiiqr6rljoofrz/1tbiNxSz0lWBV-FhrwAAs+
X-Mailman-Approved-At: Tue, 19 Dec 2017 15:39:47 +0100
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: Tue, 19 Dec 2017 09:27:18 -0000

Hi Tiwei,

> Hi Ning,
>=20
> 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.
>=20
> Is this a fix? Did you meet any issue? If so, please describe
> the issue and add a fixline.

Specify the MAC address for tap0 may be a requirement for some =
applications.

As described in doc/guides/howto/virtio_user_as_exceptional_path.rst, =
when
virtio-user with vhost-kernel is used to exchange packet with kernel =
networking
stack, instead of KNI, application need to set the physical NIC in =
promiscuous
mode, otherwise, the packet send to tap0 will be discarded by physical =
NIC. So,
this will be a probleam, when some applications not set NIC in =
promiscuous mode.
This problem will be easy to solve, If application can specify the MAC =
address
of the NIC for tap0.

Thanks,
Ning Li

>=20
> >
> > 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 =3D sizeof(struct virtio_net_hdr);
> >
> > -	tapfd =3D vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq);
> > +	tapfd =3D 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 @@
>=20
> You forgot to add the mac param for vhost_kernel_open_tap().
> The patch isn't buildable on my machine.
>=20
> >  		PMD_DRV_LOG(ERR, "TUNSETOFFLOAD ioctl() failed: %s",
> >  			   strerror(errno));
> >
> > +	memset(&ifr, 0, sizeof(ifr));
> > +	ifr.ifr_hwaddr.sa_family =3D ARPHRD_ETHER;
>=20
> ARPHRD_ETHER? Please explain.
>=20
> > +	memcpy(ifr.ifr_hwaddr.sa_data, mac, ETH_ALEN);
>=20
> You can use ETHER_ADDR_LEN.
> There is no need to define ETH_ALEN.
>=20
> > +	if (ioctl(tapfd, SIOCSIFHWADDR, (void *)&ifr) =3D=3D -1) {
> > +		PMD_DRV_LOG(ERR, "SIOCSIFHWADDR failed: %s", strerror(errno));
> > +		goto error;
> > +	}
> > +
> >  	if (!(*p_ifname))
> >  		*p_ifname =3D 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 */
>=20
> There is no need to define this.
>=20
> > +
> > +/* ARP protocol HARDWARE identifiers. */
> > +#define ARPHRD_ETHER    1               /* Ethernet
> */
> > +
> > +/* Length of Ethernet address. */
> > +#define ETH_ALEN  6
> > +
>=20
> There is no need to define this.
>=20
> Thanks,
> Tiwei
>=20
> > +int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int =
req_mq,
> > +			 const char *mac);
> > --
> > 1.8.3.1
> >
> >