servers/net: Make DHCP client more RFC 2131 compliant

* The client should enter state REBINDING only when RENEWING times
   out.
 * When in RENEWING or REBINDING state do not give up retrying
   unless the lease expires.
 * Fix bug sending 2^n DHCP requests at n-th lease renewal.
 * Use timeout values and renewal/rebinding times suggested by the
   RFC.
 * Use different XIDs in subsequent transactions.

src/servers/net/DHCPClient.cpp

@@ -16,6 +16,7 @@
 #include <NetworkDevice.h>
 #include <NetworkInterface.h>
 
+#include <algorithm>
 #include <arpa/inet.h>
 #include <errno.h>
 #include <stdio.h>
@@ -38,8 +39,10 @@
 #define DHCP_CLIENT_PORT	68
 #define DHCP_SERVER_PORT	67
 
-#define DEFAULT_TIMEOUT		2	// secs
+#define DEFAULT_TIMEOUT		4	// secs
-#define MAX_TIMEOUT			15	// secs
+#define MAX_TIMEOUT			64	// secs
+
+#define MAX_RETRIES			5
 
 enum message_opcode {
 	BOOT_REQUEST = 1,
@@ -427,10 +430,7 @@ DHCPClient::DHCPClient(BMessenger target, const char* device)
 	fServer(AF_INET, NULL, DHCP_SERVER_PORT),
 	fLeaseTime(0)
 {
-	fStartTime = system_time();
+	fTransactionID = (uint32)system_time() ^ rand();
-	fTransactionID = (uint32)fStartTime;
-
-	srand(fTransactionID);
 
 	BNetworkAddress link;
 	BNetworkInterface interface(device);
@@ -498,13 +498,22 @@ DHCPClient::Initialize()
 status_t
 DHCPClient::_Negotiate(dhcp_state state)
 {
+	if (state == BOUND)
+		return B_OK;
+
+	fStartTime = system_time();
+	fTransactionID++;
+
+	char hostName[MAXHOSTNAMELEN];
+	if (gethostname(hostName, MAXHOSTNAMELEN) == 0)
+		fHostName.SetTo(hostName, MAXHOSTNAMELEN);
+	else
+		fHostName.Truncate(0);
+
 	int socket = ::socket(AF_INET, SOCK_DGRAM, 0);
 	if (socket < 0)
 		return errno;
 
-	BNetworkAddress local;
-	local.SetToWildcard(AF_INET, DHCP_CLIENT_PORT);
-
 	// Enable reusing the port. This is needed in case there is more
 	// than 1 interface that needs to be configured. Note that the only reason
 	// this works is because there is code below to bind to a specific
@@ -512,108 +521,69 @@ DHCPClient::_Negotiate(dhcp_state state)
 	int option = 1;
 	setsockopt(socket, SOL_SOCKET, SO_REUSEPORT, &option, sizeof(option));
 
+	BNetworkAddress local;
+	local.SetToWildcard(AF_INET, DHCP_CLIENT_PORT);
+
+	option = 1;
+	setsockopt(socket, SOL_SOCKET, SO_BROADCAST, &option, sizeof(option));
+
 	if (bind(socket, local, local.Length()) < 0) {
 		close(socket);
 		return errno;
 	}
 
-	BNetworkAddress broadcast;
-	broadcast.SetToBroadcast(AF_INET, DHCP_SERVER_PORT);
-
-	option = 1;
-	setsockopt(socket, SOL_SOCKET, SO_BROADCAST, &option, sizeof(option));
-
-	if (state == INIT) {
-		// The local interface does not have an address yet, bind the socket
-		// to the device directly.
-		BNetworkDevice device(Device());
-		int index = device.Index();
-
-		setsockopt(socket, SOL_SOCKET, SO_BINDTODEVICE, &index, sizeof(int));
-	}
-
 	bigtime_t previousLeaseTime = fLeaseTime;
-	fLeaseTime = 0;
-	fRenewalTime = 0;
-	fRebindingTime = 0;
-
-	status_t status = B_ERROR;
-	time_t timeout;
-	uint32 tries;
-	_ResetTimeout(socket, timeout, tries);
-
-	char hostName[MAXHOSTNAMELEN];
-	if (gethostname(hostName, MAXHOSTNAMELEN) == 0)
-		fHostName.SetTo(hostName, MAXHOSTNAMELEN);
-	else
-		fHostName.Truncate(0);
-
-	dhcp_message discover(DHCP_DISCOVER);
-	_PrepareMessage(discover, state);
-
-	dhcp_message request(DHCP_REQUEST);
-	_PrepareMessage(request, state);
-
-	// send discover/request message
-	_SendMessage(socket, state == INIT ? discover : request,
-		state != RENEWING ? broadcast : fServer);
-		// no need to check the status; in case of an error we'll just send
-		// the message again
-
-	// receive loop until we've got an offer and acknowledged it
 
+	status_t status = B_OK;
 	while (state != BOUND) {
-		char buffer[2048];
+		status = _StateTransition(socket, state);
-		struct sockaddr_in from;
+		if (status != B_OK && (state == SELECTING || state == REBOOTING))
-		socklen_t fromLength = sizeof(from);
+			break;
-		ssize_t bytesReceived = recvfrom(socket, buffer, sizeof(buffer),
+	}
-			0, (struct sockaddr*)&from, &fromLength);
+
-		if (bytesReceived < 0 && errno == B_TIMED_OUT) {
-			// depending on the state, we'll just try again
-			if (!_TimeoutShift(socket, timeout, tries)) {
 	close(socket);
-				return B_TIMED_OUT;
+
+	if (fLeaseTime == 0)
+		fLeaseTime = previousLeaseTime;
+	if (fLeaseTime == 0)
+		fLeaseTime = 60;
+
+	if (fRenewalTime == 0)
+		fRenewalTime = fLeaseTime / 2;
+	if (fRebindingTime == 0)
+		fRebindingTime = fLeaseTime * 7 / 8;
+	fLeaseTime += fRequestTime;
+	fRenewalTime += fRequestTime;
+	fRebindingTime += fRequestTime;
+	_RestartLease(fRenewalTime);
+
+	fStatus = status;
+	if (status)
+		return status;
+
+	// configure interface
+	BMessage reply;
+	status = Target().SendMessage(&fConfiguration, &reply);
+	if (status == B_OK)
+		status = reply.FindInt32("status", &fStatus);
+
+	// configure resolver
+	reply.MakeEmpty();
+	fResolverConfiguration.AddString("device", Device());
+	status = Target().SendMessage(&fResolverConfiguration, &reply);
+	if (status == B_OK)
+		status = reply.FindInt32("status", &fStatus);
+	return status;
 }
 
-			_SendMessage(socket, state == INIT ? discover : request,
-				state != RENEWING ? broadcast : fServer);
-			continue;
 
-		} else if (bytesReceived < 0)
+status_t
-			break;
+DHCPClient::_GotMessage(dhcp_state& state, dhcp_message* message)
-
-		dhcp_message* message = (dhcp_message*)buffer;
-		if (message->transaction_id != htonl(fTransactionID)
-			|| !message->HasOptions()
-			|| memcmp(message->mac_address, discover.mac_address,
-				discover.hardware_address_length)) {
-			// this message is not for us
-			continue;
-		}
-
-		// advance from startup state
-		if (state == INIT)
-			state = SELECTING;
-		else if (state == INIT_REBOOT)
-			state = REBOOTING;
-
-		syslog(LOG_DEBUG, "%s: Received %s from %s\n",
-			Device(), dhcp_message::TypeToString(message->Type()),
-			_AddressToString(from.sin_addr.s_addr).String());
-
-		switch (message->Type()) {
-			case DHCP_NONE:
-			default:
-				// ignore this message
-				break;
-
-			case DHCP_OFFER:
 {
-				// first offer wins
+	switch (state) {
-				if (state != SELECTING)
+		case SELECTING:
-					break;
+			if (message->Type() == DHCP_OFFER) {
-
+				state = REQUESTING;
-				// collect interface options
 
 				fAssignedAddress = message->your_address;
 				syslog(LOG_INFO, "  your_address: %s\n",
@@ -630,27 +600,16 @@ DHCPClient::_Negotiate(dhcp_state state)
 				_ParseOptions(*message, address, fResolverConfiguration);
 
 				fConfiguration.AddMessage("address", &address);
-
+				return B_OK;
-				// request configuration from the server
-
-				_ResetTimeout(socket, timeout, tries);
-				state = REQUESTING;
-				_PrepareMessage(request, state);
-
-				status = _SendMessage(socket, request, broadcast);
-					// we're sending a broadcast so that all potential offers
-					// get an answer
-				break;
 			}
 
-			case DHCP_ACK:
+			return B_BAD_VALUE;
-			{
-				if (state != REQUESTING
-					&& state != REBOOTING
-					&& state != REBINDING
-					&& state != RENEWING)
-					continue;
 
+		case REBOOTING:
+		case REBINDING:
+		case RENEWING:
+		case REQUESTING:
+			if (message->Type() == DHCP_ACK) {
 				// TODO: we might want to configure the stuff, don't we?
 				BMessage address;
 				fResolverConfiguration.MakeEmpty();
@@ -661,67 +620,96 @@ DHCPClient::_Negotiate(dhcp_state state)
 				// our address request has been acknowledged
 				state = BOUND;
 
-				// configure interface
+				return B_OK;
-				BMessage reply;
-				status = Target().SendMessage(&fConfiguration, &reply);
-				if (status == B_OK)
-					status = reply.FindInt32("status", &fStatus);
-
-				// configure resolver
-				reply.MakeEmpty();
-				fResolverConfiguration.AddString("device", Device());
-				status = Target().SendMessage(&fResolverConfiguration, &reply);
-				if (status == B_OK)
-					status = reply.FindInt32("status", &fStatus);
-				break;
 			}
 
-			case DHCP_NACK:
+			if (message->Type() == DHCP_NACK) {
-				if (state != REQUESTING
-					&& state != REBOOTING
-					&& state != REBINDING
-					&& state != RENEWING)
-					continue;
-
-				if (state == REBOOTING) {
 				// server reject our request on previous assigned address
 				// back to square one...
 				fAssignedAddress = 0;
+				state = INIT;
+				return B_OK;
 			}
 
-				// try again (maybe we should prefer other servers if this
+		default:
-				// happens more than once)
+			return B_BAD_VALUE;
-				status = _SendMessage(socket, discover, broadcast);
+	}
-				if (status == B_OK)
+}
-					state = INIT;
+
+
+status_t
+DHCPClient::_StateTransition(int socket, dhcp_state& state)
+{
+	if (state == INIT) {
+		// The local interface does not have an address yet, bind the socket
+		// to the device directly.
+		BNetworkDevice device(Device());
+		int index = device.Index();
+
+		setsockopt(socket, SOL_SOCKET, SO_BINDTODEVICE, &index, sizeof(int));
+	}
+
+	BNetworkAddress broadcast;
+	broadcast.SetToBroadcast(AF_INET, DHCP_SERVER_PORT);
+
+	time_t timeout;
+	uint32 tries;
+	_ResetTimeout(socket, state, timeout, tries);
+
+	dhcp_message discover(DHCP_DISCOVER);
+	_PrepareMessage(discover, state);
+
+	dhcp_message request(DHCP_REQUEST);
+	_PrepareMessage(request, state);
+
+	bool skipRequest = false;
+	dhcp_state originalState = state;
+	fRequestTime = system_time();
+	while (true) {
+		if (!skipRequest) {
+			_SendMessage(socket, originalState == INIT ? discover : request,
+				originalState == RENEWING ? fServer : broadcast);
+
+			if (originalState == INIT)
+				state = SELECTING;
+			else if (originalState == INIT_REBOOT)
+				state = REBOOTING;
+		}
+
+		char buffer[2048];
+		struct sockaddr_in from;
+		socklen_t fromLength = sizeof(from);
+
+		ssize_t bytesReceived = recvfrom(socket, buffer, sizeof(buffer),
+			0, (struct sockaddr*)&from, &fromLength);
+		if (bytesReceived < 0 && errno == B_TIMED_OUT) {
+			// depending on the state, we'll just try again
+			if (!_TimeoutShift(socket, state, timeout, tries))
+				return B_TIMED_OUT;
+			skipRequest = false;
+			continue;
+		} else if (bytesReceived < 0)
+			return errno;
+
+		skipRequest = true;
+		dhcp_message* message = (dhcp_message*)buffer;
+		if (message->transaction_id != htonl(fTransactionID)
+			|| !message->HasOptions()
+			|| memcmp(message->mac_address, discover.mac_address,
+				discover.hardware_address_length)) {
+			// this message is not for us
+			continue;
+		}
+
+		syslog(LOG_DEBUG, "%s: Received %s from %s\n",
+			Device(), dhcp_message::TypeToString(message->Type()),
+			_AddressToString(from.sin_addr.s_addr).String());
+
+		if (_GotMessage(state, message) == B_OK)
 			break;
 	}
-	}
 
-	close(socket);
+	return B_OK;
-
-	if (status == B_OK && fLeaseTime > 0) {
-		// notify early enough when the lease is
-		if (fRenewalTime == 0)
-			fRenewalTime = fLeaseTime * 2/3;
-		if (fRebindingTime == 0)
-			fRebindingTime = fLeaseTime * 5/6;
-
-		bigtime_t now = system_time();
-		_RestartLease(fRenewalTime);
-
-		fLeaseTime += now;
-		fRenewalTime += now;
-		fRebindingTime += now;
-			// make lease times absolute
-	} else {
-		fLeaseTime = previousLeaseTime;
-		bigtime_t now = system_time();
-		fRenewalTime = (fLeaseTime - now) * 2/3 + now;
-		fRebindingTime = (fLeaseTime - now) * 5/6 + now;
-	}
-
-	return status;
 }
 
 
@@ -732,7 +720,7 @@ DHCPClient::_RestartLease(bigtime_t leaseTime)
 		return;
 
 	BMessage lease(kMsgLeaseTime);
-	fRunner = new BMessageRunner(this, &lease, leaseTime, 1);
+	fRunner = new BMessageRunner(this, &lease, leaseTime - system_time(), 1);
 }
 
 
@@ -899,7 +887,8 @@ DHCPClient::_PrepareMessage(dhcp_message& message, dhcp_state state)
 
 
 void
-DHCPClient::_ResetTimeout(int socket, time_t& timeout, uint32& tries)
+DHCPClient::_ResetTimeout(int socket, dhcp_state& state, time_t& timeout,
+	uint32& tries)
 {
 	timeout = DEFAULT_TIMEOUT;
 	tries = 0;
@@ -912,15 +901,36 @@ DHCPClient::_ResetTimeout(int socket, time_t& timeout, uint32& tries)
 
 
 bool
-DHCPClient::_TimeoutShift(int socket, time_t& timeout, uint32& tries)
+DHCPClient::_TimeoutShift(int socket, dhcp_state& state, time_t& timeout,
+	uint32& tries)
 {
-	timeout += timeout;
+	if (state == RENEWING && system_time() > fRebindingTime) {
-	if (timeout > MAX_TIMEOUT) {
+		state = REBINDING;
-		timeout = DEFAULT_TIMEOUT;
-
-		if (++tries > 2)
 		return false;
 	}
+
+	if (state == REBINDING && system_time() > fLeaseTime) {
+		state = INIT;
+		return false;
+	}
+		
+	tries++;
+	timeout += timeout;
+	if (timeout > MAX_TIMEOUT)
+		timeout = MAX_TIMEOUT;
+
+	if (tries > MAX_RETRIES) {
+		bigtime_t remaining = 0;
+		if (state == RENEWING)
+			remaining = (fRebindingTime - system_time()) / 2 + 1;
+		else if (state == REBINDING)
+			remaining = (fLeaseTime - system_time()) / 2 + 1;
+		else
+			return false;
+
+		timeout = std::max(remaining / 1000000, bigtime_t(60));
+	}
+
 	syslog(LOG_DEBUG, "%s: Timeout shift: %lu secs (try %lu)\n",
 		Device(), timeout, tries);
 
@@ -978,13 +988,12 @@ DHCPClient::_CurrentState() const
 {
 	bigtime_t now = system_time();
 
-	if (now > fLeaseTime || fStatus < B_OK)
+	if (now > fLeaseTime || fStatus != B_OK)
 		return INIT;
 	if (now >= fRebindingTime)
 		return REBINDING;
 	if (now >= fRenewalTime)
 		return RENEWING;
-
 	return BOUND;
 }
 
@@ -994,35 +1003,8 @@ DHCPClient::MessageReceived(BMessage* message)
 {
 	switch (message->what) {
 		case kMsgLeaseTime:
-		{
+			_Negotiate(_CurrentState());
-			dhcp_state state = _CurrentState();
-
-			bigtime_t next;
-			if (_Negotiate(state) == B_OK) {
-				switch (state) {
-					case RENEWING:
-						next = fRebindingTime;
 			break;
-					case REBINDING:
-					default:
-						next = fRenewalTime;
-						break;
-				}
-			} else {
-				switch (state) {
-					case RENEWING:
-						next = (fLeaseTime - fRebindingTime) / 4 + system_time();
-						break;
-					case REBINDING:
-					default:
-						next = (fLeaseTime - fRenewalTime) / 4 + system_time();
-						break;
-				}
-			}
-
-			_RestartLease(next - system_time());
-			break;
-		}
 
 		default:
 			BHandler::MessageReceived(message);

src/servers/net/DHCPClient.h

@@ -44,6 +44,9 @@ public:
 
 private:
 			status_t			_Negotiate(dhcp_state state);
+			status_t			_GotMessage(dhcp_state& state,
+									dhcp_message* message);
+			status_t			_StateTransition(int socket, dhcp_state& state);
 			void				_ParseOptions(dhcp_message& message,
 									BMessage& address,
 									BMessage& resolverConfiguration);
@@ -52,10 +55,10 @@ private:
 			status_t			_SendMessage(int socket, dhcp_message& message,
 									const BNetworkAddress& address) const;
 			dhcp_state			_CurrentState() const;
-			void				_ResetTimeout(int socket, time_t& timeout,
+			void				_ResetTimeout(int socket, dhcp_state& state,
-									uint32& tries);
+									time_t& timeout, uint32& tries);
-			bool				_TimeoutShift(int socket, time_t& timeout,
+			bool				_TimeoutShift(int socket, dhcp_state& state,
-									uint32& tries);
+									time_t& timeout, uint32& tries);
 			void				_RestartLease(bigtime_t lease);
 
 	static	BString				_AddressToString(const uint8* data);
@@ -71,6 +74,7 @@ private:
 			in_addr_t			fAssignedAddress;
 			BNetworkAddress		fServer;
 			bigtime_t			fStartTime;
+			bigtime_t			fRequestTime;
 			bigtime_t			fRenewalTime;
 			bigtime_t			fRebindingTime;
 			bigtime_t			fLeaseTime;

src/servers/net/NetServer.cpp

@@ -1388,6 +1388,8 @@ NetServer::_ConvertNetworkFromSettings(BMessage& message)
 int
 main(int argc, char** argv)
 {
+	srand(system_time());
+
 	status_t status;
 	NetServer server(status);
 	if (status != B_OK) {