[quagga-dev 11527] Re: [PATCH] pimd merge

Everton Marques everton.marques at gmail.com
Wed Sep 24 21:04:27 BST 2014


Paul,

I have just sent version 2 of the patch, hopefully addressing your comments:

https://lists.quagga.net/pipermail/quagga-dev/2014-September/011587.html

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?


Yes, done.


>
>  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) :
>>     0;
>>
>
> 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
>> Quagga.
>>
>
> 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
>> them.
>>
>
>  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
> perhaps?


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


Done.

If you will, please review the 2nd version of the patchsets.

Thanks,
Everton
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20140924/0cb685ff/attachment-0001.html>


More information about the Quagga-dev mailing list