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

[Nehal J Wani]

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

The last 17 chars of the client DUID represent the MAC address
[00:01:00:01:19:dd:fb:af:((52:54:00:e7:85:1e))]. But RFC3315 strictly
suggests:
"""
   Clients and servers MUST treat DUIDs as opaque values and MUST only
   compare DUIDs for equality.  Clients and servers MUST NOT in any
   other way interpret DUIDs.  Clients and servers MUST NOT restrict
   DUIDs to the types defined in this document, as additional DUID types
   may be defined in the future.
"""

So, should we ignore leases containing ipv6 addresses?

-- 
Nehal J Wani