12:45 p.m.
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?
It appears to me that the addresses and routes assigned to a bridge are
all removed when the network is destroyed but otherwise remain.
Gene
2:11 p.m.
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... :-/
It appears to me that the addresses and routes assigned to a bridge
are all removed when the network is destroyed but otherwise remain.
2:23 p.m.
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
Daniel--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:34 a.m.
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 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. 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
11:32 a.m.
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4272
days inactive
4274
days old
4 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Gene Czarcinski -
Laine Stump