[U-Boot] [PATCH v2] net: More BOOTP retry timeout improvements

[Thierry Reding]

From: Thierry Reding <treding at nvidia.com>

It's not unusual for DHCP servers to take a couple hundred milliseconds
to respond to DHCP discover messages. One possible reason for the delay
can be that the server checks (typically using an ARP request) that the
IP it's about to hand out isn't in use yet. To make matters worse, some
servers may also queue up requests and process them sequentially, which
can cause excessively long delays if clients retry too fast.

Commit f59be6e850b3 ("net: BOOTP retry timeout improvements") shortened
the retry timeouts significantly, but the BOOTP/DHCP implementation in
U-Boot doesn't handle that well because it will ignore incoming replies
to earlier requests. In one particular setup this increases the time it
takes to obtain a DHCP lease from 630 ms to 8313 ms.

This commit attempts to fix this in two ways. First it increases the
initial retry timeout from 10 ms to 250 ms to give DHCP servers some
more time to respond. At the same time a cache of outstanding DHCP
request IDs is kept so that the implementation will know to continue
transactions even after a retransmission of the DISCOVER message. The
maximum retry timeout is also increased from 1 second to 2 seconds. An
ID cache of size 4 will keep DHCP requests around for 8 seconds (once
the maximum retry timeout has been reached) before dropping them. This
should give servers plenty of time to respond. If it ever turns out
that this isn't enough, the size of the cache can easily be increased.

With this commit the DHCP lease on the above-mentioned setup still takes
longer (1230 ms) than originally, but that's an acceptable compromise to
improve DHCP lease acquisition time for a broader range of setups.

To make it easier to benchmark DHCP in the future, this commit also adds
the time it took to obtain a lease to the final "DHCP client bound to
address x.x.x.x" message.

Tested-by: Stephen Warren <swarren at nvidia.com>
Signed-off-by: Thierry Reding <treding at nvidia.com>
---
Changes in v2:
- make boot ID cache size configurable

 README      | 18 ++++++++++++++++++
 net/bootp.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/README b/README
index 807b2d0ef0f9..2f62a19cf41c 100644
--- a/README
+++ b/README
@@ -2036,6 +2036,24 @@ CBFS (Coreboot Filesystem) support
 		4th and following
 		BOOTP requests:		delay 0 ... 8 sec
 
+		CONFIG_BOOTP_ID_CACHE_SIZE
+
+		BOOTP packets are uniquely identified using a 32-bit ID. The
+		server will copy the ID from client requests to responses and
+		U-Boot will use this to determine if it is the destination of
+		an incoming response. Some servers will check that addresses
+		aren't in use before handing them out (usually using an ARP
+		ping) and therefore take up to a few hundred milliseconds to
+		respond. Network congestion may also influence the time it
+		takes for a response to make it back to the client. If that
+		time is too long, U-Boot will retransmit requests. In order
+		to allow earlier responses to still be accepted after these
+		retransmissions, U-Boot's BOOTP client keeps a small cache of
+		IDs. The CONFIG_BOOTP_ID_CACHE_SIZE controls the size of this
+		cache. The default is to keep IDs for up to four outstanding
+		requests. Increasing this will allow U-Boot to accept offers
+		from a BOOTP client in networks with unusually high latency.
+
 - DHCP Advanced Options:
 		You can fine tune the DHCP functionality by defining
 		CONFIG_BOOTP_* symbols:
diff --git a/net/bootp.c b/net/bootp.c
index a4f6db570c6c..d4c86cf1794e 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -47,7 +47,12 @@
 #define CONFIG_DHCP_MIN_EXT_LEN 64
 #endif
 
-ulong		BootpID;
+#ifndef CONFIG_BOOTP_ID_CACHE_SIZE
+#define CONFIG_BOOTP_ID_CACHE_SIZE 4
+#endif
+
+ulong		bootp_ids[CONFIG_BOOTP_ID_CACHE_SIZE];
+unsigned int	bootp_num_ids;
 int		BootpTry;
 ulong		bootp_start;
 ulong		bootp_timeout;
@@ -77,6 +82,30 @@ static char *dhcpmsg2str(int type)
 #endif
 #endif
 
+static void bootp_add_id(ulong id)
+{
+	if (bootp_num_ids >= ARRAY_SIZE(bootp_ids)) {
+		size_t size = sizeof(bootp_ids) - sizeof(id);
+
+		memmove(bootp_ids, &bootp_ids[1], size);
+		bootp_ids[bootp_num_ids - 1] = id;
+	} else {
+		bootp_ids[bootp_num_ids] = id;
+		bootp_num_ids++;
+	}
+}
+
+static bool bootp_match_id(ulong id)
+{
+	unsigned int i;
+
+	for (i = 0; i < bootp_num_ids; i++)
+		if (bootp_ids[i] == id)
+			return true;
+
+	return false;
+}
+
 static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len)
 {
 	struct Bootp_t *bp = (struct Bootp_t *) pkt;
@@ -96,7 +125,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len)
 		retval = -4;
 	else if (bp->bp_hlen != HWL_ETHER)
 		retval = -5;
-	else if (NetReadLong((ulong *)&bp->bp_id) != BootpID)
+	else if (!bootp_match_id(NetReadLong((ulong *)&bp->bp_id)))
 		retval = -6;
 
 	debug("Filtering pkt = %d\n", retval);
@@ -351,8 +380,8 @@ BootpTimeout(void)
 #endif
 	} else {
 		bootp_timeout *= 2;
-		if (bootp_timeout > 1000)
-			bootp_timeout = 1000;
+		if (bootp_timeout > 2000)
+			bootp_timeout = 2000;
 		NetSetTimeout(bootp_timeout, BootpTimeout);
 		BootpRequest();
 	}
@@ -616,9 +645,10 @@ static int BootpExtended(u8 *e)
 
 void BootpReset(void)
 {
+	bootp_num_ids = 0;
 	BootpTry = 0;
 	bootp_start = get_timer(0);
-	bootp_timeout = 10;
+	bootp_timeout = 250;
 }
 
 void
@@ -631,6 +661,7 @@ BootpRequest(void)
 #ifdef CONFIG_BOOTP_RANDOM_DELAY
 	ulong rand_ms;
 #endif
+	ulong BootpID;
 
 	bootstage_mark_name(BOOTSTAGE_ID_BOOTP_START, "bootp_start");
 #if defined(CONFIG_CMD_DHCP)
@@ -699,7 +730,8 @@ BootpRequest(void)
 		| ((ulong)NetOurEther[4] << 8)
 		| (ulong)NetOurEther[5];
 	BootpID += get_timer(0);
-	BootpID	 = htonl(BootpID);
+	BootpID = htonl(BootpID);
+	bootp_add_id(BootpID);
 	NetCopyLong(&bp->bp_id, &BootpID);
 
 	/*
@@ -960,8 +992,8 @@ DhcpHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 			/* Store net params from reply */
 			BootpCopyNetParams(bp);
 			dhcp_state = BOUND;
-			printf("DHCP client bound to address %pI4\n",
-				&NetOurIP);
+			printf("DHCP client bound to address %pI4 (%lu ms)\n",
+				&NetOurIP, get_timer(bootp_start));
 			bootstage_mark_name(BOOTSTAGE_ID_BOOTP_STOP,
 				"bootp_stop");
 
-- 
2.0.4