[radvd-devel-l] reload_config in another place

[Reuben Hawkins]

Hi All,

The timers seem buggy and make debugging more challenging.  A while
back, I proposed removing the timers and using select (or poll).
Using select (or poll) allows the timers to be entirely maintained in
the OS code.  We just have to tell the OS how long to wait for IO on
sock.  Also, the timers introduce race conditions with vlog and other
non-reentrent functions.  Attached is updated code using poll.  I've
done a fair amount of testing and now it's ready for your eyes.
Please review...

Happy Holidays!
Reuben

On Mon, Dec 21, 2009 at 12:24 PM, Pekka Savola <pekkas at netcore.fi> wrote:
> On Thu, 17 Dec 2009, LiKai wrote:
>>
>> I observed  that version 1.5 has fixed an infinite loop problem
>> caused by reload_config() and timer list corruption.
>>
>> Besides send_ra(), I saw reload_config() is also called in another
>> place. Actually, it is in the loop of main() after sighup is received.
>> I suspect this piece of code has similiar problem when interacted
>> with timer_handler() -> set_timer().
>>
>> Maybe I will do a test to see if this is a real problem, when I
>> get free. Or anyone can please stop me by pointing out my fault.
>>
>> B.R.
>> Victor
>
> Please, if you have time, do take a look at this!  Much appreciated.
> If this needs to be fixed, and we already have the flexible options patch,
> we might have enough material for the next release.
>
> --
> Pekka Savola                 "You each name yourselves king, yet the
> Netcore Oy                    kingdom bleeds."
> Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
> --
> radvd-devel-l mailing list  :  radvd-devel-l at litech.org
> http://lists.litech.org/listinfo/radvd-devel-l
>

>On Fri, 28 Aug 2009, LiKai wrote:
>> I have a rough understanding about this call: it is not a reentrant call, which means it is not supposed to be used in
>> signal handling context.
>>
>> vlog() call is used to print log in different methods, and localtime() is called to format time stamp in log. It can be
>> called in both normal application context and signal handling context, which maybe will cause deadlock.
>>
>> Is this a problem? Please help to point it out, if I have any misunderstanding here.

>I fear you're correct.  Luckily enough, only a few calls to vlog are
>active unless you run radvd in debug mode.  I don't recall seeing this
>problem in practise.

>The use of logging is most pervasive in timer_handler which in turn
>calls send_ra_forall().  The latter really needs some printing
>capabilities.

>You could avoid these problems by using select() as Reuben Hawkins did
>with a patched version, but select has its own complications.
>Reuben's implementation for example uses usleep() in one place as a
>replacement for mdelay and rescheduling timers.  AFAIK this would
>block radvd from serving other interfaces or requests for the time its
>sleeping.  In the end, in a select() implementation you would probably
>need to manipulate iface->expiration from various places in a bit
>similar manner that timers currently do, to take into account all the
>complications that the RFC requires such as:

>1) if you've just sent a periodic RA, and get an RS, you may need to
>   a) skip the next scheduled RA if they're too close together, or
>   b) reschedule the next scheduled RA to be sooner,
>2) add various random delays to RS solicited RAs,
>3) send more tightly paced RAs when radvd is starting up, and
>4) send an additional customized RA sent when radvd is shutting down

>I think these would likely be solvable but would result in a bit more
>complicated implementation expiration timer implementation in a
>select() loop.

>Having had to fight and debug the timer code just recently, moving to
>select like approach might have some appeal though :-)

>--
>Pekka Savola                 "You each name yourselves king, yet the
>Netcore Oy                    kingdom bleeds."
>Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings
-------------- next part --------------
A non-text attachment was scrubbed...
Name: radvd-1.5-poll-1.3.tgz
Type: application/x-gzip
Size: 227430 bytes
Desc: not available
URL: <http://lists.litech.org/pipermail/radvd-devel-l/attachments/20091221/2c86bac6/attachment-0001.bin>