From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <muziding001@163.com>
Received: from m12-13.163.com (m12-13.163.com [220.181.12.13])
by dpdk.org (Postfix) with ESMTP id 36ABCE5D
for <dev@dpdk.org>; Fri, 29 Dec 2017 04:04:33 +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=Wy7Dz
Oi/hg5W9nzh47uIE6GPVAwaGafkHaFH6y4TPXQ=; b=hXwkWQP+6HZJwaBpedgDY
7+c+mnzun49P9lU7iDtTUAge6QpoaZMFRpnX4wjhpCBsnnhx5wjQAce9o+rRhvOo
aloHStH9UiPkCIjD+pdDSUIZPBDCMCEFuWJ9BgsBYfkNXiq9wg39fb0AAGAtj8pa
fqL8evwSj7zcFv1AcqVwkk=
Received: from NJYJYLINING71 (unknown [221.226.39.82])
by smtp9 (Coremail) with SMTP id DcCowACHJTq_sEVaM4YaAQ--.4049S2;
Fri, 29 Dec 2017 11:04:32 +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>
<002401d378b1$46e25960$d4a70c20$@163.com>
<20171228084345.li5xfnfkbuahp4ur@debian-xvivbkq.sh.intel.com>
In-Reply-To: <20171228084345.li5xfnfkbuahp4ur@debian-xvivbkq.sh.intel.com>
Date: Fri, 29 Dec 2017 11:04:33 +0800
Message-ID: <000701d38051$c1368b80$43a3a280$@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+oT4lS7InBsImmTxtopwGBP3IwAmDjNN0DFBEOxKEott0Q
Content-Language: zh-cn
X-CM-TRANSID: DcCowACHJTq_sEVaM4YaAQ--.4049S2
X-Coremail-Antispam: 1Uf129KBjvJXoWxtr45tr1fJw43JrykGw4kWFg_yoW7Wr15pr
4DGF43Zr15Jr17Ca1Sq3W5Wr9avas7Ar17Wr9rXw1rurn0qFnrCF1j9F1Ygr17XFW8GF4I
vF109FZIg3s8Z3DanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2
9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jwHUgUUUUU=
X-Originating-IP: [221.226.39.82]
X-CM-SenderInfo: ppx2xvhlqjiiqr6rljoofrz/xtbBSQC90laDwL75UgAAsm
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: Fri, 29 Dec 2017 03:04:35 -0000
HI Tiwei,
Thanks for the reply!
> Hi Ning,
>=20
> On Tue, Dec 19, 2017 at 06:08:10PM +0800, Ning Li wrote:
> > Hi Tiwei,
> >
> > > 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.
> >
> > 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.
> >
>=20
> Thanks for the detailed info!
>=20
> From my understanding, it's not necessary to set the MAC
> of the tap via virtio-user. Because the tap is a virtual
> interface which belongs to kernel, and it's connected to
> the virtual interface which is emulated by virtio-user.
>=20
> So actually they are different virtual interfaces. If an
> application wants the virtio-user, tap and any other NIC
> share the same MAC address, it just needs to set the MAC
> for each interface individually in the standard way. Any
> thoughts?
>=20
> Even if we really want to support specifying the MAC via
> virtio-user for the corresponding tap, the MAC should be
> passed via another device argument, e.g. tap_mac.
>=20
You are right. It is not necessary to set the MAC of tap0 via =
virtio-user.
Applications can use ioctl() to set the MAC of tap0 by themselves.
> Besides, I think you missed my other comments in my last
> mail. E.g. The patch isn't buildable.
Sorry for this. I will submit a buildable patch later.
Thanks,
Ning Li
>=20
> Thanks,
> Tiwei
>=20
> > Thanks,
> > Ning Li
> >
> > >
> > > >
> > > > 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 @@
> > >
> > > 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 =3D 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) =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
> */
> > >
> > > 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
> > > >
> > > >
> >
> >