[dhcp] Use Ethernet-compatible chaddr, if possible

For IPoIB, we currently use the hardware address (i.e. the eight-byte
GUID) as the DHCP chaddr.  This works, but some PXE servers (notably
Altiris RDP) refuse to respond if the chaddr field is anything other
than six bytes in length.

We already have the notion of an Ethernet-compatible link-layer
address, which is used in the iBFT (the design of which similarly
fails to account for non-Ethernet link layers).  Use this as the first
preferred alternative to the actual link-layer address when
constructing the DHCP chaddr field.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/include/ipxe/dhcp.h b/src/include/ipxe/dhcp.h
index aac79b29f8590a202004ef6b691602d96cf1b85c..54a85f6614bbe37bf0f5d226d070e5c4402dfd8d 100644 (file)
--- a/src/include/ipxe/dhcp.h
+++ b/src/include/ipxe/dhcp.h
@@ -620,8 +620,8 @@ struct dhcphdr {
 /** Setting block name used for BootServerDHCP responses */
 #define PXEBS_SETTINGS_NAME "pxebs"
 
-extern void * dhcp_chaddr ( struct net_device *netdev, uint8_t *hlen,
-                           uint16_t *flags );
+extern unsigned int dhcp_chaddr ( struct net_device *netdev, void *chaddr,
+                                 uint16_t *flags );
 extern int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
                                struct net_device *netdev, uint8_t msgtype,
                                const void *options, size_t options_len,

diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index 0524385a94edb9c49ba9b625b4942048c414257f..64c49cdd960bcca72651271acb52d7fd61fa5f9a 100644 (file)
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -867,20 +867,24 @@ static struct dhcp_session_state dhcp_state_pxebs = {
  * Construct DHCP client hardware address field and broadcast flag
  *
  * @v netdev           Network device
- * @v hlen             DHCP hardware address length to fill in
- * @v flags            DHCP flags to fill in
- * @ret chaddr         DHCP client hardware address
+ * @v chaddr           Hardware address buffer
+ * @v flags            Flags to set (or NULL)
+ * @ret hlen           Hardware address length
  */
-void * dhcp_chaddr ( struct net_device *netdev, uint8_t *hlen,
-                    uint16_t *flags ) {
+unsigned int dhcp_chaddr ( struct net_device *netdev, void *chaddr,
+                          uint16_t *flags ) {
        struct ll_protocol *ll_protocol = netdev->ll_protocol;
-       typeof ( ( ( struct dhcphdr * ) NULL )->chaddr ) chaddr;
+       struct dhcphdr *dhcphdr;
+       int rc;
 
        /* If the link-layer address cannot fit into the chaddr field
-        * (as is the case for IPoIB) then try using the hardware
-        * address instead.  If we do this, set the broadcast flag,
-        * since chaddr then does not represent a valid link-layer
-        * address for the return path.
+        * (as is the case for IPoIB) then try using the Ethernet-
+        * compatible link-layer address.  If we do this, set the
+        * broadcast flag, since chaddr then does not represent a
+        * valid link-layer address for the return path.
+        *
+        * If we cannot produce an Ethernet-compatible link-layer
+        * address, try using the hardware address.
         *
         * If even the hardware address is too large, use an empty
         * chaddr field and set the broadcast flag.
@@ -891,16 +895,19 @@ void * dhcp_chaddr ( struct net_device *netdev, uint8_t *hlen,
         * storage, or by eliminating the hardware address completely
         * from the DHCP packet, which seems unfriendly to users.
         */
-       if ( ( *hlen = ll_protocol->ll_addr_len ) <= sizeof ( chaddr ) ) {
-               return netdev->ll_addr;
+       if ( ll_protocol->ll_addr_len <= sizeof ( dhcphdr->chaddr ) ) {
+               memcpy ( chaddr, netdev->ll_addr, ll_protocol->ll_addr_len );
+               return ll_protocol->ll_addr_len;
        }
-       *flags = htons ( BOOTP_FL_BROADCAST );
-       if ( ( *hlen = ll_protocol->hw_addr_len ) <= sizeof ( chaddr ) ) {
-               return netdev->hw_addr;
-       } else {
-               *hlen = 0;
-               return NULL;
+       if ( flags )
+               *flags |= htons ( BOOTP_FL_BROADCAST );
+       if ( ( rc = ll_protocol->eth_addr ( netdev->ll_addr, chaddr ) ) == 0 )
+               return ETH_ALEN;
+       if ( ll_protocol->hw_addr_len <= sizeof ( dhcphdr->chaddr ) ) {
+               memcpy ( chaddr, netdev->hw_addr, ll_protocol->hw_addr_len );
+               return ll_protocol->hw_addr_len;
        }
+       return 0;
 }
 
 /**
@@ -923,7 +930,6 @@ int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
                         const void *options, size_t options_len,
                         void *data, size_t max_len ) {
        struct dhcphdr *dhcphdr = data;
-       void *chaddr;
        int rc;
 
        /* Sanity check */
@@ -936,8 +942,8 @@ int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
        dhcphdr->magic = htonl ( DHCP_MAGIC_COOKIE );
        dhcphdr->htype = ntohs ( netdev->ll_protocol->ll_proto );
        dhcphdr->op = dhcp_op[msgtype];
-       chaddr = dhcp_chaddr ( netdev, &dhcphdr->hlen, &dhcphdr->flags );
-       memcpy ( dhcphdr->chaddr, chaddr, dhcphdr->hlen );
+       dhcphdr->hlen = dhcp_chaddr ( netdev, dhcphdr->chaddr,
+                                     &dhcphdr->flags );
        memcpy ( dhcphdr->options, options, options_len );
 
        /* Initialise DHCP packet structure */

diff --git a/src/usr/dhcpmgmt.c b/src/usr/dhcpmgmt.c
index ed8a9fe6541fd3b173833f5007eb05838faf4b12..e651dfda1059d06f67d57bc50f8b3977eb1eb888 100644 (file)
--- a/src/usr/dhcpmgmt.c
+++ b/src/usr/dhcpmgmt.c
@@ -37,9 +37,10 @@ FILE_LICENCE ( GPL2_OR_LATER );
  */
 
 int dhcp ( struct net_device *netdev ) {
-       uint8_t *chaddr;
-       uint8_t hlen;
-       uint16_t flags;
+       struct dhcphdr *dhcphdr;
+       typeof ( dhcphdr->chaddr ) chaddr;
+       unsigned int hlen;
+       unsigned int i;
        int rc;
 
        /* Check we can open the interface first */
@@ -51,10 +52,11 @@ int dhcp ( struct net_device *netdev ) {
                return rc;
 
        /* Perform DHCP */
-       chaddr = dhcp_chaddr ( netdev, &hlen, &flags );
-       printf ( "DHCP (%s ", netdev->name );
-       while ( hlen-- )
-               printf ( "%02x%c", *(chaddr++), ( hlen ? ':' : ')' ) );
+       printf ( "DHCP (%s", netdev->name );
+       hlen = dhcp_chaddr ( netdev, chaddr, NULL );
+       for ( i = 0 ; i < hlen ; i++ )
+               printf ( "%c%02x", ( i ? ':' : ' ' ), chaddr[i] );
+       printf ( ")" );
 
        if ( ( rc = start_dhcp ( &monojob, netdev ) ) == 0 ) {
                rc = monojob_wait ( "" );