From 4343e68779751605274e80336b56a2793f7482f0 Mon Sep 17 00:00:00 2001
From: Dean Camera <dean@fourwalledcubicle.com>
Date: Mon, 23 May 2011 07:13:45 +0000
Subject: [PATCH] Correct errors in the DHCP Server application in the
 Webserver project, that was causing random server restarts and/or incorrectly
 allocated IP addresses.

---
 Projects/Webserver/Lib/DHCPServerApp.c | 139 +++++++++++++++----------
 Projects/Webserver/Lib/DHCPServerApp.h |  13 +--
 2 files changed, 90 insertions(+), 62 deletions(-)

diff --git a/Projects/Webserver/Lib/DHCPServerApp.c b/Projects/Webserver/Lib/DHCPServerApp.c
index 84c57d0a2..7a36066cb 100644
--- a/Projects/Webserver/Lib/DHCPServerApp.c
+++ b/Projects/Webserver/Lib/DHCPServerApp.c
@@ -68,62 +68,83 @@ void DHCPServerApp_Callback(void)
 	DHCP_Header_t* const AppData     = (DHCP_Header_t*)uip_appdata;
 	uint16_t             AppDataSize = 0;
 
-	uint8_t DHCPMessageType;
-	if (!(DHCPCommon_GetOption(AppData->Options, DHCP_OPTION_MSG_TYPE, &DHCPMessageType)))
-		return;
-
-	uip_ipaddr_t Netmask, GatewayIPAddress;
-	struct uip_eth_addr RemoteMACAddress;
-	uint32_t            TransactionID;
+	/* Only process when new data arrives - don't retransmit lost packets */
+	if (uip_newdata())
+	{
+		/* Get the DHCP message type (if present), otherwise early-abort */
+		uint8_t DHCPMessageType;
+		if (!(DHCPCommon_GetOption(AppData->Options, DHCP_OPTION_MSG_TYPE, &DHCPMessageType)))
+			return;
 
-	memcpy(&RemoteMACAddress, &AppData->ClientHardwareAddress, sizeof(struct uip_eth_addr));
-	uip_getnetmask(&Netmask);
-	uip_getdraddr(&GatewayIPAddress);	
-	TransactionID = AppData->TransactionID;
+		uip_ipaddr_t        Netmask, GatewayIPAddress, PreferredClientIP;
+		struct uip_eth_addr RemoteMACAddress;
+		uint32_t            TransactionID;
 
-	switch (DHCPMessageType)
-	{
-		case DHCP_DISCOVER:
-			AppDataSize += DHCPServerApp_FillDHCPHeader(AppData, DHCP_OFFER, &RemoteMACAddress, TransactionID);
+		/* Get configured network mask, gateway IP and extract out DHCP transaction ID and remote IP */
+		uip_getnetmask(&Netmask);
+		uip_getdraddr(&GatewayIPAddress);
+		memcpy(&RemoteMACAddress, &AppData->ClientHardwareAddress, sizeof(struct uip_eth_addr));
+		TransactionID = AppData->TransactionID;
 
-			AppDataSize += DHCPCommon_SetOption(AppData->Options, DHCP_OPTION_SUBNET_MASK,
-			                                    sizeof(uip_ipaddr_t), &Netmask);
-			AppDataSize += DHCPCommon_SetOption(AppData->Options, DHCP_OPTION_ROUTER,
-			                                    sizeof(uip_ipaddr_t), &GatewayIPAddress);
-			
-			/* Send the DHCP OFFER packet */
-			uip_poll_conn(BroadcastConnection);
-			memcpy(&uip_udp_conn->ripaddr, &uip_broadcast_addr, sizeof(uip_ipaddr_t));
-			uip_udp_send(AppDataSize);
+		/* Try to extract out the client's preferred IP address if it is indicated in the packet */
+		if (!(DHCPCommon_GetOption(AppData->Options, DHCP_OPTION_REQ_IPADDR, &PreferredClientIP)))
+		  memcpy(&PreferredClientIP, &uip_all_zeroes_addr, sizeof(uip_ipaddr_t));	
+		
+		switch (DHCPMessageType)
+		{
+			case DHCP_DISCOVER:
+				/* If no preference was made or the preferred IP is already taken, find a new address */
+				if (DHCPServerApp_CheckIfIPLeased(&PreferredClientIP))
+				  DHCPServerApp_GetUnleasedIP(&PreferredClientIP);
 
-			break;
-		case DHCP_REQUEST:
-			if (!(DHCPServerApp_CheckIfIPLeased(&AppData->YourIP)))
-			{
-				AppDataSize += DHCPServerApp_FillDHCPHeader(AppData, DHCP_ACK, &RemoteMACAddress, TransactionID);
+				/* Create a new DHCP OFFER packet with the offered IP address */
+				AppDataSize += DHCPServerApp_FillDHCPHeader(AppData, DHCP_OFFER, &RemoteMACAddress, &PreferredClientIP, TransactionID);
 
+				/* Add network mask and router information to the list of DHCP OFFER packet options */
 				AppDataSize += DHCPCommon_SetOption(AppData->Options, DHCP_OPTION_SUBNET_MASK,
 													sizeof(uip_ipaddr_t), &Netmask);
 				AppDataSize += DHCPCommon_SetOption(AppData->Options, DHCP_OPTION_ROUTER,
-													sizeof(uip_ipaddr_t), &GatewayIPAddress);
-
-				DHCPServerApp_LeaseIP(&AppData->YourIP);
-			}
-			else
-			{
-				AppDataSize += DHCPServerApp_FillDHCPHeader(AppData, DHCP_NAK, &RemoteMACAddress, TransactionID);			
-			}
+					                                sizeof(uip_ipaddr_t), &GatewayIPAddress);
+
+				/* Send the DHCP OFFER packet */
+				uip_poll_conn(BroadcastConnection);
+				memcpy(&uip_udp_conn->ripaddr, &uip_broadcast_addr, sizeof(uip_ipaddr_t));
+				uip_udp_send(AppDataSize);
+
+				break;
+			case DHCP_REQUEST:
+				/* Check to see if the requested IP address has already been leased to a client */
+				if (!(DHCPServerApp_CheckIfIPLeased(&PreferredClientIP)))
+				{
+					/* Create a new DHCP ACK packet to accept the IP address lease */					
+					AppDataSize += DHCPServerApp_FillDHCPHeader(AppData, DHCP_ACK, &RemoteMACAddress, &PreferredClientIP, TransactionID);
+
+					/* Add network mask and router information to the list of DHCP ACK packet options */
+					AppDataSize += DHCPCommon_SetOption(AppData->Options, DHCP_OPTION_SUBNET_MASK,
+														sizeof(uip_ipaddr_t), &Netmask);
+					AppDataSize += DHCPCommon_SetOption(AppData->Options, DHCP_OPTION_ROUTER,
+					                                    sizeof(uip_ipaddr_t), &GatewayIPAddress);
+
+					/* Mark the requested IP as leased to a client */
+					DHCPServerApp_LeaseIP(&PreferredClientIP);
+				}
+				else
+				{
+					/* Create a new DHCP NAK packet to reject the requested allocation */
+					AppDataSize += DHCPServerApp_FillDHCPHeader(AppData, DHCP_NAK, &RemoteMACAddress, &uip_all_zeroes_addr, TransactionID);
+				}
+				
+				/* Send the DHCP ACK or NAK packet */
+				uip_poll_conn(BroadcastConnection);
+				memcpy(&uip_udp_conn->ripaddr, &uip_broadcast_addr, sizeof(uip_ipaddr_t));
+				uip_udp_send(AppDataSize);
 			
-			/* Send the DHCP ACK or NAK packet */
-			uip_poll_conn(BroadcastConnection);
-			memcpy(&uip_udp_conn->ripaddr, &uip_broadcast_addr, sizeof(uip_ipaddr_t));
-			uip_udp_send(AppDataSize);
-		
-			break;
-		case DHCP_RELEASE:
-			/* Mark the IP address as released in the allocation table */
-			DHCPServerApp_UnleaseIP(&uip_udp_conn->ripaddr);
-			break;
+				break;
+			case DHCP_RELEASE:
+				/* Mark the IP address as released in the allocation table */
+				DHCPServerApp_UnleaseIP(&uip_udp_conn->ripaddr);
+				break;
+		}
 	}
 }
 
@@ -139,8 +160,9 @@ void DHCPServerApp_Callback(void)
  */
 static uint16_t DHCPServerApp_FillDHCPHeader(DHCP_Header_t* const DHCPHeader,
                                              const uint8_t DHCPMessageType,
-                                             struct uip_eth_addr* ClientHardwareAddress,
-                                             uint32_t TransactionID)
+                                             const struct uip_eth_addr* const ClientHardwareAddress,
+											 const uip_ipaddr_t* const PreferredClientIP,
+                                             const uint32_t TransactionID)
 {
 	/* Erase existing packet data so that we start will all 0x00 DHCP header data */
  	memset(DHCPHeader, 0, sizeof(DHCP_Header_t));
@@ -153,8 +175,7 @@ static uint16_t DHCPServerApp_FillDHCPHeader(DHCP_Header_t* const DHCPHeader,
 	DHCPHeader->ElapsedSeconds        = 0;
 	DHCPHeader->Flags                 = 0;
 	memcpy(&DHCPHeader->NextServerIP, &uip_hostaddr, sizeof(uip_ipaddr_t));
-	if (uip_ipaddr_cmp(&DHCPHeader->YourIP, &uip_all_zeroes_addr))
-	  DHCPServerApp_GetUnleasedIP(&DHCPHeader->YourIP);
+	memcpy(&DHCPHeader->YourIP, PreferredClientIP, sizeof(uip_ipaddr_t));
 	memcpy(&DHCPHeader->ClientHardwareAddress, ClientHardwareAddress, sizeof(struct uip_eth_addr));
 	DHCPHeader->Cookie                = DHCP_MAGIC_COOKIE;
 	  
@@ -176,12 +197,13 @@ static uint16_t DHCPServerApp_FillDHCPHeader(DHCP_Header_t* const DHCPHeader,
  *
  *  \return Boolean true if the IP has already been leased to a client, false otherwise.
  */
-static bool DHCPServerApp_CheckIfIPLeased(uip_ipaddr_t* IPAddress)
+static bool DHCPServerApp_CheckIfIPLeased(const uip_ipaddr_t* const IPAddress)
 {
 	uint8_t Byte = (IPAddress->u8[3] / 8);
 	uint8_t Mask = (1 << (IPAddress->u8[3] % 8));
 	
-	if (!(IPAddress->u8[3] == uip_hostaddr.u8[3]) && !(LeasedIPs[Byte] & Mask))
+	/* Make sure that the requested IP address isn't already leased to the virtual server or another client */
+	if (IPAddress->u8[3] && !(IPAddress->u8[3] == uip_hostaddr.u8[3]) && !(LeasedIPs[Byte] & Mask))
 	  return false;
 	else
 	  return true;
@@ -191,14 +213,17 @@ static bool DHCPServerApp_CheckIfIPLeased(uip_ipaddr_t* IPAddress)
  *
  *  \param[out] NewIPAddress  Location where the generated IP Address should be stored
  */
-static void DHCPServerApp_GetUnleasedIP(uip_ipaddr_t* NewIPAddress)
+static void DHCPServerApp_GetUnleasedIP(uip_ipaddr_t* const NewIPAddress)
 {
 	uip_ipaddr_copy(NewIPAddress, &uip_hostaddr);
 	
+	/** Look through the current subnet, skipping the broadcast and zero IP addresses */
 	for (uint8_t IP = 1; IP < 254; IP++)
 	{
+		/* Update new IP address to lease with the current IP address to test */
 		NewIPAddress->u8[3] = IP;
 		
+		/* If we've found an unleased IP, abort with the updated IP stored for the called */
 		if (!(DHCPServerApp_CheckIfIPLeased(NewIPAddress)))
 		  return;
 	}
@@ -211,11 +236,12 @@ static void DHCPServerApp_GetUnleasedIP(uip_ipaddr_t* NewIPAddress)
  *
  *  \pre The IP address must be within the same /24 subnet as the virtual webserver.
  */
-static void DHCPServerApp_LeaseIP(uip_ipaddr_t* IPAddress)
+static void DHCPServerApp_LeaseIP(const uip_ipaddr_t* const IPAddress)
 {
 	uint8_t Byte = (IPAddress->u8[3] / 8);
 	uint8_t Mask = (1 << (IPAddress->u8[3] % 8));
 	
+	/* Mark the IP address as leased in the allocation table */
 	LeasedIPs[Byte] |= Mask;
 }
 
@@ -226,11 +252,12 @@ static void DHCPServerApp_LeaseIP(uip_ipaddr_t* IPAddress)
  *
  *  \pre The IP address must be within the same /24 subnet as the virtual webserver.
  */
-static void DHCPServerApp_UnleaseIP(uip_ipaddr_t* IPAddress)
+static void DHCPServerApp_UnleaseIP(const uip_ipaddr_t* const IPAddress)
 {
 	uint8_t Byte = (IPAddress->u8[3] / 8);
 	uint8_t Mask = (1 << (IPAddress->u8[3] % 8));
 	
+	/* Mark the IP address as unleased in the allocation table */
 	LeasedIPs[Byte] &= ~Mask;
 }
 #endif
diff --git a/Projects/Webserver/Lib/DHCPServerApp.h b/Projects/Webserver/Lib/DHCPServerApp.h
index e70869588..9d91d6be8 100644
--- a/Projects/Webserver/Lib/DHCPServerApp.h
+++ b/Projects/Webserver/Lib/DHCPServerApp.h
@@ -51,12 +51,13 @@
 		#if defined(INCLUDE_FROM_DHCPSERVERAPP_C)
 		static uint16_t DHCPServerApp_FillDHCPHeader(DHCP_Header_t* const DHCPHeader,
 		                                             const uint8_t DHCPMessageType,
-		                                             struct uip_eth_addr* ClientHardwareAddress,
-		                                             uint32_t TransactionID);
-		static bool DHCPServerApp_CheckIfIPLeased(uip_ipaddr_t* IPAddress);
-		static void DHCPServerApp_GetUnleasedIP(uip_ipaddr_t* NewIPAddress);
-		static void DHCPServerApp_LeaseIP(uip_ipaddr_t* IPAddress);
-		static void DHCPServerApp_UnleaseIP(uip_ipaddr_t* IPAddress);
+		                                             const struct uip_eth_addr* const ClientHardwareAddress,
+		                                             const uip_ipaddr_t* const PreferredClientIP,
+		                                             const uint32_t TransactionID);
+		static bool DHCPServerApp_CheckIfIPLeased(const uip_ipaddr_t* const IPAddress);
+		static void DHCPServerApp_GetUnleasedIP(uip_ipaddr_t* const NewIPAddress);
+		static void DHCPServerApp_LeaseIP(const uip_ipaddr_t* const IPAddress);
+		static void DHCPServerApp_UnleaseIP(const uip_ipaddr_t* const IPAddress);
 		#endif
 #endif
 
-- 
GitLab