[quagga-dev 7710] Re: [PATCH] zebra: consider all nexthops when looking fora gateway match
paul at jakma.org
paul at jakma.org
Mon Feb 1 10:24:09 GMT 2010
On Mon, 1 Feb 2010, Alexis Rosen wrote:
> I'm completely in sympathy with the desire to clean up the code,
> but refusing this patch won't make that happen any faster.
> Reviewing it and accepting it would probably take less time than
> responding in the negative multiple times a year as the issue comes
> up, which is in fact what's been going on.
There have not been negative responses to that patch. To the contrary
The problems are 2 fold, one relating to the RIB and one relating to
"How are maintainers to know your patch is 'good'":
1 The RIB:
It's a bit of a mess at present. People poke at it to fix small bugs
every now and then, but it's clearly much harder to understand than
it ought to be. Joakim's own patch makes that comment; patches to the
RIB (including the one that initiated this thread) often go through
more than 1 revision even when the fix is conceptually simple.
Further, precisely because the code is a mess and hard to understand,
it becomes hard for maintainers to apply patches. Which brings us
2. How can we maintainers judge whether a patch is 'good' or not?
We tend to go by:
a) Our ability to understand the fix
b) Hard empirical evidence (i.e. the output of test cases; even
better: unit tests we can easily run ourselves)
c) Anecdotal evidence from feedback on the list
Now, the last one is an interesting one. Clearly, feedback is from a
self-selected set - ones who have had the problem. From this, and
somewhat confirmed by experience, it follows that feedback can not be
relied upon to spot regressions.
So the regression risk can only really be evaluated using a and b.
The patch in its current form was posted last november afaict (and is
significantly expanded upon from the patch from last jan), and I
think there have been a couple of emails since about this problem,
which this patch might solve. The patch has never been rejected, it
simply has not yet been integrated (see above).
So, ignoring the cleanup, I don't think this patch has been treated
Regarding the RIB reform process. It is clearly the correct thing to
do. The state of the current code is clearly a big obstacle to
getting bugs fixed and patches integrated. Even if bug fixers do
eventually manage to mentally untangle the code in their head, that
knowledge doesn't transfer to reviewers and it doesn't transfer to
the next time bugs are to be fixed (even by same fixer).
There are some big problems in the RIB that simply can NOT be solved
without reforming it significantly. E.g. I'd be amazed if anyone gets
the various recursion bugs fixed (architecturally it'll just be hard
to do with duplicated nexthop information, I suspect).
Must bug contributors build on the clean-up, no. They can do what
they want. Equally though, they need to have an appreciation of the
difficulties of the maintainer to understand whether a patch is
'good' or not.
Paul Jakma paul at jakma.org Key ID: 64A2FF6A
I *____knew* I had some reason for not logging you off... If I could just
remember what it was.
More information about the Quagga-dev