[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] dhcp6, radvd, ip6tables, etc. (update)



On 10/30/2012 02:28 PM, Laine Stump wrote:
On 10/30/2012 11:58 AM, Gene Czarcinski wrote:

Well, ff02::1:2 does have some meaning in dhcp6.

 From what I have seen by "well behaved" clients is that the client
always uses port 546 and the server always uses port 547.  But,
dnsmasq have some comments/code which indicates that not all clients
are "well behaved."

In dhcp6, a little four dhcpv6 dance is performed to establish the
clients address:

1. dhcpv6 solicit:  from=fe80::client:546  to=ff02::1:2:547
2. dhcpv6 advertise:  from=fe80::server:547  to=fe80::client:546
3. dhcpv6 request:  from=fe80::client:546  to=ff02::1:2:547
4. dhcpv6 reply:  from=fe80::server:547  to=fe80::client:546

Or, in other words: (1) need dhcpv6, (2) I serve it, (3) OK, give me
one, and (4) here it is.
Ah, so that address has a special meaning/function.
OK, I took the easy approach. I will go back and "do it right" so that both "-d ff02::1:2" and "--dport 547" are specified.


Since dnsmasq does its own packet filtering and with bind-interfaces
having a real meaning, it all works (assuming that radvd has the right
configuration).

This works With the radvd configuration and a dhcp-range specified for
a ipv6 subnet, a guest will get a dhcp6 address and RA default route.
Interesting - so both radvd and dnsmasq are involved, correct?
Yes, why not?
No reason. Just curious.

Sometime in the future this should be reconsidered and either
everything is done by dnsmasq or it is at least an option.
Yeah, it couldn't be mandatory, at least not now, since there are still
lots of people using dnsmasq versions that don't have all the required
capabilities.
I am having second thoughts and think it might be a good idea to just have dnsmasq do everything since it can do both dhcp6 and RA.

This sure does simplify things: if you have dhcp6, add the dnsmasq parameter "enable-ra". If not, don't add it.

About being compatible with old code. I see the dhcp6 patches and this potential patch as being based on v1.0.0 and not being back-ported. So I am not sure I see a potential conflict.


   I must say that having dnsmasq do everything does have appeal ...
one less dependency.
4.  After getting all of this working to my satisfaction, my next
mountain to climb is VM ... it really does not like network xml
definitions which include a dhcp-range for an ipv6 definition.

Comments?

NOTE:  I am implementing all of this assuming that my previous
patches have been accepted ... the ones for creating a dnsmasq
conf-file for parameters rather than using the dnsmasq command-line.
I have no problem with the "convert from long commandline to conf file"
patch except for the bit that points to a "conf directory" where user
supplied conf files can be added. Aside from that part needing to be in
a spearate patch, if we're going to add that kind of configurability, we
need to do it in a way that will allow us to easily see that the user is
playing outside the fence (otherwise we spend a lot of time chasing
"bugs" that end up being caused by user-supplied options).
Originally, I wanted the conf-dir so that I could pop in/out some
configuration changes that would happen when dnsmasq re-read the
configuration.  Well, it does not work that way and dnsmasq has to be
restarted for some of the more interesting changes.  Given this, I
believe that conf-dir serves no useful purpose and should be removed
and I will remove it and resubmit the patch.
Because we're in freeze right now I haven't spent a lot of time
discussing that, but planned to send a message about it when I get a
minute.
Right now I am working on getting dhcpv6 functional and, while what I
have works, there is still more to do.
I am sure that someone could spend the time refitting the dhcp6
patches to the old code but why get aggravated?  If you folks do not
want to do things that way, fine, please say so.  But if it is going
to be accepted, then I would like some indication of this.
5. As far as I can tell (or at least this is for dnsmasq),
"dhcp-no-override", "enable-tftp", "tftp-root=", and "dhcp-boot=" are
all IPv4 only and thus ignored for IPv6 in bridge_driver.  I have not
looked to see what network_conf.c does.
"what network_conf.c does"? Well, it of course doesn't deal directly
with those options, but the config that feeds into some of those options
is parsed in virNetworkIPParseXML(), and is only done if the <ip>
element is ipv4. But then you've already seen that code if you have dhcp
working for ipv6 - the <dhcp> element is also only parsed for ipv6. The
format-side code doesn't have that extra check; I guess I figured that
if there was no way to configure ipv6 with dhcp or tftp, it was safe to
assume any ip element with dhcp or tftp info was ipv4 anyway.
I have now dived into network_conf.c and a little into dnsmasq.c. Yet
again I was surprised because most of what was needed for dhcpv6 was a
little tweaking here and there.
I'd like to think that's because we've tried to do things in a manner
that anticipates future expansions, but some of it may be pure luck :-)
A little luck can go a long way because no matter how good you are, it is impractical to predict the future.

To support dhcp-host for IPv6, I did assumed that for IPv6 no MAC
address would be specified since it does not have a defined meaning in
DHCPv6.  Therefore, in dnsmasq.c/hostsfileAdd(), if the mac==NULL, I
use the IPv6 format of <hostname>,[<ipv6-addr>]  whereas for IPv4 it
is either MAC,<ipv4-addr>,<hostname. or MAC,<ipv4-addr>.

This way most of the code works as is.  Dnsmasq has lots of options
and different ways that dhcp-host= can be specified, but this is
simple and I know it works.
6.  Handling of the info for addn-hosts file and the dhcp-hostsfile.
This currently works because things are forced so that one and only
one IPv4 dhcp definition will be handled.  With the addition of IPv6
dhcp, things fall apart.

6a. addn-hosts:  The addn-hosts file is similar to the /etc/hosts file
in both form and function.  The <dns>-<host> specification is done on
an interface bases and, thus, the processing of the data and creation
of the file should only be done once.

6b. dhcp-hostsfile (dhcp-host=):  This needs to be done at least for
every ip definition that is processed for dhcp.  Initially, this will
be for dhcp4 only until I can figure out how to do it for dhcp6.

6c. Thus, networkBuildDnsmasqHostsfile() needs to be split into two
functions [one for addn-hosts and one for dhcp-hosts]. Additionally,
all the functions which call dnsmasqSave() need to be reworked
appropriately.
I've actually never liked the "dnsmasqContext" concept, as it seems like
overkill and has conceptual problems such as what you've described. I
would be just as happy with replacements that were simpler and easier to
deal with. I think part of what complicates dnsmasq.[hc] is that it's in
the util directory, so it isn't allowed to understand the contents of
virNetworkDef, and must instead be sent the list of hosts in a simpler
format. If, instead, there was a file src/network/bridge_dnsmasq.[hc],
that could have functions that took virNetworkDef as an arg, and just
immediately return a string (or, in a separate function, write to a
file). That should simplify calling, and writing tests. And existing
dnsmasq-related functions in bridge_driver.c could be moved there as
well, reducing clutter in bridge_driver.c. (Of course I'm saying all
this without ever seriously considering it, just talking off the cuff,
so I may be completely wrong :-)
Right now I have fixed things up so they work.  I would like to leave
this as an exercise for someone else [or at least a later time].

Besides, isn't bridge_driver.c pretty much for dnsmasq only?
Right now, yes. It's always possible that an alternative could come
along, of course...
The alternative would have to be really good to replace all the functionality of dnsmasq and would likely have it's own driver.


7. So far, the only things I have done involving the xml specification
is to enable <dhcp> for IPv6.  However, the  xml to specify a dns
addn-hosts appears, IMHO, to be overly verbose and complicated.
It is made that way because a single IP address may have multiple
hostnames associated with it, and we want to avoid having multiple
methods of describing the same thing. What you propose in the next
couple sentences was already proposed, tried, and rejected when dns host
support was originally added.
OK.
    So, while allowing the current xml to be valid, I suggest adding an
alternate form for which is similar to that used for dhcp-host.  An
example is "<host ip='1.2.3.4' name='one' />"

There are a few places in libvirt's XML where the same thing can be
expressed in two different ways, but that is only done when necessary
because the existing XML is unable to completely describe the new
functionality but backward compatibility is required. It creates all
sorts of problems when formatting the config back into XML though (which
of the two do you choose? Or do you do both? Either of these is a bad
answer), therefore we definitely don't want to do that except in cases
where it is absolutely necessary; this isn't one of those cases.
This is why I have tried to tweak and bend things so it works (and
because it mostly worked before).

And now, as the saying goes, one more thing.

I now realize that I am going to need to get into virsh net-update
since I am adding things to the xml specification and net-update will
need to differentiate between dhcp4 and dhcp6 changes.

Another thought that occurs to me is whether there has any
consideration been given having a "virsh net-restart" which would just
restart dnsmasq and radvd.  Typing stuff in for the command line of
net-update is a little prone to typos.
You can always put them in (temporary) files :-)
I am still thinking about this and I still have to fix net-update for dhcp6.


Gene


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]