[systemd-devel] [PATCH 20/28] dhcp: Add timeout and main loop support

[Lennart Poettering]

On Wed, 13.11.13 23:22, Patrik Flykt (patrik.flykt at linux.intel.com) wrote:

>          int index;
>          uint8_t *req_opts;
>          int req_opts_size;
>          struct in_addr *last_addr;
>          struct ether_addr mac_addr;
>          uint32_t xid;
> +        uint64_t start_time;

Internally we prefer "usec_t" for times, since that indicates the unit
used here. usec_t is actually identical to uint64_t, but it's more
descriptive, hence preferred. Only in the public APIs we restraint
ourselves a bit and expose this as uint64_t, to not pollute the
namespace.

> +                if (!client->start_time)
> +                        client->start_time = usec;
> +
> +                secs = (usec - client->start_time) / USEC_PER_SEC;
> +
> +                next_timeout = usec + 2 * USEC_PER_SEC + (random() &
> 0x1fffff);
> +
> +                err = sd_event_add_monotonic(sd_event_get(s), next_timeout,
> +                                             10 * USEC_PER_MSEC,
> +                                             client_timeout_resend, client,
> +
> &client->timeout_resend);

if you don't have a very good reason to specify the accuracy as 10ms,
I'd always recommend to pass 0 instead, which results in the default
accuracy of 250ms (I wouldn't be too surprised if 250ms is too
inaccurate for this usecase, so your code might be fine, just wanted to
take the opportuntine to point this out...

> -        return client_send_discover(client, 0);
> +        err = sd_event_add_monotonic(client->event, now(CLOCK_MONOTONIC), 0,
> +                                     client_timeout_resend, client,
> +                                     &client->timeout_resend);

Hmm, so this would immediately trigger since "now" is already passed, by
the time you read it... Note that "0" can be used as time here too, to
indicate that you want to be executed immediately, i.e. it indicates in
this context the ealiest sensible time.

Lennart

-- 
Lennart Poettering, Red Hat