Centralise construction of the DHCP request and response packets.

src/arch/i386/image/nbi.c

@@ -397,16 +397,13 @@ static int nbi_prepare_dhcp ( struct image *image ) {
 		return -ENODEV;
 	}
 
-	if ( ( rc = create_dhcp_packet ( boot_netdev, DHCPACK, basemem_packet,
+	if ( ( rc = create_dhcp_response ( boot_netdev, DHCPACK, NULL,
-					 sizeof ( basemem_packet ),
+					   basemem_packet,
-					 &dhcppkt ) ) != 0 ) {
+					   sizeof ( basemem_packet ),
+					   &dhcppkt ) ) != 0 ) {
 		DBGC ( image, "NBI %p failed to build DHCP packet\n", image );
 		return rc;
 	}
-	if ( ( rc = copy_dhcp_packet_options ( &dhcppkt, NULL ) ) != 0 ) {
-		DBGC ( image, "NBI %p failed to copy DHCP options\n", image );
-		return rc;
-	}
 
 	return 0;
 }

src/include/gpxe/dhcp.h

@@ -503,15 +503,19 @@ extern void find_global_dhcp_ipv4_option ( unsigned int tag,
 					   struct in_addr *inp );
 extern void delete_dhcp_option ( struct dhcp_option_block *options,
 				 unsigned int tag );
+
 extern int apply_dhcp_options ( struct dhcp_option_block *options );
 extern int apply_global_dhcp_options ( void );
 
-extern struct dhcp_option_block dhcp_request_options;
+extern int create_dhcp_request ( struct net_device *netdev, int msgtype,
-extern int create_dhcp_packet ( struct net_device *netdev, uint8_t msgtype,
+				 struct dhcp_option_block *options,
-				void *data, size_t max_len,
+				 void *data, size_t max_len,
-				struct dhcp_packet *dhcppkt );
+				 struct dhcp_packet *dhcppkt );
-extern int copy_dhcp_packet_options ( struct dhcp_packet *dhcppkt,
+extern int create_dhcp_response ( struct net_device *netdev, int msgtype,
-				      struct dhcp_option_block *options );
+				  struct dhcp_option_block *options,
+				  void *data, size_t max_len,
+				  struct dhcp_packet *dhcppkt );
+
 extern int start_dhcp ( struct job_interface *job, struct net_device *netdev,
 			int (*register_options) ( struct net_device *,
 						  struct dhcp_option_block * ));

src/interface/pxe/pxe_preboot.c

@@ -69,10 +69,12 @@ 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 net_device *, int,
+					struct dhcp_option_block *, void *,
+					size_t, struct dhcp_packet * );
+	unsigned int msgtype;
 	void *data = NULL;
 	size_t len;
-	int msgtype;
-	struct dhcp_option_block *options;
 	userptr_t buffer;
 	int rc;
 
@@ -102,21 +104,17 @@ PXENV_EXIT_t pxenv_get_cached_info ( struct s_PXENV_GET_CACHED_INFO
 
 	/* Construct DHCP packet */
 	if ( get_cached_info->PacketType == PXENV_PACKET_TYPE_DHCP_DISCOVER ) {
+		dhcp_packet_creator = create_dhcp_request;
 		msgtype = DHCPDISCOVER;
-		options = &dhcp_request_options;
 	} else {
+		dhcp_packet_creator = create_dhcp_response;
 		msgtype = DHCPACK;
-		options = NULL;
 	}
-	if ( ( rc = create_dhcp_packet ( pxe_netdev, msgtype, data, len,
+	if ( ( rc = dhcp_packet_creator ( pxe_netdev, msgtype, NULL,
-					 &dhcppkt ) ) != 0 ) {
+					  data, len, &dhcppkt ) ) != 0 ) {
 		DBG ( " failed to build packet" );
 		goto err;
 	}
-	if ( ( rc = copy_dhcp_packet_options ( &dhcppkt, options ) ) != 0 ) {
-		DBG ( " failed to copy options" );
-		goto err;
-	}
 
 	/* Overwrite filename to work around Microsoft RIS bug */
 	if ( pxe_ris_filename ) {

src/net/udp/dhcp.c

@@ -108,7 +108,7 @@ static uint32_t dhcp_xid ( struct net_device *netdev ) {
 }
 
 /** Options common to all DHCP requests */
-struct dhcp_option_block dhcp_request_options = {
+static struct dhcp_option_block dhcp_request_options = {
 	.data = dhcp_request_options_data,
 	.max_len = sizeof ( dhcp_request_options_data ),
 	.len = sizeof ( dhcp_request_options_data ),
@@ -270,8 +270,8 @@ static int copy_dhcp_packet_encap_options ( struct dhcp_packet *dhcppkt,
  * @c options may specify a single options block, or be left as NULL
  * in order to copy options from all registered options blocks.
  */
-int copy_dhcp_packet_options ( struct dhcp_packet *dhcppkt,
+static int copy_dhcp_packet_options ( struct dhcp_packet *dhcppkt,
-			       struct dhcp_option_block *options ) {
+				      struct dhcp_option_block *options ) {
 	return copy_dhcp_packet_encap_options ( dhcppkt, options, 0 );
 }
 
@@ -289,9 +289,10 @@ int copy_dhcp_packet_options ( struct dhcp_packet *dhcppkt,
  * dhcp_packet structure that can be passed to
  * set_dhcp_packet_option() or copy_dhcp_packet_options().
  */
-int create_dhcp_packet ( struct net_device *netdev, uint8_t msgtype,
+static int create_dhcp_packet ( struct net_device *netdev,
-			 void *data, size_t max_len,
+				unsigned int msgtype,
-			 struct dhcp_packet *dhcppkt ) {
+				void *data, size_t max_len,
+				struct dhcp_packet *dhcppkt ) {
 	struct dhcphdr *dhcphdr = data;
 	int rc;
 
@@ -473,6 +474,97 @@ static struct dhcp_option_block * dhcp_parse ( const struct dhcphdr *dhcphdr,
 	return options;
 }
 
+/****************************************************************************
+ *
+ * Whole-packet construction
+ *
+ */
+
+/**
+ * Create DHCP request
+ *
+ * @v netdev		Network device
+ * @v msgtype		DHCP message type
+ * @v options		DHCP server response options, or NULL
+ * @v data		Buffer for DHCP packet
+ * @v max_len		Size of DHCP packet buffer
+ * @v dhcppkt		DHCP packet structure to fill in
+ * @ret rc		Return status code
+ */
+int create_dhcp_request ( struct net_device *netdev, int msgtype,
+			  struct dhcp_option_block *options,
+			  void *data, size_t max_len,
+			  struct dhcp_packet *dhcppkt ) {
+	int rc;
+
+	/* Create DHCP packet */
+	if ( ( rc = create_dhcp_packet ( netdev, msgtype, data, max_len,
+					 dhcppkt ) ) != 0 ) {
+		DBG ( "DHCP could not create DHCP packet: %s\n",
+		      strerror ( rc ) );
+		return rc;
+	}
+
+	/* Copy in options common to all requests */
+	if ( ( rc = copy_dhcp_packet_options ( dhcppkt,
+					       &dhcp_request_options )) !=0 ){
+		DBG ( "DHCP could not set common DHCP options: %s\n",
+		      strerror ( rc ) );
+		return rc;
+	}
+
+	/* Copy any required options from previous server repsonse */
+	if ( options ) {
+		if ( ( rc = copy_dhcp_packet_option ( dhcppkt, options,
+					  DHCP_SERVER_IDENTIFIER,
+					  DHCP_SERVER_IDENTIFIER ) ) != 0 ) {
+			DBG ( "DHCP could not set server identifier "
+			      "option: %s\n", strerror ( rc ) );
+			return rc;
+		}
+		if ( ( rc = copy_dhcp_packet_option ( dhcppkt, options,
+					  DHCP_EB_YIADDR,
+					  DHCP_REQUESTED_ADDRESS ) ) != 0 ) {
+			DBG ( "DHCP could not set requested address "
+			      "option: %s\n", strerror ( rc ) );
+			return rc;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * Create DHCP response
+ *
+ * @v netdev		Network device
+ * @v msgtype		DHCP message type
+ * @v options		DHCP options, or NULL
+ * @v data		Buffer for DHCP packet
+ * @v max_len		Size of DHCP packet buffer
+ * @v dhcppkt		DHCP packet structure to fill in
+ * @ret rc		Return status code
+ */
+int create_dhcp_response ( struct net_device *netdev, int msgtype,
+			   struct dhcp_option_block *options,
+			   void *data, size_t max_len,
+			   struct dhcp_packet *dhcppkt ) {
+	int rc;
+
+	/* Create packet and copy in options */
+	if ( ( rc = create_dhcp_packet ( netdev, msgtype, data, max_len,
+					 dhcppkt ) ) != 0 ) {
+		DBG ( " failed to build packet" );
+		return rc;
+	}
+	if ( ( rc = copy_dhcp_packet_options ( dhcppkt, options ) ) != 0 ) {
+		DBG ( " failed to copy options" );
+		return rc;
+	}
+
+	return 0;
+}
+
 /****************************************************************************
  *
  * DHCP to UDP interface
@@ -556,8 +648,8 @@ static int dhcp_send_request ( struct dhcp_session *dhcp ) {
 	struct xfer_metadata meta = {
 		.netdev = dhcp->netdev,
 	};
-	struct dhcp_packet dhcppkt;
 	struct io_buffer *iobuf;
+	struct dhcp_packet dhcppkt;
 	int rc;
 	
 	DBGC ( dhcp, "DHCP %p transmitting %s\n",
@@ -577,40 +669,15 @@ static int dhcp_send_request ( struct dhcp_session *dhcp ) {
 		return -ENOMEM;
 
 	/* Create DHCP packet in temporary buffer */
-	if ( ( rc = create_dhcp_packet ( dhcp->netdev, dhcp->state,
+	if ( ( rc = create_dhcp_request ( dhcp->netdev, dhcp->state,
-					 iobuf->data, iob_tailroom ( iobuf ),
+					  dhcp->options, iobuf->data,
-					 &dhcppkt ) ) != 0 ) {
+					  iob_tailroom ( iobuf ),
-		DBGC ( dhcp, "DHCP %p could not create DHCP packet: %s\n",
+					  &dhcppkt ) ) != 0 ) {
+		DBGC ( dhcp, "DHCP %p could not construct DHCP request: %s\n",
 		       dhcp, strerror ( rc ) );
 		goto done;
 	}
 
-	/* Copy in options common to all requests */
-	if ( ( rc = copy_dhcp_packet_options ( &dhcppkt,
-					       &dhcp_request_options ) ) != 0){
-		DBGC ( dhcp, "DHCP %p could not set common DHCP options: %s\n",
-		       dhcp, strerror ( rc ) );
-		goto done;
-	}
-
-	/* Copy any required options from previous server repsonse */
-	if ( dhcp->options ) {
-		if ( ( rc = copy_dhcp_packet_option ( &dhcppkt, dhcp->options,
-					    DHCP_SERVER_IDENTIFIER,
-					    DHCP_SERVER_IDENTIFIER ) ) != 0 ) {
-			DBGC ( dhcp, "DHCP %p could not set server identifier "
-			       "option: %s\n", dhcp, strerror ( rc ) );
-			goto done;
-		}
-		if ( ( rc = copy_dhcp_packet_option ( &dhcppkt, dhcp->options,
-					    DHCP_EB_YIADDR,
-					    DHCP_REQUESTED_ADDRESS ) ) != 0 ) {
-			DBGC ( dhcp, "DHCP %p could not set requested address "
-			       "option: %s\n", dhcp, strerror ( rc ) );
-			goto done;
-		}
-	}
-
 	/* Transmit the packet */
 	iob_put ( iobuf, dhcppkt.len );
 	rc = xfer_deliver_iob_meta ( &dhcp->xfer, iobuf, &meta );