Loïc Grenié | 1 Jun 2011 10:15
Picon

Re: nearly-tickless-tinc

2011/5/31 Guus Sliepen <guus@...>:
> On Mon, May 30, 2011 at 08:39:48AM +0200, Loïc Grenié wrote:
>
>>     My wife has a Mac with MacOS/X (I think). I should be able to compile
>>   something next week. pselect seems to be available since at least 10.3
>
> Great.

    I'll test asap.

>>     However, once again, if pselect() is not available or not compatible or
>>   whatever it can be emulated with a select:
>
> Yes. You can add a check in configure.in for pselect(), and add the necessary
> #ifdefs to ensure it automatically selects the right thing.

     What do you think about the racy "glibc pselect" ? Right now I do
  not test for it. It is probably possible to test for it using
syscall(2), either
  at compile time or at runtime.

>> PS: in my first patch, the signal mask I gave as last argument of
>>   pselect() was wrong, sorry. The attached patch should be correct.
>
> Looks good! If you can add the configure.in checks, and write a description for
> your patch, I can commit it to the git repository without any changes :)

       We'll see ! Right now I have hijacked timespec and tv_nsec in net.c
  -- when not using pselect() -- to streamline mail_loop source code -- it's
  probably not very good software engineering. What do you prefer ?
(Continue reading)

Guus Sliepen | 1 Jun 2011 23:26
Gravatar

Re: nearly-tickless-tinc

On Wed, Jun 01, 2011 at 10:15:26AM +0200, Loïc Grenié wrote:

> >>     However, once again, if pselect() is not available or not compatible or
> >>   whatever it can be emulated with a select:
> >
> > Yes. You can add a check in configure.in for pselect(), and add the necessary
> > #ifdefs to ensure it automatically selects the right thing.
> 
>      What do you think about the racy "glibc pselect" ? Right now I do
>   not test for it. It is probably possible to test for it using
> syscall(2), either
>   at compile time or at runtime.

Well, it is not so critical, and the problem is only with kernels that are more
than five years old. If you really want, you could add a runtime check using
the uname() function.

> > Looks good! If you can add the configure.in checks, and write a description for
> > your patch, I can commit it to the git repository without any changes :)
> 
>        We'll see ! Right now I have hijacked timespec and tv_nsec in net.c
>   -- when not using pselect() -- to streamline mail_loop source code -- it's
>   probably not very good software engineering. What do you prefer ?

Hm, in this case it is better to explicitly use #ifdef ... #else ... #endif in
the main loop, instead of #defining tinc_pselect and friends. With the former
it is more clear what happens.

--

-- 
Met vriendelijke groet / with kind regards,
(Continue reading)

Loïc Grenié | 2 Jun 2011 10:43
Picon

Re: nearly-tickless-tinc

2011/6/1 Guus Sliepen <guus@...>:
> On Wed, Jun 01, 2011 at 10:15:26AM +0200, Loïc Grenié wrote:
>
>> >>     However, once again, if pselect() is not available or not compatible or
>> >>   whatever it can be emulated with a select:
>> >
>> > Yes. You can add a check in configure.in for pselect(), and add the necessary
>> > #ifdefs to ensure it automatically selects the right thing.
>>
>>      What do you think about the racy "glibc pselect" ? Right now I do
>>   not test for it. It is probably possible to test for it using
>> syscall(2), either
>>   at compile time or at runtime.
>
> Well, it is not so critical, and the problem is only with kernels that are more
> than five years old.

    tincd is not tickless anyway: there is a tick for keepalive, that means that
  signal handling will be at most delayed.

> If you really want, you could add a runtime check using the uname() function.

     I do not "really" want. If you think it's not critical, I'm happy
to leave it
  that way.

>> > Looks good! If you can add the configure.in checks, and write a description for
>> > your patch, I can commit it to the git repository without any changes :)
>>
>>        We'll see ! Right now I have hijacked timespec and tv_nsec in net.c
(Continue reading)

Guus Sliepen | 2 Jun 2011 21:45
Gravatar

Re: nearly-tickless-tinc

On Thu, Jun 02, 2011 at 10:43:43AM +0200, Loïc Grenié wrote:

> > Well, it is not so critical, and the problem is only with kernels that are more
> > than five years old.
> 
>     tincd is not tickless anyway: there is a tick for keepalive, that means that
>   signal handling will be at most delayed.

Indeed. So let's ignore racy pselect()s.

> > Hm, in this case it is better to explicitly use #ifdef ... #else ... #endif in
> > the main loop, instead of #defining tinc_pselect and friends. With the former
> > it is more clear what happens.
> 
>      Here you go !

By "Here you go", did you mean there should have been a patch attached to the
email? :)

>     It is probably a good idea to also change time comparison: with this
>   patch "now" is evaluated after pselect(), thus with code like this:
> 
> timeout = now+1;
> [... pselect() lasting 1s ... ]
> if (timeout < now) {
>     contional code
> }
> 
>   conditional code will not get executed. It would be if comparison were
>   timeout <= now (the instruction "next_event++;" in the patch would
(Continue reading)

Loïc Grenié | 3 Jun 2011 11:36
Picon

Re: nearly-tickless-tinc

2011/6/2 Guus Sliepen <guus@...>:
> On Thu, Jun 02, 2011 at 10:43:43AM +0200, Loïc Grenié wrote:
>> 2011/6/2 Guus Sliepen <guus@...>:
>> > Hm, in this case it is better to explicitly use #ifdef ... #else ... #endif in
>> > the main loop, instead of #defining tinc_pselect and friends. With the former
>> > it is more clear what happens.
>>
>>      Here you go !
>
> By "Here you go", did you mean there should have been a patch attached to the
> email? :)

     I'll ignore such a futile insinuation that I might have forgotten
to attach anything.

>>     It is probably a good idea to also change time comparison: with this
>>   patch "now" is evaluated after pselect(), thus with code like this:
>>
>> timeout = now+1;
>> [... pselect() lasting 1s ... ]
>> if (timeout < now) {
>>     contional code
>> }
>>
>>   conditional code will not get executed. It would be if comparison were
>>   timeout <= now (the instruction "next_event++;" in the patch would
>>   also disappear). I can take care of it.
>
> now = time(NULL) should indeed by right after pselect(). Using <= seems better
> as well.
(Continue reading)

Guus Sliepen | 3 Jun 2011 12:49
Gravatar

Re: nearly-tickless-tinc

On Fri, Jun 03, 2011 at 11:36:58AM +0200, Loïc Grenié wrote:

> > now = time(NULL) should indeed by right after pselect(). Using <= seems better
> > as well.
> 
>     Ok. Please check that that comparison with now in try_harder is
> still correct
>   and check also "diff = " in "expire_events".

They are still correct.

> >> +                   if(next_event <= now || sighup || sigalrm)
> >
> > It shouldn't be necessary to check for sighup or sigalrm at that point; if they
> > are set there then surely pselect() will return with EINTR immediately anyway.
> 
>     It is indeed not necessary because signals are blocked outside pselect
>   and sighup || sigalrm can only be set by the handlers. However if ever they
>   were set pselect would *not* return EINTR (signals have already been
>   caught). I've removed them.

Ah yes.

> diff -ru tinc-1.0.14.orig/config.h.in tinc-1.0.14/config.h.in
> diff -ru tinc-1.0.14.orig/configure tinc-1.0.14/configure

You should not include the diff of these autogenerated files.

> +event_t *peek_next_event(void) {
> +	if (event_tree->head)
(Continue reading)

Loïc Grenié | 4 Jun 2011 09:05
Picon

Re: nearly-tickless-tinc

2011/6/3 Guus Sliepen <guus@...>:
>> diff -ru tinc-1.0.14.orig/config.h.in tinc-1.0.14/config.h.in
>> diff -ru tinc-1.0.14.orig/configure tinc-1.0.14/configure
>
> You should not include the diff of these autogenerated files.

    Removed.

>> +event_t *peek_next_event(void) {
>> +     if (event_tree->head)
>> +             return event_tree->head->data;
>> +}
>
> My compiler complains that you can reach the end of the function without
> returning anything.

    How correct ! Sorry about that.

       Thanks,

           Loïc
Attachment (tinc-pselect.diff): text/x-diff, 7035 bytes
2011/6/3 Guus Sliepen <guus@...>:
>> diff -ru tinc-1.0.14.orig/config.h.in tinc-1.0.14/config.h.in
>> diff -ru tinc-1.0.14.orig/configure tinc-1.0.14/configure
>
> You should not include the diff of these autogenerated files.

(Continue reading)

Guus Sliepen | 4 Jun 2011 11:29
Gravatar

Re: nearly-tickless-tinc

On Sat, Jun 04, 2011 at 09:05:23AM +0200, Loïc Grenié wrote:

> > You should not include the diff of these autogenerated files.
> 
>     Removed.
> 
> > My compiler complains that you can reach the end of the function without
> > returning anything.
> 
>     How correct ! Sorry about that.

Excellent! I've committed it to the git repository.

--

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus@...>
On Sat, Jun 04, 2011 at 09:05:23AM +0200, Loïc Grenié wrote:

> > You should not include the diff of these autogenerated files.
> 
>     Removed.
> 
> > My compiler complains that you can reach the end of the function without
> > returning anything.
> 
>     How correct ! Sorry about that.

Excellent! I've committed it to the git repository.
(Continue reading)

Loïc Grenié | 4 Jun 2011 12:41
Picon

Re: nearly-tickless-tinc

2011/6/4 Guus Sliepen <guus@...>:
> Excellent! I've committed it to the git repository.

    Thanks !

        Loïc
Michael Schmitz | 16 Jun 2011 07:27

1.0.14 error on Windows-XP

Hi...

Tinc v1.0.13 works fine on Windows XP but Tinc v1.0.14 shows only an error
message (tinc -n xxx -d5 -D). The error message is "System command
'setpriority' failed. Command not found"...

Any suggestions ?

Kind regards,
Michael


Gmane