On Thu, Mar 29, 2018 at 08:50:37AM +0800, Tan, Jianfeng wrote:

quoted

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

quoted

Hi Ning,

On Fri, Dec 29, 2017 at 11:38:42AM +0800, Ning Li wrote:

quoted

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 <redacted>
Tested-by: Seán Harte <redacted>

Reviewed-by: Jianfeng Tan <redacted>

quoted

quoted

quoted

---

[...]

quoted

quoted

quoted

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

quoted

quoted

snip...

quoted

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