[ipxe-devel] [IPXE PATCH] [dhcp] Use random transaction ID to associate messages

[Amos Kong]

RFC2131.txt:
xid 4  Transaction ID, a random number chosen by the
       client, used by the client and server to associate
       messages and responses between a client and a
       server.
The 'xid' field is used by the client to match incoming DHCP messages
with pending requests.  A DHCP client MUST choose 'xid's in such a
way as to minimize the chance of using an 'xid' identical to one used
by another client. For example, a client may choose a different,
random initial 'xid' each time the client is rebooted, and
subsequently use sequential 'xid's until the next reboot.  Selecting
a new 'xid' for each retransmission is an implementation decision.  A
client may choose to reuse the same 'xid' or select a new 'xid' for
each retransmitted message.

Users may boot up a QEMU guest without default mac address, it's easy to
repeat, it always to be failed to get IP with PXE boot.
RFC suggests us to use random xid, it's necessary. This patch generates
random id when start dhcp, and record it to netdev struct.

Signed-off-by: Amos Kong <akong at redhat.com>
CC: Eduardo Habkost <ehabkost at redhat.com>
CC: Michael Brown <mcb30 at ipxe.org>
---
 src/include/ipxe/netdevice.h |    3 +++
 src/net/udp/dhcp.c           |   23 ++++-------------------
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/src/include/ipxe/netdevice.h b/src/include/ipxe/netdevice.h
index 3633a16..cd68dd0 100644
--- a/src/include/ipxe/netdevice.h
+++ b/src/include/ipxe/netdevice.h
@@ -319,6 +319,9 @@ struct net_device {
 	/** Link-layer broadcast address */
 	const uint8_t *ll_broadcast;
 
+	/* DHCP Transaction ID */
+	uint32_t xid;
+
 	/** Current device state
 	 *
 	 * This is the bitwise-OR of zero or more NETDEV_XXX constants.
diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index a0b74df..ed104a7 100644
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -137,23 +137,6 @@ static inline const char * dhcp_msgtype_name ( unsigned int msgtype ) {
 	}
 }
 
-/**
- * Calculate DHCP transaction ID for a network device
- *
- * @v netdev		Network device
- * @ret xid		DHCP XID
- *
- * Extract the least significant bits of the hardware address for use
- * as the transaction ID.
- */
-static uint32_t dhcp_xid ( struct net_device *netdev ) {
-	uint32_t xid;
-
-	memcpy ( &xid, ( netdev->ll_addr + netdev->ll_protocol->ll_addr_len
-			 - sizeof ( xid ) ), sizeof ( xid ) );
-	return xid;
-}
-
 /****************************************************************************
  *
  * DHCP session
@@ -938,7 +921,7 @@ int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
 
 	/* Initialise DHCP packet content */
 	memset ( dhcphdr, 0, max_len );
-	dhcphdr->xid = dhcp_xid ( netdev );
+	dhcphdr->xid = netdev->xid;
 	dhcphdr->magic = htonl ( DHCP_MAGIC_COOKIE );
 	dhcphdr->htype = ntohs ( netdev->ll_protocol->ll_proto );
 	dhcphdr->op = dhcp_op[msgtype];
@@ -1187,7 +1170,8 @@ static int dhcp_deliver ( struct dhcp_session *dhcp,
 			&server_id, sizeof ( server_id ) );
 
 	/* Check for matching transaction ID */
-	if ( dhcphdr->xid != dhcp_xid ( dhcp->netdev ) ) {
+	if ( dhcphdr->xid != dhcp->netdev->xid ) {
+
 		DBGC ( dhcp, "DHCP %p %s from %s:%d has bad transaction "
 		       "ID\n", dhcp, dhcp_msgtype_name ( msgtype ),
 		       inet_ntoa ( peer->sin_addr ),
@@ -1304,6 +1288,7 @@ int start_dhcp ( struct interface *job, struct net_device *netdev ) {
 	dhcp = zalloc ( sizeof ( *dhcp ) );
 	if ( ! dhcp )
 		return -ENOMEM;
+	netdev->xid = random();
 	ref_init ( &dhcp->refcnt, dhcp_free );
 	intf_init ( &dhcp->job, &dhcp_job_desc, &dhcp->refcnt );
 	intf_init ( &dhcp->xfer, &dhcp_xfer_desc, &dhcp->refcnt );