[git commit] udhcpd: send DHCPOFFERs as unicast (unless clients specifically asks for bcast)

[Denys Vlasenko]

commit: https://git.busybox.net/busybox/commit/?id=d2d23c848acd6f336d9b0edae613eef443a6de20
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

RFC 2131 says we should do that.
Evidently, since for so many years no one complained, sending them broadcast
works too, but finally we've got someone who wants RFC-compliand behavior.

function                                             old     new   delta
send_packet                                          141     179     +38
.rodata                                           105680  105681      +1
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 39/0)               Total: 39 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 networking/udhcp/dhcpd.c | 58 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 16 deletions(-)

diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c
index 2904119e5..b9cbd6464 100644
--- a/networking/udhcp/dhcpd.c
+++ b/networking/udhcp/dhcpd.c
@@ -575,29 +575,51 @@ static void send_packet_to_client(struct dhcp_packet *dhcp_pkt, int force_broadc
 	const uint8_t *chaddr;
 	uint32_t ciaddr;
 
-	// Was:
+	// Logic:
 	//if (force_broadcast) { /* broadcast */ }
 	//else if (dhcp_pkt->ciaddr) { /* unicast to dhcp_pkt->ciaddr */ }
+	// ^^^ dhcp_pkt->ciaddr comes from client's request packet.
+	// We expect such clients to have an UDP socket listening on that IP.
 	//else if (dhcp_pkt->flags & htons(BROADCAST_FLAG)) { /* broadcast */ }
 	//else { /* unicast to dhcp_pkt->yiaddr */ }
-	// But this is wrong: yiaddr is _our_ idea what client's IP is
-	// (for example, from lease file). Client may not know that,
-	// and may not have UDP socket listening on that IP!
-	// We should never unicast to dhcp_pkt->yiaddr!
-	// dhcp_pkt->ciaddr, OTOH, comes from client's request packet,
-	// and can be used.
-
-	if (force_broadcast
-	 || (dhcp_pkt->flags & htons(BROADCAST_FLAG))
-	 || dhcp_pkt->ciaddr == 0
+	// ^^^ The last case is confusing, but *should* work.
+	// It's a case where client have sent a DISCOVER
+	// and does not have a kernel UDP socket listening on the IP
+	// we are offering in yiaddr (it does not know the IP yet)!
+	// This *should* work because client *should* listen on a raw socket
+	// instead at this time (IOW: it should examine ALL IPv4 packets
+	// "by hand", not relying on kernel's UDP stack.)
+
+	chaddr = dhcp_pkt->chaddr;
+
+	if (dhcp_pkt->ciaddr == 0
+	 || force_broadcast /* sending DHCPNAK pkt? */
 	) {
-		log1s("broadcasting packet to client");
-		ciaddr = INADDR_BROADCAST;
-		chaddr = MAC_BCAST_ADDR;
+		if (dhcp_pkt->flags & htons(BROADCAST_FLAG)
+		 || force_broadcast /* sending DHCPNAK pkt? */
+		) {
+// RFC 2131:
+// If 'giaddr' is zero and 'ciaddr' is zero, and the broadcast bit is
+// set, then the server broadcasts DHCPOFFER and DHCPACK messages to
+// 0xffffffff. ...
+// In all cases, when 'giaddr' is zero, the server broadcasts any DHCPNAK
+// messages to 0xffffffff.
+			ciaddr = INADDR_BROADCAST;
+			chaddr = MAC_BCAST_ADDR;
+			log1s("broadcasting packet to client");
+		} else {
+// If the broadcast bit is not set and 'giaddr' is zero and
+// 'ciaddr' is zero, then the server unicasts DHCPOFFER and DHCPACK
+// messages to the client's hardware address and 'yiaddr' address.
+			ciaddr = dhcp_pkt->yiaddr;
+			log1("unicasting packet to client %ciaddr", 'y');
+		}
 	} else {
-		log1s("unicasting packet to client ciaddr");
+// If the 'giaddr'
+// field is zero and the 'ciaddr' field is nonzero, then the server
+// unicasts DHCPOFFER and DHCPACK messages to the address in 'ciaddr'.
 		ciaddr = dhcp_pkt->ciaddr;
-		chaddr = dhcp_pkt->chaddr;
+		log1("unicasting packet to client %ciaddr", 'c');
 	}
 
 	udhcp_send_raw_packet(dhcp_pkt,
@@ -624,6 +646,10 @@ static void send_packet_to_relay(struct dhcp_packet *dhcp_pkt)
 static void send_packet(struct dhcp_packet *dhcp_pkt, int force_broadcast)
 {
 	if (dhcp_pkt->gateway_nip)
+// RFC 2131:
+// If the 'giaddr' field in a DHCP message from a client is non-zero,
+// the server sends any return messages to the 'DHCP server' port on the
+// BOOTP relay agent whose address appears in 'giaddr'.
 		send_packet_to_relay(dhcp_pkt);
 	else
 		send_packet_to_client(dhcp_pkt, force_broadcast);