[DHCP] Fix up fake-packet creation as used by PXENV_GET_CACHED_INFO

Add dedicated functions create_dhcpdiscover(), create_dhcpack() and
create_proxydhcpack() for use by external code such as the PXE preboot
code.

Register ProxyDHCP options under the global scope "proxydhcp".

Unregister previously-acquired DHCP and ProxyDHCP settings when DHCP
succeeds.

src/arch/i386/image/nbi.c

@@ -390,7 +390,6 @@ static struct net_device * guess_boot_netdev ( void ) {
  * @ret rc		Return status code
  */
 static int nbi_prepare_dhcp ( struct image *image ) {
-	struct dhcp_packet dhcppkt;
 	struct net_device *boot_netdev;
 	int rc;
 
@@ -401,9 +400,8 @@ static int nbi_prepare_dhcp ( struct image *image ) {
 		return -ENODEV;
 	}
 
-	if ( ( rc = create_dhcp_response ( &dhcppkt, boot_netdev, DHCPACK,
-					   NULL, basemem_packet,
-					   sizeof ( basemem_packet ) ) ) != 0){
+	if ( ( rc = create_dhcpack ( boot_netdev, basemem_packet,
+				     sizeof ( basemem_packet ) ) ) != 0 ) {
 		DBGC ( image, "NBI %p failed to build DHCP packet\n", image );
 		return rc;
 	}

src/include/gpxe/dhcp.h

@@ -445,14 +445,12 @@ struct dhcphdr {
 /** Maximum time that we will wait for ProxyDHCP offers */
 #define PROXYDHCP_WAIT_TIME ( TICKS_PER_SEC * 1 )
 
-extern int create_dhcp_request ( struct dhcp_packet *dhcppkt,
-				 struct net_device *netdev, int msgtype,
-				 struct settings *offer_settings,
+extern int create_dhcpdiscover ( struct net_device *netdev,
+				 void *data, size_t max_len );
+extern int create_dhcpack ( struct net_device *netdev,
+			    void *data, size_t max_len );
+extern int create_proxydhcpack ( struct net_device *netdev,
 				 void *data, size_t max_len );
-extern int create_dhcp_response ( struct dhcp_packet *dhcppkt,
-				  struct net_device *netdev, int msgtype,
-				  struct settings *settings,
-				  void *data, size_t max_len );
 extern int start_dhcp ( struct job_interface *job, struct net_device *netdev );
 
 #endif /* _GPXE_DHCP_H */

src/interface/pxe/pxe_preboot.c

@@ -65,6 +65,26 @@ union pxe_cached_info {
 	BOOTPLAYER_t packet;
 } __attribute__ (( packed ));
 
+/** A PXE DHCP packet creator */
+struct pxe_dhcp_packet_creator {
+	/** Create DHCP packet
+	 *
+	 * @v netdev		Network device
+	 * @v data		Buffer for DHCP packet
+	 * @v max_len		Size of DHCP packet buffer
+	 * @ret rc		Return status code
+	 */
+	int ( * create ) ( struct net_device *netdev, void *data,
+			   size_t max_len );
+};
+
+/** PXE DHCP packet creators */
+static struct pxe_dhcp_packet_creator pxe_dhcp_packet_creators[] = {
+	[CACHED_INFO_DHCPDISCOVER] = { create_dhcpdiscover },
+	[CACHED_INFO_DHCPACK] = { create_dhcpack },
+	[CACHED_INFO_BINL] = { create_proxydhcpack },
+};
+
 /* The case in which the caller doesn't supply a buffer is really
  * awkward to support given that we have multiple sources of options,
  * and that we don't actually store the DHCP packets.  (We may not
@@ -117,13 +137,9 @@ PXENV_EXIT_t pxenv_unload_stack ( struct s_PXENV_UNLOAD_STACK *unload_stack ) {
  */
 PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 				     *get_cached_info ) {
-	struct dhcp_packet dhcppkt;
-	int ( * dhcp_packet_creator ) ( struct dhcp_packet *dhcppkt,
-					struct net_device *netdev, int msgtype,
-					struct settings *settings,
-					void *data, size_t max_len );
+	struct pxe_dhcp_packet_creator *creator;
+	union pxe_cached_info *info;
 	unsigned int idx;
-	unsigned int msgtype;
 	size_t len;
 	userptr_t buffer;
 	int rc;
@@ -135,25 +151,18 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 
 	/* Sanity check */
         idx = ( get_cached_info->PacketType - 1 );
-	if ( idx >= ( sizeof ( cached_info ) / sizeof ( cached_info[0] ) ) ) {
+	if ( idx >= NUM_CACHED_INFOS ) {
 		DBG ( " bad PacketType" );
 		goto err;
 	}
+	info = &cached_info[idx];
 
 	/* Construct cached version of packet, if not already constructed. */
-	if ( ! cached_info[idx].dhcphdr.op ) {
+	if ( ! info->dhcphdr.op ) {
 		/* Construct DHCP packet */
-		if ( get_cached_info->PacketType ==
-		     PXENV_PACKET_TYPE_DHCP_DISCOVER ) {
-			dhcp_packet_creator = create_dhcp_request;
-			msgtype = DHCPDISCOVER;
-		} else {
-			dhcp_packet_creator = create_dhcp_response;
-			msgtype = DHCPACK;
-		}
-		if ( ( rc = dhcp_packet_creator ( &dhcppkt, pxe_netdev,
-				       msgtype, NULL, &cached_info[idx],
-				       sizeof ( cached_info[idx] ) ) ) != 0 ) {
+		creator = &pxe_dhcp_packet_creators[idx];
+		if ( ( rc = creator->create ( pxe_netdev, info,
+					      sizeof ( *info ) ) ) != 0 ) {
 			DBG ( " failed to build packet" );
 			goto err;
 		}
@@ -188,8 +197,8 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 		 */
 		get_cached_info->Buffer.segment = rm_ds;
 		get_cached_info->Buffer.offset =
-			( unsigned ) ( & __from_data16 ( cached_info[idx] ) );
-		get_cached_info->BufferSize = sizeof ( cached_info[idx] );
+			( unsigned ) ( __from_data16 ( info ) );
+		get_cached_info->BufferSize = sizeof ( *info );
 		DBG ( " returning %04x:%04x+%04x['%x']",
 		      get_cached_info->Buffer.segment,
 		      get_cached_info->Buffer.offset,
@@ -197,13 +206,13 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 		      get_cached_info->BufferLimit );
 	} else {
 		/* Copy packet to client buffer */
-		if ( len > sizeof ( cached_info[idx] ) )
-			len = sizeof ( cached_info[idx] );
-		if ( len < sizeof ( cached_info[idx] ) )
+		if ( len > sizeof ( *info ) )
+			len = sizeof ( *info );
+		if ( len < sizeof ( *info ) )
 			DBG ( " buffer may be too short" );
 		buffer = real_to_user ( get_cached_info->Buffer.segment,
 					get_cached_info->Buffer.offset );
-		copy_to_user ( buffer, 0, &cached_info[idx], len );
+		copy_to_user ( buffer, 0, info, len );
 		get_cached_info->BufferSize = len;
 	}
 

src/net/udp/dhcp.c

@@ -130,6 +130,9 @@ static uint32_t dhcp_xid ( struct net_device *netdev ) {
 	return xid;
 }
 
+/** Settings block name used for ProxyDHCP responses */
+#define PROXYDHCP_SETTINGS_NAME "proxydhcp"
+
 /**
  * Create a DHCP packet
  *
@@ -145,10 +148,10 @@ static uint32_t dhcp_xid ( struct net_device *netdev ) {
  * dhcp_packet structure that can be passed to
  * set_dhcp_packet_option() or copy_dhcp_packet_options().
  */
-static int create_dhcp_packet ( struct dhcp_packet *dhcppkt,
-				struct net_device *netdev, uint8_t msgtype,
-				struct dhcp_options *options, 
-				void *data, size_t max_len ) {
+int create_dhcp_packet ( struct dhcp_packet *dhcppkt,
+			 struct net_device *netdev, uint8_t msgtype,
+			 struct dhcp_options *options, 
+			 void *data, size_t max_len ) {
 	struct dhcphdr *dhcphdr = data;
 	size_t options_len;
 	unsigned int hlen;
@@ -160,10 +163,6 @@ static int create_dhcp_packet ( struct dhcp_packet *dhcppkt,
 		return -ENOSPC;
 
 	/* Initialise DHCP packet content */
-
-        /* FIXME: wrong place to fix this. */
-        memset ( dhcppkt, 0, sizeof ( *dhcppkt ) );
-
 	memset ( dhcphdr, 0, max_len );
 	dhcphdr->xid = dhcp_xid ( netdev );
 	dhcphdr->magic = htonl ( DHCP_MAGIC_COOKIE );
@@ -182,6 +181,7 @@ static int create_dhcp_packet ( struct dhcp_packet *dhcppkt,
 	memcpy ( dhcphdr->options, options->data, options_len );
 
 	/* Initialise DHCP packet structure and settings interface */
+	memset ( dhcppkt, 0, sizeof ( *dhcppkt ) );
 	dhcppkt_init ( dhcppkt, NULL, data, max_len );
 	
 	/* Set DHCP_MESSAGE_TYPE option */
@@ -221,29 +221,30 @@ struct dhcp_client_uuid {
 #define DHCP_CLIENT_UUID_TYPE 0
 
 /**
- * Create DHCP request
+ * Create DHCP request packet
  *
  * @v dhcppkt		DHCP packet structure to fill in
  * @v netdev		Network device
- * @v msgtype		DHCP message type
- * @v offer_settings	Settings received in DHCPOFFER, or NULL
+ * @v dhcpoffer		DHCPOFFER packet received from server
  * @v data		Buffer for DHCP packet
  * @v max_len		Size of DHCP packet buffer
  * @ret rc		Return status code
  */
-int create_dhcp_request ( struct dhcp_packet *dhcppkt,
-			  struct net_device *netdev, int msgtype,
-			  struct settings *offer_settings,
-			  void *data, size_t max_len ) {
+static int create_dhcp_request ( struct dhcp_packet *dhcppkt,
+				 struct net_device *netdev,
+				 struct dhcp_packet *dhcpoffer,
+				 void *data, size_t max_len ) {
 	struct device_description *desc = &netdev->dev->desc;
 	struct dhcp_netdev_desc dhcp_desc;
 	struct dhcp_client_id client_id;
 	struct dhcp_client_uuid client_uuid;
+	unsigned int msgtype;
 	size_t dhcp_features_len;
 	size_t ll_addr_len;
 	int rc;
 
 	/* Create DHCP packet */
+	msgtype = ( dhcpoffer ? DHCPREQUEST : DHCPDISCOVER );
 	if ( ( rc = create_dhcp_packet ( dhcppkt, netdev, msgtype,
 					 &dhcp_request_options, data,
 					 max_len ) ) != 0 ) {
@@ -253,10 +254,10 @@ int create_dhcp_request ( struct dhcp_packet *dhcppkt,
 	}
 
 	/* Copy any required options from previous server repsonse */
-	if ( offer_settings ) {
+	if ( dhcpoffer ) {
 		if ( ( rc = copy_setting ( &dhcppkt->settings,
 					   DHCP_SERVER_IDENTIFIER,
-					   offer_settings,
+					   &dhcpoffer->settings,
 					   DHCP_SERVER_IDENTIFIER ) ) != 0 ) {
 			DBG ( "DHCP could not set server identifier "
 			      "option: %s\n", strerror ( rc ) );
@@ -264,7 +265,7 @@ int create_dhcp_request ( struct dhcp_packet *dhcppkt,
 		}
 		if ( ( rc = copy_setting ( &dhcppkt->settings,
 					   DHCP_REQUESTED_ADDRESS,
-					   offer_settings,
+					   &dhcpoffer->settings,
 					   DHCP_EB_YIADDR ) ) != 0 ) {
 			DBG ( "DHCP could not set requested address "
 			      "option: %s\n", strerror ( rc ) );
@@ -322,27 +323,87 @@ int create_dhcp_request ( struct dhcp_packet *dhcppkt,
 }
 
 /**
- * Create DHCP response
+ * Create DHCPDISCOVER packet
+ *
+ * @v netdev		Network device
+ * @v data		Buffer for DHCP packet
+ * @v max_len		Size of DHCP packet buffer
+ * @ret rc		Return status code
+ *
+ * Used by external code.
+ */
+int create_dhcpdiscover ( struct net_device *netdev,
+			  void *data, size_t max_len ) {
+	struct dhcp_packet dhcppkt;
+
+	return create_dhcp_request ( &dhcppkt, netdev, NULL, data, max_len );
+}
+
+/**
+ * Create DHCPACK packet
+ *
+ * @v netdev		Network device
+ * @v data		Buffer for DHCP packet
+ * @v max_len		Size of DHCP packet buffer
+ * @ret rc		Return status code
+ *
+ * Used by external code.
+ */
+int create_dhcpack ( struct net_device *netdev,
+		     void *data, size_t max_len ) {
+	struct dhcp_packet dhcppkt;
+	int rc;
+
+	/* Create base DHCPACK packet */
+	if ( ( rc = create_dhcp_packet ( &dhcppkt, netdev, DHCPACK, NULL,
+					 data, max_len ) ) != 0 )
+		return rc;
+
+	/* Merge in globally-scoped settings, then netdev-specific
+	 * settings.  Do it in this order so that netdev-specific
+	 * settings take precedence regardless of stated priorities.
+	 */
+	if ( ( rc = copy_settings ( &dhcppkt.settings, NULL ) ) != 0 )
+		return rc;
+	if ( ( rc = copy_settings ( &dhcppkt.settings,
+				    netdev_settings ( netdev ) ) ) != 0 )
+		return rc;
+
+	return 0;
+}
+
+/**
+ * Create ProxyDHCPACK packet
  *
- * @v dhcppkt		DHCP packet structure to fill in
  * @v netdev		Network device
- * @v msgtype		DHCP message type
- * @v settings		Settings to include, or NULL
  * @v data		Buffer for DHCP packet
  * @v max_len		Size of DHCP packet buffer
  * @ret rc		Return status code
+ *
+ * Used by external code.
  */
-int create_dhcp_response ( struct dhcp_packet *dhcppkt,
-			   struct net_device *netdev, int msgtype,
-			   struct settings *settings,
-			   void *data, size_t max_len ) {
+int create_proxydhcpack ( struct net_device *netdev,
+			  void *data, size_t max_len ) {
+	struct dhcp_packet dhcppkt;
+	struct settings *settings;
 	int rc;
 
-	/* Create packet and copy in options */
-	if ( ( rc = create_dhcp_packet ( dhcppkt, netdev, msgtype, NULL,
+	/* Identify ProxyDHCP settings */
+	settings = find_settings ( PROXYDHCP_SETTINGS_NAME );
+
+	/* No ProxyDHCP settings => return empty block */
+	if ( ! settings ) {
+		memset ( data, 0, max_len );
+		return 0;
+	}
+
+	/* Create base DHCPACK packet */
+	if ( ( rc = create_dhcp_packet ( &dhcppkt, netdev, DHCPACK, NULL,
 					 data, max_len ) ) != 0 )
 		return rc;
-	if ( ( rc = copy_settings ( &dhcppkt->settings, settings ) ) != 0 )
+
+	/* Merge in ProxyDHCP options */
+	if ( ( rc = copy_settings ( &dhcppkt.settings, settings ) ) != 0 )
 		return rc;
 
 	return 0;
@@ -356,10 +417,10 @@ int create_dhcp_response ( struct dhcp_packet *dhcppkt,
 
 /** A DHCP packet contained in an I/O buffer */
 struct dhcp_iobuf_packet {
-	/** Reference counter */
-	struct refcnt refcnt;
 	/** DHCP packet */
 	struct dhcp_packet dhcppkt;
+	/** Reference counter */
+	struct refcnt refcnt;
 	/** Containing I/O buffer */
 	struct io_buffer *iobuf;
 };
@@ -480,18 +541,28 @@ static void dhcp_finished ( struct dhcp_session *dhcp, int rc ) {
  * @ret rc		Return status code
  */
 static int dhcp_register_settings ( struct dhcp_session *dhcp ) {
+	struct settings *old_settings;
 	struct settings *settings;
 	struct settings *parent;
 	int rc;
 
+	/* Register ProxyDHCP settings, if present */
 	if ( dhcp->proxy_response ) {
 		settings = &dhcp->proxy_response->dhcppkt.settings;
+		settings->name = PROXYDHCP_SETTINGS_NAME;
+		old_settings = find_settings ( settings->name );
+		if ( old_settings )
+			unregister_settings ( old_settings );
 		if ( ( rc = register_settings ( settings, NULL ) ) != 0 )
 			return rc;
 	}
 
-	settings = &dhcp->response->dhcppkt.settings;
+	/* Register DHCP settings */
 	parent = netdev_settings ( dhcp->netdev );
+	settings = &dhcp->response->dhcppkt.settings;
+	old_settings = find_child_settings ( parent, settings->name );
+	if ( old_settings )
+		unregister_settings ( old_settings );
 	if ( ( rc = register_settings ( settings, parent ) ) != 0 )
 		return rc;
 
@@ -514,8 +585,8 @@ static int dhcp_send_request ( struct dhcp_session *dhcp ) {
 	struct xfer_metadata meta = {
 		.netdev = dhcp->netdev,
 	};
-	struct settings *offer_settings = NULL;
 	struct io_buffer *iobuf;
+	struct dhcp_packet *dhcpoffer;
 	struct dhcp_packet dhcppkt;
 	int rc;
 	
@@ -536,10 +607,9 @@ static int dhcp_send_request ( struct dhcp_session *dhcp ) {
 		return -ENOMEM;
 
 	/* Create DHCP packet in temporary buffer */
-	if ( dhcp->response )
-		offer_settings = &dhcp->response->dhcppkt.settings;
-	if ( ( rc = create_dhcp_request ( &dhcppkt, dhcp->netdev, dhcp->state,
-					  offer_settings, iobuf->data,
+	dhcpoffer = ( dhcp->response ? &dhcp->response->dhcppkt : NULL );
+	if ( ( rc = create_dhcp_request ( &dhcppkt, dhcp->netdev,
+					  dhcpoffer, iobuf->data,
 					  iob_tailroom ( iobuf ) ) ) != 0 ) {
 		DBGC ( dhcp, "DHCP %p could not construct DHCP request: %s\n",
 		       dhcp, strerror ( rc ) );

src/usr/dhcpmgmt.c

@@ -33,18 +33,12 @@
  */
 
 int dhcp ( struct net_device *netdev ) {
-	struct settings *settings;
 	int rc;
 
 	/* Check we can open the interface first */
 	if ( ( rc = ifopen ( netdev ) ) != 0 )
 		return rc;
 
-	/* Unregister any option blocks acquired via DHCP */
-	settings = find_child_settings ( netdev_settings ( netdev ), "dhcp" );
-	if ( settings )
-		unregister_settings ( settings );
-
 	/* Perform DHCP */
 	printf ( "DHCP (%s %s)", netdev->name, netdev_hwaddr ( netdev ) );
 	if ( ( rc = start_dhcp ( &monojob, netdev ) ) == 0 )