[quagga-dev 1090] Re: point-to-point patch

Andrew J. Schorr aschorr at telemetry-investments.com
Wed Apr 28 16:07:52 BST 2004


Hi Greg,

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.

Regards,
Andy



More information about the Quagga-dev mailing list