Re: [PATCH] net/virtio-user: specify the MAC of the tap

["Tan, Jianfeng" <jianfeng.tan@intel.com>]

> -----Original Message-----
> From: Seán Harte [mailto:seanbh@gmail.com]
> Sent: Thursday, March 29, 2018 12:55 AM
> To: Ning Li; Yuanhan Liu; Maxime Coquelin; Bie, Tiwei
> Cc: dev@dpdk.org; Tan, Jianfeng
> Subject: Re: [dpdk-dev] [PATCH] net/virtio-user: specify the MAC of the tap
> 
> On 29/12/2017 09:44, tiwei.bie at intel.com (Tiwei Bie) wrote:
> > Hi Ning,
> >
> > On Fri, Dec 29, 2017 at 11:38:42AM +0800, Ning Li wrote:
> >> When using virtio-user with vhost-kernel to exchange
> >> packet with kernel networking stack, application can
> >> set the MAC of the tap interface via parameter.
> >>
> >> Signed-off-by: Ning Li <muziding001 at 163.com>
> 
> Reviewed-by: Seán Harte <seanbh@gmail.com>
> Tested-by: Seán Harte <seanbh@gmail.com>

Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>

> >> ---
> >
> > Thanks for the new version.
> >
> > Just FYI, when sending a new version, you also need to
> > add the version number, e.g. using -v2 when generating
> > the patch. You can find more details in the "Contribute
> > by sending patches" section in below link:
> >
> > http://dpdk.org/dev
> >
> >>   drivers/net/virtio/virtio_user/vhost_kernel.c     |  3 ++-
> >>   drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 14 +++++++++++++-
> >>   drivers/net/virtio/virtio_user/vhost_kernel_tap.h |  3 ++-
> >>   3 files changed, 17 insertions(+), 3 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);
> >
> > I think it's better to add a new device argument for
> > virtio-user to specify the MAC for the corresponding
> > tap. But I don't have a very strong opinion on this
> > for now. So I'd like to hear others' opinions.
> 
> No harm if it was a seperate argument, although I can't think of a
> scenario where you care about the MAC address and would want the tap and
> virtio devices to have differnet MAC addresses.

I would also prefer to keep it as simple as possible before we find a real use case for different mac addr.


> 
> >
> 
> snip...
> 
> >
> 
> I'm not sure if this patch is still under consideration, but it looks
> good to me, and works.

I think yes. Maxime, would you like to consider merging this patch?

Thanks,
Jianfeng