From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ning Li" Subject: Re: [PATCH] net/virtio-user: specify MAC address for tap port Date: Tue, 19 Dec 2017 18:08:10 +0800 Message-ID: <002401d378b1$46e25960$d4a70c20$@163.com> References: <1513251494-9980-1-git-send-email-muziding001@163.com> <20171218061907.fo3ausw5msfhic6q@debian-xvivbkq> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Cc: "'Yuanhan Liu'" , "'Maxime Coquelin'" , To: "'Tiwei Bie'" Return-path: Received: from m12-17.163.com (m12-17.163.com [220.181.12.17]) by dpdk.org (Postfix) with ESMTP id 097E91B161 for ; Tue, 19 Dec 2017 11:08:11 +0100 (CET) In-Reply-To: <20171218061907.fo3ausw5msfhic6q@debian-xvivbkq> Content-Language: zh-cn List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > > --- > > 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 > > > >