dhcp: Fix retransmission timeouts

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 strongswan/strongswan#1154
This commit is contained in:
Tobias Brunner 2022-07-20 15:13:39 +02:00
parent 44ab5533b0
commit 77553bfee6

View File

@ -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))
@ -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;
}
@ -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;
}