[libvirt] [PATCH] conf: forbid use of multicast mac addresses
Laine Stump
laine at laine.org
Tue Mar 20 00:36:42 UTC 2012
On 03/19/2012 01:46 PM, Eric Blake wrote:
> On 03/19/2012 11:32 AM, Laine Stump wrote:
>> A few times libvirt users manually setting mac addresses have
>> complained of networking failure that ends up being due to a multicast
>> mac address being used for a guest interface. This patch prevents that
>> by loggin an error and failing if a multicast mac address is
> s/loggin/logging/
>
>> encountered in each of the three following cases:
>>
>> 1) domain xml <interface> mac address.
>> 2) network xml bridge mac address.
>> 3) network xml dhcp/host mac address.
>>
>> There are several other places where a mac address cna be input that
> s/cna/can/
>
>> aren't controlled in this manner because failure to do so has no
>> consequences (e.g., if the address will be used to search through
>> existing interfaces for a match).
>>
>> The RNG has been updated to add multiMacAddr and uniMacAddr along with
>> the existing macAddr, and macAddr was switched to uniMacAddr where
>> appropriate.
>> ---
>> docs/schemas/basictypes.rng | 17 ++++++++++++++++-
>> docs/schemas/domaincommon.rng | 6 +++---
>> docs/schemas/network.rng | 4 ++--
>> src/conf/domain_conf.c | 8 +++++++-
>> src/conf/network_conf.c | 33 ++++++++++++++++++++++++---------
>> src/libvirt_private.syms | 2 ++
>> src/util/virmacaddr.c | 13 +++++++++++++
>> src/util/virmacaddr.h | 3 ++-
> Is there any way to add a test of an expected error, where we prove that
> a multicast mac fails to validate? But since the existing tests use
> unicast addrs, and those still validate, you've at least tested for no
> regressions on sane usage.
>
>> + <!-- The lowest bit of the 1st byte is the "multicast" bit. a -->
>> + <!-- uniMacAddr requires that bit to be 0, and a multiMacAddr -->
>> + <!-- requires it to be 1. Plain macAddr will accept either. -->
>> + <!-- Currently there is no use of multiMacAddr in libvirt, it -->
>> + <!-- is included here for documentation/comparison purposes. -->
>> + <define name="uniMacAddr">
>> + <data type="string">
>> + <param name="pattern">[a-fA-F0-9][02468aAcCeE](:[a-fA-F0-9]{2}){5}</param>
>> + </data>
>> + </define>
>> + <define name="multiMacAddr">
>> + <data type="string">
>> + <param name="pattern">[a-fA-F0-9][13579bBdDfF](:[a-fA-F0-9]{2}){5}</param>
>> + </data>
>> + </define>
>> <define name="macAddr">
>> <data type="string">
>> - <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param>
>> + <param name="pattern">[a-fA-F0-9]{2}(:[a-fA-F0-9]{2}){5}</param>
> Changed to match the earlier patterns. Nice patterns.
>
> ACK. If you do figure out how to add a negative test for expected
> validation failure, I'm okay if you submit it as a separate followup patch.
>
Pushed (I agree that negative tests are in general a weak spot of the
xml2argv and xml2xml tests, just can't afford the distraction to deal
with it now).
More information about the libvir-list
mailing list