Skip to content

Commit

Permalink
dhcp: Fix retransmission timeouts
Browse files Browse the repository at this point in the history
The previous code did not ensure that there was a delay of at least
`try` seconds after each sent request.  Instead, whenever the condvar was
signaled, which could be due to retransmitted responses or messages for
unrelated transactions (there could even be spurious wakeups), the counter
was increased and a retransmit sent.  So instead of actually waiting for
15 seconds for a response (and sending 4 retransmits over that timespan),
it could happen that all five messages were sent within a second without
enough time to actually receive a response.

Using an absolute timeout that we reuse as long as there was no timeout
and the condvar was signaled for something unrelated, should ensure we
wait at least the intended delay after each sent message.

Closes #1154
  • Loading branch information
tobiasbrunner committed Jul 22, 2022
1 parent 44ab553 commit 77553bf
Showing 1 changed file with 36 additions and 4 deletions.
40 changes: 36 additions & 4 deletions src/libcharon/plugins/dhcp/dhcp_socket.c
Expand Up @@ -384,11 +384,24 @@ static bool request(private_dhcp_socket_t *this,
return TRUE;
}

/**
* Calculate the timeout to wait for a response for the given try
*/
static inline void get_timeout(int try, timeval_t *timeout)
{
timeval_t delay = { .tv_sec = try };

time_monotonic(timeout);
timeradd(timeout, &delay, timeout);
}

METHOD(dhcp_socket_t, enroll, dhcp_transaction_t*,
private_dhcp_socket_t *this, identification_t *identity)
{
dhcp_transaction_t *transaction;
timeval_t timeout;
uint32_t id;
bool got_response;
int try;

if (!this->rng->get_bytes(this->rng, sizeof(id), (uint8_t*)&id))
Expand All @@ -400,11 +413,21 @@ METHOD(dhcp_socket_t, enroll, dhcp_transaction_t*,

this->mutex->lock(this->mutex);
this->discover->insert_last(this->discover, transaction);

try = 1;
got_response = FALSE;
while (try <= DHCP_TRIES && discover(this, transaction))
{
if (!this->condvar->timed_wait(this->condvar, this->mutex, 1000 * try) &&
this->request->find_first(this->request, NULL, (void**)&transaction))
get_timeout(try, &timeout);
while (!this->condvar->timed_wait_abs(this->condvar, this->mutex, timeout))
{
if (this->request->find_first(this->request, NULL, (void**)&transaction))
{
got_response = TRUE;
break;
}
}
if (got_response)
{
break;
}
Expand All @@ -422,10 +445,19 @@ METHOD(dhcp_socket_t, enroll, dhcp_transaction_t*,
transaction->get_server(transaction));

try = 1;
got_response = FALSE;
while (try <= DHCP_TRIES && request(this, transaction))
{
if (!this->condvar->timed_wait(this->condvar, this->mutex, 1000 * try) &&
this->completed->remove(this->completed, transaction, NULL))
get_timeout(try, &timeout);
while (!this->condvar->timed_wait_abs(this->condvar, this->mutex, timeout))
{
if (this->completed->remove(this->completed, transaction, NULL))
{
got_response = TRUE;
break;
}
}
if (got_response)
{
break;
}
Expand Down

0 comments on commit 77553bf

Please sign in to comment.