[libvirt] Adding and Clearing bridge addresses

[Laine Stump]

On 03/13/2013 08:34 AM, Gene Czarcinski wrote:
> On 03/12/2013 03:23 PM, Daniel P. Berrange wrote:
>> On Tue, Mar 12, 2013 at 03:11:56PM -0400, Laine Stump wrote:
>>> On 03/12/2013 01:45 PM, Gene Czarcinski wrote:
>>>> I have been working on this patch to have libvirt optionally set
>>>> static routes.
>>>>
>>>> So I found the function that adds both IPv4 and IPv6 addresses to the
>>>> bridge in virnetdev.c. I found that besides the
>>>> virNetDevAddIPv4Address() there is also virNetDevCleanIPv4Address().
>>>> I patterned my virNetDevAddGateway() after the
>>>> virNetDevAddIPv4Address() function.
>>>>
>>>> What I am puzzled about is that it appears that nobody calls the Clear
>>>> function.  What don't I understand?
>>> Probably it's there just for completeness of API.
>>>
>>> The thing that I find strange is that these functions include "IPv4" in
>>> their names, in spite of working just as well for IPv6. It's very
>>> likely
>>> that if I dig back through the blame history (which won't be simple
>>> since the code has been moved into different files), I'll find that I
>>> originally wrote it (I have a vague memory of it), but don't think I
>>> would purposefully add IPv4 to the names... :-/
>> The virNetDevGetIPv4Address method uses an ioctl to get the address
>> and this only works on IPv4 addrs. The intent was to turn the other
>> two calls into ioctl() calls too, to kill off shell commands.
>>
>> Setting IPv6 addresses would require netlink usage instead IIRC
>>
> I seem to recall some comments in some source code I have browsed in
> the last couple days which stated that ioctl was NOT to be used
> because it could not be guaranteed long term ... 

I hadn't heard anything about that, but it is true that the ioctls can't
be used for IPv6 addresses, or for multiple IP addresses (v4 or v6) on
an interface. That's why what we really need to do is to switch all
these functions to use netlink (I guess with /usr/bin/ip as a fallback).

> I think it was in libvert code but it may have been in NetworkManager
> code.  When you jump around to different projects, the details get
> confusing.
>
> Anyway, I like the way /usr/sbin/ip works because things just seem to
> work.

There are two problems with using /usr/bin/ip:

1) it makes libvirt dependent on the iproute2 package

2) it adds one more external binary that libvirt must run as root,
creating a potential for exploit.

The problem with using ioctl(SIOCSIFADDR) is that it only supports a
single IP address (IPv4 only) per interface.

The "good" way to set IP addresses is (as Daniel mentioned) netlink
messages, at least on those platforms that have netlink. /usr/bin/ip
itself uses netlink.

When I added basic support for IPv6 to libvirt networks, libvirt didn't
yet have macvtap support (which also requires netlink), so none of the
infrastructure to do netlink was there yet, and adding a dependency on
iproute2 seemed like a reasonable expedient way to get the functionality
we needed. Three years later, we have other parts of the code using
netlink, and it would be nice to remove the dependency on iproute2 and
the exec of an external binary.


> A couple of things I have learned:
>
> 1.  For IPv6 addr and route, the order does not seem to matter and the
> bridge can be offline.
>
> 2.  Both IPv4 and IPv6 addr can be done on the offline bridge.
>
> 3.  To set an IPv4 static route, the addresses need to be defined and
> the bridge online (IPv6 is not so picky).
>
> 4.  You better have the unmasked portion of the address specified zero
> for the route ... that is, it better be a network address rather than
> a host address [I need to add some checks for this rather than letter
> the system issue a cryptic error message]
>
> 5.  I have yet to figure out how to test for dhcp, etc. when via is
> defined so that I can issue an error message when it is
> created/defined [the xml is being parsed and read in].
>
> 6.  When the xml is being written for an IP definition, I currently
> error out.  A better approach might be to issue an error message and
> suppress writing the xml.  This needs further thought.
>
> 7.  I have not come up with any tests.  The only thing that might be
> meaningful is if a bad network definition was created so that it would
> get errors.  I am not sure how to do this with the tests. There might
> be something in the networkxml2xmltest.
>
> I should have something with works real soon and will send the patch
> up for review.
>
> Gene
>
>
> -- 
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>