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

[Patrik Flykt]

	Hi,

On Thu, 2013-11-14 at 01:30 +0100, Lennart Poettering wrote:
> 
> > +                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...

I tried to figure out some reasonable accuracy for sending the DHCP
messages so that hordes of clients would not trigger all at the same
time. The default 250ms seems to be a too coarse interval for this, 10ms
looked decently low enough to spread out the requests without being
overly aggressive. At some point there will be real numbers from real
use cases and the accuracy should be adjusted accordingly.

> > -        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.

Bummer. I was thinking that the new event would be run immediately after
the current one had returned to the main loop (or actually started in
this case). The general idea was to do the resending in one place only
and keep the new event handling clear from possible interference with
the currently running one, if needed. 'now(CLOCK_MONOTONIC) + 1' would
be enough in those cases, I'd guess. But in here it'd be easier to just
call the function directly. I'll check the other related parts also.

Thanks for all the comments in getting me on track with systemd
internals. I'll update the patch set according to your and zbvszek's
comments.


Cheers,

	Patrik