Don't build option-overloaded packets; they just confuse people (ISC

DHCPD and Windows RIS in particular).

3 files changed, 23 insertions, 55 deletions

diff --git a/contrib/bochs/README.windows-ris b/contrib/bochs/README.windows-ris
index 00bddfbb..830db6a8 100644
--- a/contrib/bochs/README.windows-ris
+++ b/contrib/bochs/README.windows-ris
@@ -21,3 +21,11 @@ startrom.com in a hex editor, search for the hex string
 now accept the "F" key instead of "F12".
 
 
+
+DHCP filename
+=============
+
+Must use Windows backslash separator e.g. 'filename
+"OSChooser\\i386\\startrom.com";', otherwise startrom.com fails to
+correctly identify the path to NTLDR.
+
diff --git a/src/include/gpxe/dhcp.h b/src/include/gpxe/dhcp.h
index 634e47c7..3183a3a9 100644
--- a/src/include/gpxe/dhcp.h
+++ b/src/include/gpxe/dhcp.h
@@ -387,12 +387,12 @@ struct dhcphdr {
 	 *
 	 * This field may be overridden and contain DHCP options
 	 */
-	uint8_t sname[64];
+	char sname[64];
 	/** Boot file name (null terminated)
 	 *
 	 * This field may be overridden and contain DHCP options
 	 */
-	uint8_t file[128];
+	char file[128];
 	/** DHCP magic cookie
 	 *
 	 * Must have the value @c DHCP_MAGIC_COOKIE.
@@ -423,20 +423,6 @@ struct dhcphdr {
  */
 #define DHCP_MIN_LEN 552
 
-/** DHCP packet option block fill order
- *
- * This is the order in which option blocks are filled when
- * reassembling a DHCP packet.  We fill the smallest field ("sname")
- * first, to maximise the chances of being able to fit large options
- * within fields which are large enough to contain them.
- */
-enum dhcp_packet_option_block_fill_order {
-	OPTS_SNAME = 0,
-	OPTS_FILE,
-	OPTS_MAIN,
-	NUM_OPT_BLOCKS
-};
-
 /**
  * A DHCP packet
  *
@@ -448,14 +434,8 @@ struct dhcp_packet {
 	size_t max_len;
 	/** Used length of the DHCP packet buffer */
 	size_t len;
-	/** DHCP option blocks within a DHCP packet
-	 *
-	 * A DHCP packet contains three fields which can be used to
-	 * contain options: the actual "options" field plus the "file"
-	 * and "sname" fields (which can be overloaded to contain
-	 * options).
-	 */
-	struct dhcp_option_block options[NUM_OPT_BLOCKS];
+	/** DHCP options */
+	struct dhcp_option_block options;
 };
 
 /**
diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index fc0be374..961f66a5 100644
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -138,7 +138,7 @@ static int set_dhcp_packet_option ( struct dhcp_packet *dhcppkt,
 				    unsigned int tag, const void *data,
 				    size_t len ) {
 	struct dhcphdr *dhcphdr = dhcppkt->dhcphdr;
-	struct dhcp_option_block *options = dhcppkt->options;
+	struct dhcp_option_block *options = &dhcppkt->options;
 	struct dhcp_option *option = NULL;
 
 	/* Special-case the magic options */
@@ -152,31 +152,23 @@ static int set_dhcp_packet_option ( struct dhcp_packet *dhcppkt,
 	case DHCP_EB_SIADDR:
 		memcpy ( &dhcphdr->siaddr, data, sizeof ( dhcphdr->siaddr ) );
 		return 0;
-	case DHCP_MESSAGE_TYPE:
-	case DHCP_REQUESTED_ADDRESS:
-	case DHCP_PARAMETER_REQUEST_LIST:
-		/* These options have to be within the main options
-		 * block.  This doesn't seem to be required by the
-		 * RFCs, but at least ISC dhcpd refuses to recognise
-		 * them otherwise.
-		 */
-		options = &dhcppkt->options[OPTS_MAIN];
-		break;
+	case DHCP_TFTP_SERVER_NAME:
+		strncpy ( dhcphdr->sname, data, sizeof ( dhcphdr->sname ) );
+		return 0;
+	case DHCP_BOOTFILE_NAME:
+		strncpy ( dhcphdr->file, data, sizeof ( dhcphdr->file ) );
+		return 0;
 	default:
 		/* Continue processing as normal */
 		break;
 	}
 		
-	/* Set option in first available options block */
-	for ( ; options < &dhcppkt->options[NUM_OPT_BLOCKS] ; options++ ) {
-		option = set_dhcp_option ( options, tag, data, len );
-		if ( option )
-			break;
-	}
+	/* Set option */
+	option = set_dhcp_option ( options, tag, data, len );
 
 	/* Update DHCP packet length */
 	dhcppkt->len = ( offsetof ( typeof ( *dhcppkt->dhcphdr ), options )
-			 + dhcppkt->options[OPTS_MAIN].len );
+			 + dhcppkt->options.len );
 
 	return ( option ? 0 : -ENOSPC );
 }
@@ -296,8 +288,6 @@ int create_dhcp_packet ( struct net_device *netdev, uint8_t msgtype,
 			 void *data, size_t max_len,
 			 struct dhcp_packet *dhcppkt ) {
 	struct dhcphdr *dhcphdr = data;
-	static const uint8_t overloading = ( DHCP_OPTION_OVERLOAD_FILE |
-					     DHCP_OPTION_OVERLOAD_SNAME );
 	int rc;
 
 	/* Sanity check */
@@ -316,20 +306,10 @@ int create_dhcp_packet ( struct net_device *netdev, uint8_t msgtype,
 	/* Initialise DHCP packet structure */
 	dhcppkt->dhcphdr = dhcphdr;
 	dhcppkt->max_len = max_len;
-	init_dhcp_options ( &dhcppkt->options[OPTS_MAIN], dhcphdr->options,
+	init_dhcp_options ( &dhcppkt->options, dhcphdr->options,
 			    ( max_len -
 			      offsetof ( typeof ( *dhcphdr ), options ) ) );
-	init_dhcp_options ( &dhcppkt->options[OPTS_FILE], dhcphdr->file,
-			    sizeof ( dhcphdr->file ) );
-	init_dhcp_options ( &dhcppkt->options[OPTS_SNAME], dhcphdr->sname,
-			    sizeof ( dhcphdr->sname ) );
 	
-	/* Set DHCP_OPTION_OVERLOAD option within the main options block */
-	if ( set_dhcp_option ( &dhcppkt->options[OPTS_MAIN],
-			       DHCP_OPTION_OVERLOAD, &overloading,
-			       sizeof ( overloading ) ) == NULL )
-		return -ENOSPC;
-
 	/* Set DHCP_MESSAGE_TYPE option */
 	if ( ( rc = set_dhcp_packet_option ( dhcppkt, DHCP_MESSAGE_TYPE,
 					     &msgtype,