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

Christian Franke nobody at nowhere.ws
Tue Sep 23 16:47:34 BST 2014


On 09/16/2014 08:47 PM, Everton Marques wrote:
> 3/3 - pim-specific changes rebased over reconnect branch:
> https://github.com/udhos/qpimd/tree/pim-only-rebased

All in all, this looks quite good. From looking over it briefly, there
are three things which I found additionally to what Paul already mentioned:

One thing that I also don't like all too much from a readability
standpoint it the zclient_lookup_connect() method. (discovered while
trying to find out why the static storage attribute was removed from
some of the socket helper functions) Could you elaborate on why this
debugging is necessary and why it is only necessary for PIMd? Right now
this looks to me like it should be either dropped completely or moved
into the generic zclient_socket_connect, but maybe I am overlooking
something.

commit 9bcd24bd4f7ac6a [pim] Initial pim 0.155
Drops the install_element call for show_memory_isis_cmd - that's
probably the result of some bad merge.

commit 6ef4e562184eca91066c8e8
Makes a change to nexthop_active_ipv4. I do not understand how the
modification helps with the problem described in the comment/TODO, maybe
you could explain it in some different way?
A case where this may help is if there is a route in the kernel which
Zebra would consider inactive. With the change, zebra will try recursive
resolution and might come to the conclusion that it's indeed reachable.
But this particular issue also becomes apparent e.g. when the user adds
a route with onlink flag to the kernel where the gateway is not
otherwise reachable. Zebra will see the route but will judge it as
inactive, even with recursive resolution, so this change wouldn't
address this problem completely.

-Christian





More information about the Quagga-dev mailing list