[quagga-dev 1090] Re: point-to-point patch
Andrew J. Schorr
aschorr at telemetry-investments.com
Wed Apr 28 16:07:52 BST 2004
On Wed, Apr 28, 2004 at 09:23:37AM -0400, Greg Troxel wrote:
> I have not studied all this carefully, but another point is that I
> would be reluctant to change the daemon/zebra protocol unless it is
> really necessary. It seems that adding a flag for whether the peer
> addr is there is a protocol change, and since INADDR_ANY is never a
> legitimate address to use (as opposed to part of a prefix that is a
> default route), that's a perfectly good value for 'unknown peer addr'.
I'm sympathetic to not wanting to change the wire protocol, since I guess
that would allow independent upgrading of various daemons. But there
is an easy fix to that problem:
in lib/zclient.c:zebra_interface_address_add_read(), instead of reading
a flag byte, one can simply test the address after reading it in,
and, if it's all zeroes, just replace it with a NULL pointer. In fact,
that's the way I coded it up the first time, but then I got nervous
about whether INADDR_ANY could ever be a valid destination address.
So I don't think wire protocol compatibility bears on the larger question
of whether to use NULL pointers or zero destination addresses.
When I developed this patch, I based it on the behavior of the existing
code. Currently, the software either checks for a NULL destination pointer
or completely ignores the question of whether a valid destination address
is available and goes ahead and uses it without checking. So I decided
to make the software consistent and always check for a NULL pointer before
using the destination address.
I may be wrong, but I do not believe that there is any existing code that
checks for a zero (INADDR_ANY) destination address. So if you guys would
like to shift to that paradigm, it will actually be a larger patch.
Here's what the new patch would look like:
1. Whereever struct connected's are created, code will have to be added
to create an INADDR_ANY destination address if none was supplied.
2. Whereever the current code tests for a NULL destination address
(for example, zebra/interface.c:connected_dump_vty()), that will
have to be replaced by a test for INADDR_ANY or IN6ADDR_ANY, depending
on the prefix family.
3. Whereever the current code was using the destination address without
checking, it will have to be patched to test for INADDR_ANY or
IN6ADDR_ANY, depending on the context or prefix family.
My current patch only includes the 3rd part, plus the actual coding
of part 3 is simpler because you don't have to worry about the prefix family.
So that's why I think NULL pointers are the right way to go: I think
it's a smaller patch that is more consistent with the existing behavior.
But you guys are in charge, so I defer to your judgement.
More information about the Quagga-dev