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

[Tiwei Bie <tiwei.bie@intel.com>]

On Thu, Mar 29, 2018 at 08:50:37AM +0800, Tan, Jianfeng wrote:
> > -----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>
> 
> > >> ---
[...]
> > >> --- 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.
> 

Logically they're different virtual ports, so my first thought
is that they deserve different devargs and MACs. But after a
second thought, I also can't find a scenario where we would want
the tap and virtio device to have different MACs. So I'd like to
hear others' opinions. If that's the case, maybe we can take it.

Thanks

> 
> > 
> > >
> > 
> > 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