Re: [libvirt] [PATCH-v4 2/2] Support for static routes on a virtual bridge

On 04/25/2013 04:37 PM, Laine Stump wrote: > On 04/25/2013 04:13 PM, Gene Czarcinski wrote: >> On 04/25/2013 03:13 PM, Gene Czarcinski wrote: >>> On 04/22/2013 11:59 AM, Laine Stump wrote: >>>> address should be optional unless prefix or netmask is non-0, >>>> although >>>> I've now noticed that won't be handled properly due to >>>> virSocketAddrGetIpPrefix returning -1 when there is no address or >>>> prefix >>>> or netmask (I'm fixing that before I push that patch, so you can just >>>> toss your 1/2 patch, rebase, and assume it's fixed). >>> I have most of the stuff reworked except for the address, gateway, >>> netmask, and prefix code. Getting all of those balanced so they work >>> correctly is a bit tricky.. >>> >>> 1. For <route>, I am requiring that both address= and gateway= be >>> specified with address='0.0.0.0' and address='::' being valid >>> addresses. For IPv4, netmask='0.0.0.0' works correctly but prefix=0 >>> does not. >>> >>> For IPv4, address='0.0.0.0' results in a default route. I am not >>> sure what all these extra default routes are going to do to things >>> but lets not get in the way of the experimenter. >>> >>> For IPv6, this address='::', prefix='0' is a slightly different >>> matter as default routes are usually handled differently. I am going >>> to go ahead and implement it but I am not sure it is a good idea. >>> "Normally," if you do not specify a prefix for IPv6, the default is >>> 64. But if you do specify one, then it will be used. >>> >>> It is getting real close and it should be ready "real soon now" ;)) >>> >> AARRRRGH!!!! >> >> With IPv4 using address='0.0.0.0' and netmask='0.0.0.0' things work >> just fine but with prefix not so much. The problem is that with >> prefix=0, it is not in the xml which then results it it defaulting at >> a later time. This is an extreme corner case. Usually a zero prefix >> is just ignored. > Defaulting to what? I thought that when I pushed the utility function > for that, I modified it to return a prefix of 0 if the address was > 0.0.0.0 and neither netmask nor prefix was set. > > I guess it might be problematic if address was *not* 0 and you wanted an > explicit 0 prefix, but I don't think that would ever be useful. > > If you really want prefix to show up in the xml if someone explicitly > puts "prefix='0'" in there, you can add a "bool prefix_specified;" to > the object, and set that when you see a prefix, even if it's 0. Then in > the formatter you'll know that you should write out the value of prefix, > even if it's 0. > > There are a few examples of doing this in either the network or domain > xml parser/formatter - just search for occurrences of the word > "_specified" in src/conf/*.[ch] and you'll find them. > > OK, I have been trying all kinds of things in network_conf.c (which I now need to go back an clean up/simplify) because it is not the problem. The real problem is in virSocketAddrGetIpPrefix(). For IPv4, a valid address is '0.0.0.0' and for IPv6 a similar address is '::' and both are valid addresses. For IPv4, if a netmask of '0.0.0.0' is specified, things work but a prefix=0 is ignored/overridden. For IPv6, the prefix=0 is also overriden with the default of 64. To get this to work, I have to pass INVALID IPv4 and IPv6 addresses or not use the routine if the zero addresses are specified. If the addresses are zero, then I believe we can correctly assume that a prefix of zero is also correct. This code needs to be in bridge_driver.c.