[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl: made vm mac address assignment deterministic
Wei Liu writes ("Re: [PATCH] libxl: made vm mac address assignment deterministic"): > On Wed, Sep 05, 2018 at 12:25:55PM +0000, Joshua Perrett wrote: > > Uses MD5 on the host mac address, vm name and vif index to generate the > > last three bytes of the vm mac address (for each vm). There is no such thing as "the" host mac address. The host might have several. However, generally there is a specific interface that will be used for this guest, depending on the vif connection mode. In bridge mode, for example, there is the mac address of the bridge. I think you should make sure to use the right interface. I think you need to add something to the documentation. You should mention that this approach is only deterministic *on the same host* (so in setups where the guest might be started on multiple hosts, with networked storage, it won't work) and only *with the same physical nic* (so swapping out the physical nic will change all the guests' addresses). > > MD5 code is originally from the public domain (written by Colin Plumb in > > 1993), files found in xen/tools/blktap2/drivers/. You are duplicating these files within the Xen tree. That's pretty ugly. Can we not do this a better way ? > > +static int libxl__get_host_mac(libxl__gc *gc, unsigned char *buf) > > +{ > > + int rc = ERROR_FAIL; > > Having a blank line makes things a bit clearer. > > > + #ifdef __linux__ > > + struct ifaddrs *iface_list; Can we not have #ifdefs here please ? The Linux-specific part should go in libxl_linux.c. > > + memcpy(&value, s->sll_addr, 6); > > Please use sizeof(s->sll_addr) instead of 6. Err, are you sure ? Let me quote the proposed code: > > + if (s->sll_halen == 6) { > > + uint64_t value = 0; > > + memcpy(&value, s->sll_addr, 6); This is very strange. How can it possibly be right to overwrite the first 6 bytes of value ? That does a different thing on big-endian and little-endian machines. ( Also why are we not checking sll_hatype ? > > + if (value > largest) { > > + memcpy(buf, s->sll_addr, 6); > > + largest = value; > > + rc = 0; > > Interesting algorithm, but I don't know anything better at the moment. > :) There is some reason for choosing the lexically largest address, IIRC. But the reason should be in a comment in the code. (ISTR reading something about this in the Debian bridge-interfaces(5) manpage.) Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |