Skip to content

Commit

Permalink
Revert 32bc422
Browse files Browse the repository at this point in the history
This commit added a heuristic to detect DHCP servers that
supposedly ignored DHCPREQUEST messages that were sent by
ndhc while it is in the RENEWING state, but this was an
incorrect workaround for a bug in ndhc.

The real problem was that ndhc was setting SO_DONTROUTE on
the UDP socket that was used to send the DHCPREQUEST via
unicast UDP while in RENEWING.  If the destination DHCP server
was not on the same subnet as the ndhc client, then this flag
would cause the DHCPREQUEST to be unable to reach the server,
since it disallows the local kernel from performing routing
table lookups and thus confines that UDP socket to only being
able to reach subnet-local peers.

In contrast, REBINDING uses broadcast instead of unicast,
and the DHCP relay agent acting on behalf of the DHCP server
would see the packet broadcast on the local subnet and dutifully
forward it along to the DHCP server; thus the SO_DONTROUTE flag
caused no harm in the REBINDING case.

So, the previous commit to this one was the real fix all along
and this workaround is no longer required.
  • Loading branch information
niklata committed May 23, 2024
1 parent a19c2f3 commit 6dc7764
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 8 deletions.
2 changes: 1 addition & 1 deletion ndhc.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ struct client_state_t {
int listenFd, arpFd, nlFd, rfkillFd;
int server_arp_sent, router_arp_sent;
uint32_t nlPortId;
unsigned int num_dhcp_requests, num_dhcp_renews;
unsigned int num_dhcp_requests;
uint32_t clientAddr, serverAddr, srcAddr, routerAddr;
uint32_t clientSubnet;
uint32_t lease, xid;
Expand Down
8 changes: 1 addition & 7 deletions state.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
#include "netlink.h"
#include "coroutine.h"

#define IGNORED_RENEWS_BEFORE_REBIND 3

#define SEL_SUCCESS 0
#define SEL_FAIL -1

Expand Down Expand Up @@ -56,7 +54,6 @@ static void reinit_shared_deconfig(struct client_state_t *cs)
advance_xid(cs);
cs->clientAddr = 0;
cs->num_dhcp_requests = 0;
cs->num_dhcp_renews = 0;
cs->server_arp_sent = 0;
cs->router_arp_sent = 0;
cs->server_arp_state = ARP_QUERY;
Expand Down Expand Up @@ -129,7 +126,7 @@ static int renewing_timeout(struct client_state_t *cs,
long long nowts)
{
long long rbt = cs->leaseStartTime + cs->rebindTime * 1000;
if (nowts >= rbt || cs->num_dhcp_renews >= IGNORED_RENEWS_BEFORE_REBIND)
if (nowts >= rbt)
return rebinding_timeout(cs, nowts);
start_dhcp_listen(cs);
if (send_renew(cs) < 0) {
Expand All @@ -138,7 +135,6 @@ static int renewing_timeout(struct client_state_t *cs,
return BTO_HARDFAIL;
}
cs->sent_renew_or_rebind = true;
++cs->num_dhcp_renews;
long long ts0 = nowts + (50 + nk_random_u32(&cs->rnd_state) % 20) * 1000;
cs->dhcp_wake_ts = ts0 < rbt ? ts0 : rbt;
return BTO_WAIT;
Expand Down Expand Up @@ -214,7 +210,6 @@ static int extend_packet(struct client_state_t *cs,
if (!validate_acknak(cs, packet, "DHCPACK", srcaddr))
return ANP_IGNORE;
cs->sent_renew_or_rebind = false;
cs->num_dhcp_renews = 0;
get_leasetime(cs, packet);

// Did we receive a lease with a different IP than we had before?
Expand All @@ -240,7 +235,6 @@ static int extend_packet(struct client_state_t *cs,
if (!validate_acknak(cs, packet, "DHCPNAK", srcaddr))
return ANP_IGNORE;
cs->sent_renew_or_rebind = false;
cs->num_dhcp_renews = 0;
log_line("%s: Our request was rejected. Searching for a new lease...\n",
client_config.interface);
reinit_selecting(cs, 3000);
Expand Down

0 comments on commit 6dc7764

Please sign in to comment.