nearly-tickless-tinc

[Guus Sliepen]

On Sun, May 29, 2011 at 07:43:54PM +0200, Loïc Grenié wrote:

>     tinc has a fixed timeout of 1 second for select() in the main
>   loop. I think this could be improved. Do you think the following
>   patch goes into the right direction ?

Well, the reason that it has a 1 second timeout is to avoid some problems that
you get with a tickless main loop:

>      I don't know whether pselect() is standard enough (works under
>   "current" linux, however I don't know about the other arches). Maybe
>   a config option is necessary.

pselect() is POSIX, so recent *BSDs will probably also have it. However,
Windows does not. On the other hand, Windows has no useful signals so one
could revert to normal select() there.

Another problem is that older kernels (< 2.6.16) did not have a pselect()
system call, and glibc has been emulating it, but of course it could not do so
without a race condition.

>      It's probably possible to simplify at least signal handling (I've
>   tried to keep SIGHUP and SIGALRM blocking as they are,
>   however I suspect that tincd unblocks those signals at the
>   beginning or, at least, does not modify their blocking while
>   running). It is also undoubtedly possible to improve the patch
>   in other ways.

A way to get around the whole signal blocking mess is to have a "self-pipe",
where the signal handlers write a byte to, and select() checks the pipe fd.
However, this requires somewhat more changes than using pselect().

In tinc 1.1 we do not have the problem of signals anymore, since it has a
control socket. And libevent() is used, which takes care of all the details of
event loop handling for us anyway.

>      Tell me if you are interested and whether I should modify
>   anything.

Yes, I am interested. However, it should work on all platforms supported by
tinc, and:

>  int main_loop(void) {
[...]
> +		/* Block SIGHUP & SIGALARM if not already blocked */
> +		sigprocmask(SIG_BLOCK, NULL, &omask);
> +		sigemptyset(&block_mask);
> +		if (!sigismember(&omask, SIGHUP))
> +			sigaddset(&block_mask, SIGHUP);
> +		if (!sigismember(&omask, SIGALRM))
> +			sigaddset(&block_mask, SIGALRM);
> +		sigprocmask(SIG_BLOCK, &block_mask, &omask);
[...]
> +		r = pselect(maxfd + 1, &readset, &writeset, NULL, &tv, &block_mask);
> +		/* Unlock SIGHUP & SIGALARM (if not already blocked) */
> +		sigprocmask(SIG_UNBLOCK, &block_mask, NULL);
[...]

That's a lot of sigprocmask() calls in the main loop. Not only is that
inefficient, but to prevent race conditions, only pselect() should change the
signal mask in the main loop. 

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus at tinc-vpn.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://www.tinc-vpn.org/pipermail/tinc-devel/attachments/20110529/593ef696/attachment.pgp>