[libvirt] [PATCHv4 1/4] net-dhcp-leases: Implement the public APIs

[Daniel P. Berrange]

On Wed, Oct 02, 2013 at 04:30:16PM +0530, Nehal J Wani wrote:
> On Wed, Oct 2, 2013 at 4:18 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> > On Wed, Oct 02, 2013 at 03:41:56PM +0530, Nehal J Wani wrote:
> >> On Wed, Oct 2, 2013 at 3:18 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> >> > On Wed, Oct 02, 2013 at 03:08:00PM +0530, Nehal J Wani wrote:
> >> >> On Wed, Oct 2, 2013 at 1:43 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> >> >> > On Tue, Oct 01, 2013 at 05:39:02PM -0600, Eric Blake wrote:
> >> >> >> On 09/26/2013 02:08 AM, Nehal J Wani wrote:
> >> >> >> > Introduce 3 new APIs, virNetworkGetDHCPLeases, virNetworkGetDHCPLeasesForMAC
> >> >> >> > and virNetworkDHCPLeaseFree.
> >> >> >> >
> >> >> >> > * virNetworkGetDHCPLeases: returns the dhcp leases information for a given
> >> >> >> >      virtual network.
> >> >> >> >
> >> >> >> >      For DHCPv4, the information includes:
> >> >> >> >      - Expirytime
> >> >> >> >      - MAC Address
> >> >> >> >      - IPv4 address (with type and prefix)
> >> >> >> >      - Hostname (can be NULL)
> >> >> >> >      - Client ID (can be NULL)
> >> >> >> >
> >> >> >> >      For DHCPv6, the information includes
> >> >> >> >      - Expirytime
> >> >> >> >      - IAID
> >> >> >> >      - IPv6 address (with type and prefix)
> >> >> >> >      - Hostname (can be NULL)
> >> >> >> >      - Client DUID
> >> >> >> >
> >> >> >> > * virNetworkGetDHCPLeasesForMAC: returns the dhcp leases information for a
> >> >> >> >      given virtual network and specified MAC Address.
> >> >> >> >
> >> >> >> > * virNetworkDHCPLeaseFree: allows the upper layer application to free the
> >> >> >> >      network interface object conveniently.
> >> >> >> >
> >> >> >> > There is no support for flags, so user is expected to pass 0 for
> >> >> >> > both the APIs.
> >> >> >>
> >> >> >>
> >> >> >> > +typedef struct _virNetworkDHCPLeases virNetworkDHCPLeases;
> >> >> >> > +typedef virNetworkDHCPLeases *virNetworkDHCPLeasesPtr;
> >> >> >> > +struct _virNetworkDHCPLeases {
> >> >> >> > +    long long expirytime;       /* Seconds since epoch */
> >> >> >> > +    union {
> >> >> >> > +        char *mac;              /* MAC address */
> >> >> >> > +        unsigned long iaid;     /* Identity association identifier (IAID) */
> >> >> >> > +    } id;
> >> >> >>
> >> >> >> I'm not sure I like iaid - the whole point of this interface was to
> >> >> >> return IP addresses associated with a MAC.  Either the iaid is important
> >> >> >> and deserves a separate field, or all we care about is the MAC address.
> >> >> >>  Not to mention that you didn't document which leg of the id union is
> >> >> >> valid based on the type discriminator.
> >> >> >
> >> >> > Agreed, we want the MAC address to be unconditionally available
> >> >> > here. IMHO the IAID is not something we care about exposing. That
> >> >> > is a impl detail of the DHCP comms protocol that is not useful
> >> >> > to people outside.
> >> >> >
> >> >>
> >> >> So in case DHCPv6 is used by the client, should we report the rest of
> >> >> the lease fields and report MAC as NULL?
> >> >
> >> > No, we must report the MAC. This data is useless without the MAC address
> >> > being present.
> >> >
> >> > You can't even implement the virNetworkGetDHCPLeasesForMAC API without
> >> > knowing the MAC for a lease.
> >>
> >> The issue is, in case of leases containing ipv6 addresses, there is no
> >> field for MAC address. Laine suggested extracting MAC address from the
> >> cliend DUID. For example:
> >>
> >> 1380692760 52:54:00:e7:85:1e 192.168.122.116 * *
> >> duid 00:01:00:01:19:dd:fb:37:f0:4d:a2:8c:14:51
> >> 1380692762 15172894 2001:db8:ca2:2:1::de *
> >> 00:01:00:01:19:dd:fb:af:52:54:00:e7:85:1e
> >
> > We don't want to design the API around the limitations of one particular
> > DHCP server implementation. If dnsmasq's leases file can't give us the
> > MAC addr, we still want to allow for MAC in the public struct. Have you
> > asked the dnsmasq developers if they're willing to add a field for the
> > MAC addr to the leases file for IPv6 ?
> 
> I think one of them answered this question in the thread:
> http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2013q3/007538.html
> One has to read all the follow-ups :)
> 
> For a tl;dr moment:
> 
> """
> However, if you're interested in the MAC addresses of clients, the very
> latest dnsmasq code can determine that in most cases. The MAC address is
> not stored in the leases file, but it can be used to key configurations
> to particular MAC addresses, and it's made available to the DHCP lease
> external script, so an external application can use it.
> """

So we can get the MAC addr if we use the '--dhcp-script' parameter to
make dnsmasq invoke a helper program we create, to save the lease
details we need.


Incidentally, I see our XML format is actually wrong, because it says
the 'mac' attribute is forbidden for  <host> elements using IPv6. dnsmasq
actually allows you to specify either mac addr or IAID identifiers for
IPv6 dhcp host entries.

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 :|