[quagga-dev 11527] Re: [PATCH] pimd merge
everton.marques at gmail.com
Wed Sep 24 21:04:27 BST 2014
I have just sent version 2 of the patch, hopefully addressing your comments:
See my comments below.
On Fri, Sep 19, 2014 at 8:55 AM, Paul Jakma <paul at jakma.org> wrote:
> Hi Everton,
> On Thu, 18 Sep 2014, Everton Marques wrote:
> I don't think there is need. I believe the pim_util's version of checksum
>> just predates my knowledge about the one provided by Quagga. pim_util's
>> checksum compilation is disabled for a long time.
> Ah ok. So can it just be removed?
> Next in "pimd: fix wtf code", why should the received distance be ignored
>>> in the pim_zebra.c change? A comment in the code might be useful there?
>> I see what you mean, because I just got confused over it again, but the
>> distance is not ignored, and it seems fine as is:
>> api.distance = CHECK_FLAG(api.message, ZAPI_MESSAGE_DISTANCE) ?
>> - api.distance = stream_getc(s) :
>> + stream_getc(s) :
> Ah, yes, I see it now too. :)
> I think that's a good sign this would be more clear to write with an 'if'.
Agree, though I did not make this change now. If you think that it is
critical to rewrite that portion, let me know.
> Yes, probably a quick fix for a compilation warning.
>> default_motd is changed in order to display pimd version.
>> But I think the best fix is actually to remove the motd tweaking
>> altogether, because pimd should not have its own version once included in
> Ok, even better.
> The change you pointed out has 2 effects:
> 1) Provide an optional new function pointer callback
>> (zclient.zclient_broken) to notify the application about the connection
>> loss. Enabling the daemon to easily handle such event. Because it is a new
>> optional pointer, initialized to 0 by default, it should not affect
>> existing daemons.
> 2) Clear all interfaces' addresses on connection loss, to avoid address
>> duplication, because on reconnection the zclient lib code will re-assign
> It's expected that daemons wanting to survive reconnection to zebra daemon
>> could set zclient.zclient_broken to a callback handler. Otherwise, nothing
>> is expected.
> Ok. Makes sense and it's a more restricted change than I thought then.
> Oh, should zclient_broken in zclient_new() be set to NULL rather than 0
Christian Franke noticed initialization of zclient_broken was redudant. So
it has been removed.
> Sorry, I can't remember the reason for this gettime() mess.
>> I suppose the code could be simplified to rely solely on gettimeofday().
> If there's old fallback code that isn't relevant anymore then it's usually
> good to remove it. :)
If you will, please review the 2nd version of the patchsets.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Quagga-dev