Skip to contentSkip to conversation

virtio-net: Allow setting the MAC address via set_config

PATCH In review Alex Williamson 5 participants 12 messages Jan 13, 2009 – Jan 18, 2009 KVM

lore
Share & tools
lore
#0 lore raw

PATCH 1/5 virtio-net: Allow setting the MAC address via set_config

Rename get_config for simplicity

---

 qemu/hw/virtio-net.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)
qemu/hw/virtio-net.c +16 -2
index 1f9dc16..e9b3d46 100644
@@ -42,7 +42,7 @@ static VirtIONet *to_virtio_net(VirtIODevice *vdev)
     return (VirtIONet *)vdev;
 }
 
-static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
+static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_config netcfg;
@@ -51,6 +51,19 @@ static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
     memcpy(config, &netcfg, sizeof(netcfg));
 }
 
+static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+    VirtIONet *n = to_virtio_net(vdev);
+    struct virtio_net_config netcfg;
+
+    memcpy(&netcfg, config, sizeof(netcfg));
+
+    if (memcmp(netcfg.mac, n->mac, 6)) {
+        memcpy(n->mac, netcfg.mac, 6);
+        qemu_format_nic_info_str(n->vc, n->mac);
+    }
+}
+
 static uint32_t virtio_net_get_features(VirtIODevice *vdev)
 {
     uint32_t features = (1 << VIRTIO_NET_F_MAC);
@@ -405,7 +418,8 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     if (!n)
         return NULL;
 
-    n->vdev.get_config = virtio_net_update_config;
+    n->vdev.get_config = virtio_net_get_config;
+    n->vdev.set_config = virtio_net_set_config;
     n->vdev.get_features = virtio_net_get_features;
     n->vdev.set_features = virtio_net_set_features;
     n->rx_vq = virtio_add_queue(&n->vdev, 256, virtio_net_handle_rx);
#1 lore raw
Quoted 29 lines · Alex Williamson

Rename get_config for simplicity

---

 qemu/hw/virtio-net.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)
qemu/hw/virtio-net.c +5 -1
index 1f9dc16..e9b3d46 100644
@@ -42,7 +42,7 @@ static VirtIONet *to_virtio_net(VirtIODevice *vdev)
     return (VirtIONet *)vdev;
 }
 
-static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
+static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
 {
     VirtIONet *n = to_virtio_net(vdev);
     struct virtio_net_config netcfg;
@@ -51,6 +51,19 @@ static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config)
     memcpy(config, &netcfg, sizeof(netcfg));
 }
 
+static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+    VirtIONet *n = to_virtio_net(vdev);
+    struct virtio_net_config netcfg;

qemu/hw/virtio-net.c static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config) · line 67

Quoted 10 lines
qemu/hw/virtio-net.c +9 -0
+
+    memcpy(&netcfg, config, sizeof(netcfg));
+
+    if (memcmp(netcfg.mac, n->mac, 6)) {
+        memcpy(n->mac, netcfg.mac, 6);
+        qemu_format_nic_info_str(n->vc, n->mac);
+    }
+}
+
 

What if the guest will chose the host's mac?
Thinking about it, I don't think we should test that.
A concerned host mgmt app can add ebtables roles for such a case.

Maybe we can optionally allow/deny it?

#2 lore raw
Quoted 20 lines · Dor Laor
Quoted 4 lines · Alex Williamson
qemu/hw/virtio-net.c +3 -0
+static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
+{
+    VirtIONet *n = to_virtio_net(vdev);

qemu/hw/virtio-net.c static void virtio_net_update_config(VirtIODevice *vdev, uint8_t *config) · line 66

Quoted 10 lines
qemu/hw/virtio-net.c +10 -0
+    struct virtio_net_config netcfg;
+
+    memcpy(&netcfg, config, sizeof(netcfg));
+
+    if (memcmp(netcfg.mac, n->mac, 6)) {
+        memcpy(n->mac, netcfg.mac, 6);
+        qemu_format_nic_info_str(n->vc, n->mac);
+    }
+}
+

What if the guest will chose the host's mac?
Thinking about it, I don't think we should test that.
A concerned host mgmt app can add ebtables roles for such a case.

Quoted 1 line

Maybe we can optionally allow/deny it?

What's the topology you're thinking of that the virtio-net MAC is also the host MAC? I typically use a bridge with a tap device, so the virtio-net MAC is isolated from the host. Thanks,

Alex

Signature · 1 line
Alex Williamson                             HP Open Source & Linux Org.
#3 lore raw
Quoted 9 lines · Alex Williamson
Quoted 5 lines

What if the guest will chose the host's mac?
Thinking about it, I don't think we should test that.
A concerned host mgmt app can add ebtables roles for such a case.

Maybe we can optionally allow/deny it?

What's the topology you're thinking of that the virtio-net MAC is also the host MAC? I typically use a bridge with a tap device, so the virtio-net MAC is isolated from the host. Thanks,

For example you might forward IPX packets to the guest and IP/ARP to the host, using an ebtables rule to distinguish them. From the outside, it would look equivalent to a single host processing both IPX and IP.

-- Jamie

#4 lore raw
Quoted 13 lines · Jamie Lokier
Quoted 10 lines · Alex Williamson
Quoted 6 lines

What if the guest will chose the host's mac?
Thinking about it, I don't think we should test that.
A concerned host mgmt app can add ebtables roles for such a case.

Maybe we can optionally allow/deny it?

What's the topology you're thinking of that the virtio-net MAC is also the host MAC? I typically use a bridge with a tap device, so the virtio-net MAC is isolated from the host. Thanks,

Quoted 8 lines

For example you might forward IPX packets to the guest and IP/ARP to the host, using an ebtables rule to distinguish them. From the outside, it would look equivalent to a single host processing both IPX and IP.

-- Jamie

That's a nice common scenario ;)
What I meant is that if we allow the guest to change his mac address, it can deliberately change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

Thanks,
Dor

#5 lore raw
In reply to Jamie Lokier
Quoted 13 lines · Jamie Lokier
Quoted 10 lines · Alex Williamson
Quoted 6 lines

What if the guest will chose the host's mac?
Thinking about it, I don't think we should test that.
A concerned host mgmt app can add ebtables roles for such a case.

Maybe we can optionally allow/deny it?

What's the topology you're thinking of that the virtio-net MAC is also the host MAC? I typically use a bridge with a tap device, so the virtio-net MAC is isolated from the host. Thanks,

Quoted 8 lines

For example you might forward IPX packets to the guest and IP/ARP to the host, using an ebtables rule to distinguish them. From the outside, it would look equivalent to a single host processing both IPX and IP.

-- Jamie

That's a nice common scenario ;)
What I meant is that if we allow the guest to change his mac address, it can deliberately change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

Thanks,
Dor

#6 lore raw
Quoted 5 lines

What I meant is that if we allow the guest to change his mac address, it can deliberately
change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

This doesn't seem any different to real hardware that allows you to change the MAC address.

Paul

#7 lore raw
Quoted 8 lines · Paul Brook
Quoted 5 lines

What I meant is that if we allow the guest to change his mac address, it can deliberately
change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

This doesn't seem any different to real hardware that allows you to change the MAC address.

Indeed I have used that on several occasions to workaround pointless firewalls and home networking restrictions.

People doing MAC-level hot-failover in high-availability environments do it too.

-- Jamie

#8 lore raw
In reply to Dor Laor
Quoted 5 lines · Dor Laor

What I meant is that if we allow the guest to change his mac address, it can deliberately
change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

Although it can create network problems, sometimes it is also wanted.

I think if you want to restrict the guests's ability to break the network by changing its MAC, it would be appropriate to have an option to completely lock down the MAC so the guest can't change its MAC at all.

-- Jamie

#9 lore raw
Quoted 7 lines · Jamie Lokier
Quoted 4 lines · Dor Laor

What I meant is that if we allow the guest to change his mac address, it can deliberately change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

Quoted 6 lines

Although it can create network problems, sometimes it is also wanted.

I think if you want to restrict the guests's ability to break the network by changing its MAC, it would be appropriate to have an option to completely lock down the MAC so the guest can't change its MAC at all.

I don't think locking down the MAC is very useful; the guest can still fake its IP address.

If the admin wants to lock down the guest, they should use netfilter (and live with the performance hit).

Signature · 1 line
error compiling committee.c: too many arguments to function
#10 lore raw
In reply to Jamie Lokier
Quoted 7 lines · Jamie Lokier
Quoted 4 lines · Dor Laor

What I meant is that if we allow the guest to change his mac address, it can deliberately change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

Quoted 7 lines

Although it can create network problems, sometimes it is also wanted.

I think if you want to restrict the guests's ability to break the network by changing its MAC, it would be appropriate to have an option to completely lock down the MAC so the guest can't change its MAC at all.

That's what I was shooting to.
One example this can be helpful is when kvm is used to run virtual servers in a computing farm like Amazon. You wouldn't like a VM owner to mess your network.

#11 lore raw
Quoted 10 lines · Dor Laor
Quoted 9 lines · Jamie Lokier
Quoted 4 lines · Dor Laor

What I meant is that if we allow the guest to change his mac address, it can deliberately change it to other hosts/guests mac and thus create networking problems. Although guest can always mangle packets, maybe it worth enforcing these macs for the guest.

Although it can create network problems, sometimes it is also wanted.

Quoted 8 lines
Quoted 5 lines

I think if you want to restrict the guests's ability to break the network by changing its MAC, it would be appropriate to have an option to completely lock down the MAC so the guest can't change its MAC at all.

That's what I was shooting to.
One example this can be helpful is when kvm is used to run virtual servers in a computing farm like Amazon. You wouldn't like a VM owner to mess your network.

Restricting the MAC address won't help. The guest can still forge the link layer address and/or the IP layer addresses.

This needs to be addressed by netfilter.

Signature · 1 line
error compiling committee.c: too many arguments to function