| Message ID | 1352718766-5663-1-git-send-email-lma@suse.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 12, 2012 at 07:12:46PM +0800, Lin Ma wrote: > QEMU doesn't check if there are mac collisions when adding nics. > It causes mac address collisions in guest if adding the nics which > include existing physical address. > This patch fixes the issue. I understand the issue, but are there not use cases where it is reasonable to have multiple NICs with the same MAC address ? To me this kind of policy enforcement belongs at a higher level in the mgmt stack. Daniel
Il 12/11/2012 12:18, Daniel P. Berrange ha scritto: >> > QEMU doesn't check if there are mac collisions when adding nics. >> > It causes mac address collisions in guest if adding the nics which >> > include existing physical address. >> > This patch fixes the issue. > I understand the issue, but are there not use cases where it is > reasonable to have multiple NICs with the same MAC address ? To > me this kind of policy enforcement belongs at a higher level in > the mgmt stack. I agree. Paolo
>>> Paolo Bonzini <pbonzini@redhat.com> 11/12/12 7:27 PM >>> Il 12/11/2012 12:18, Daniel P. Berrange ha scritto: >> > QEMU doesn't check if there are mac collisions when adding nics. >> > It causes mac address collisions in guest if adding the nics which >> > include existing physical address. >> > This patch fixes the issue. > I understand the issue, but are there not use cases where it is > reasonable to have multiple NICs with the same MAC address ? To > me this kind of policy enforcement belongs at a higher level in > the mgmt stack. >>>I agree. Yes, Users won't intentionally add multiple NICs with the same MAC address. But what if there is typos in command-line or upper layer applications pass a conflicting mac option to qemu? We can't fully trust other applications to do this. So qemu shouldn't depend on other applications for the verifying mac. No matter upper layer applications verify mac or not, qemu should check the mac itself again to ensure the correctness. Lin
On Mon, Nov 12, 2012 at 12:49 PM, Lin Ma <lma@suse.com> wrote: >>>> Paolo Bonzini <pbonzini@redhat.com> 11/12/12 7:27 PM >>> > > Il 12/11/2012 12:18, Daniel P. Berrange ha scritto: >>> > QEMU doesn't check if there are mac collisions when adding nics. >>> > It causes mac address collisions in guest if adding the nics which >>> > include existing physical address. >>> > This patch fixes the issue. >> I understand the issue, but are there not use cases where it is >> reasonable to have multiple NICs with the same MAC address ? To >> me this kind of policy enforcement belongs at a higher level in >> the mgmt stack. > >>>>I agree. > > Yes, Users won't intentionally add multiple NICs with the same MAC address. > But what if there is typos in command-line or upper layer applications pass > a conflicting mac option to qemu? > We can't fully trust other applications to do this. So qemu shouldn't depend > on other applications for the verifying mac. > No matter upper layer applications verify mac or not, qemu should check the > mac itself again to ensure the correctness. It's perfectly okay to use the same MAC address on multiple cards from a hardware emulation perspective (which is the scope of QEMU). Higher level tools can perform this check if they want this policy. Stefan
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 8aca0d4..23318af 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -761,6 +761,15 @@ static void set_mac(Object *obj, Visitor *v, void *opaque, } mac->a[i] = strtol(str+pos, &p, 16); } + + if (qemu_check_macaddr_uniqueness(mac->a) < 0) { + goto collision; + } + g_free(str); + return; + +collision: + error_set_from_qdev_prop_error(errp, -EEXIST, dev, prop, str); g_free(str); return; diff --git a/net.c b/net.c index e8ae13e..3077405 100644 --- a/net.c +++ b/net.c @@ -535,6 +535,29 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models, return -1; } +int qemu_check_macaddr_uniqueness(uint8_t *macaddr) +{ + NetClientState *nc; + uint8_t existing_mac[6]; + char *mac_str; + int i; + + QTAILQ_FOREACH(nc, &net_clients, next) { + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) { + mac_str = strstr(nc->info_str, "macaddr=") + strlen("macaddr="); + for (i = 0; i < 6; i++) { + existing_mac[i] = strtol(mac_str, (char **)&mac_str, 16); + mac_str++; + } + if (memcmp(macaddr, &existing_mac, sizeof(existing_mac)) == 0) { + return -1; + } + } + } + + return 0; +} + static int net_init_nic(const NetClientOptions *opts, const char *name, NetClientState *peer) { @@ -582,6 +605,11 @@ static int net_init_nic(const NetClientOptions *opts, const char *name, } qemu_macaddr_default_if_unset(&nd->macaddr); + if (qemu_check_macaddr_uniqueness(nd->macaddr.a) < 0) { + error_report("ethernet address collision"); + return -1; + } + if (nic->has_vectors) { if (nic->vectors > 0x7ffffff) { error_report("invalid # of vectors: %"PRIu32, nic->vectors); diff --git a/net.h b/net.h index 04fda1d..227e47d 100644 --- a/net.h +++ b/net.h @@ -99,6 +99,7 @@ int qemu_show_nic_models(const char *arg, const char *const *models); void qemu_check_nic_model(NICInfo *nd, const char *model); int qemu_find_nic_model(NICInfo *nd, const char * const *models, const char *default_model); +int qemu_check_macaddr_uniqueness(uint8_t *macaddr); ssize_t qemu_deliver_packet(NetClientState *sender, unsigned flags,
QEMU doesn't check if there are mac collisions when adding nics. It causes mac address collisions in guest if adding the nics which include existing physical address. This patch fixes the issue. Signed-off-by: Lin Ma <lma@suse.com> --- hw/qdev-properties.c | 9 +++++++++ net.c | 28 ++++++++++++++++++++++++++++ net.h | 1 + 3 files changed, 38 insertions(+)