[ipxe-devel] [PATCH 2/7] dhcpv6: Fallback to using DUID-LL for empty UUID

[Michael Brown]

On 01/04/15 08:26, Hannes Reinecke wrote:
> --- a/src/include/ipxe/dhcpv6.h
> +++ b/src/include/ipxe/dhcpv6.h
> @@ -11,6 +11,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
>
>   #include <stdint.h>
>   #include <ipxe/in.h>
> +#include <ipxe/if_ether.h>
>   #include <ipxe/uuid.h>
>
>   /** DHCPv6 server port */
> @@ -32,6 +33,16 @@ struct dhcpv6_option {
>   	uint8_t data[0];
>   } __attribute__ (( packed ));
>
> +/** DHCP unique identifier based on UUID (DUID-LL) */
> +struct dhcpv6_duid_ll {
> +	/** Type */
> +	uint16_t type;
> +	/** Hardware type */
> +	uint16_t hw_type;
> +	/** IPv6 link-layer address */
> +	uint8_t ll_addr[ETH_ALEN];
> +} __attribute__ (( packed ));
> +

Does DUID-LL support non-Ethernet link-layer addresses?

>   	/** Client DUID */
> -	struct dhcpv6_duid_uuid client_duid;
> +	union {
> +		struct dhcpv6_duid_uuid uuid;
> +		struct dhcpv6_duid_ll ll;
> +		uint8_t raw[24];
> +	} client_duid;

Where does the magic 24 come from?  (Do we need raw[] at all?)

> +	int client_duid_len;

size_t?

> @@ -935,15 +941,26 @@ int start_dhcpv6 ( struct interface *job, struct net_device *netdev,
>   	addresses.server.sin6.sin6_port = htons ( DHCPV6_SERVER_PORT );
>
>   	/* Construct client DUID from system UUID */
> -	dhcpv6->client_duid.type = htons ( DHCPV6_DUID_UUID );
> +	dhcpv6->client_duid_len = 18;

Where does the magic 18 come from?

> +	dhcpv6->client_duid.uuid.type = htons ( DHCPV6_DUID_UUID );
>   	if ( ( len = fetch_uuid_setting ( NULL, &uuid_setting,
> -					  &dhcpv6->client_duid.uuid ) ) < 0 ) {
> +					  &dhcpv6->client_duid.uuid.uuid ) ) <= 0 ) {
>   		rc = len;
>   		DBGC ( dhcpv6, "DHCPv6 %s could not create DUID-UUID: %s\n",
>   		       dhcpv6->netdev->name, strerror ( rc ) );
>   		goto err_client_duid;
>   	}
> -
> +	memset(&null_uuid, 0, sizeof(null_uuid));

You can save code size by having a "static const uuid null_uuid;" - it 
will end up in .bss and so will take no space in the final binary (at 
the cost of 16 bytes of runtime memory, which is cheaper).

(It might even be worth making it a global in core/uuid.c, as is done 
with e.g. eth_broadcast.)

> +	if (!memcmp( &dhcpv6->client_duid.uuid.uuid, &null_uuid,
> +		     sizeof(null_uuid) )) {
> +		DBGC ( dhcpv6, "DHCPv6 %s empty DUID-UUID, using DUID-LL\n",
> +		       dhcpv6->netdev->name );
> +		dhcpv6->client_duid.ll.type = htons ( DHCPV6_DUID_LL );
> +		dhcpv6->client_duid.ll.hw_type = ll_protocol->ll_proto & 0xFFFF;

ll_proto is already a uint16_t.

Thanks,

Michael