Hi Paul, Thanks for the detail review and comments.
I think the problem is that there are just 3 types of requests to Zebra when
there is route change: add, delete and modify. All of the request are done
after bgp_info has updated completely based on the change. Now, if it is
simply an add, it would be no issue. If it is modify, it would also be no
issue. However, when it is delete, there could be two cases for the delete.
One case is simply the route is gone which would be no issues. The other
cause is the issue we are facing today where the route can NOT be deleted in
rib now that it has a new next_hop, but the new next_hop is unreachable such
that it needed to be deleted from the forwarding database. One can also
argue this is a modify case of the route, which is from rib perspective
only.
Given how long I have been working on Quagga code base, this is more of a
conservative solution I could come up with without too much impact to other
functions. I welcome and appreciate any feedback/suggestion one can give on
the solution.
Thanks!
-----Original Message-----
From: Paul Jakma
Sent: Wednesday, October 28, 2015 6:09 AM
To: Jacqueline Yu
Cc: Martin Winter ; J Yu ; [email protected]
Subject: Re: [quagga-dev 13436] Re: Kernel routes does not get updated when
neighbor remove the "next-hop-self" configuration
On Mon, 26 Oct 2015, Jacqueline Yu wrote:
/* Withdraw the route from the kernel. */
if (old_select
&& old_select->type == ZEBRA_ROUTE_BGP
- && old_select->sub_type == BGP_ROUTE_NORMAL)
- bgp_zebra_withdraw (p, old_select, safi);
So, this should have worked fine. That it does not means the bug then is
that old_select no longer has the information it should have - i.e. the
nexthop info that was sent to zebra.
Something has fiddled with the route in between it being sent to zebra and
this withdraw?
+ && old_select->sub_type == BGP_ROUTE_NORMAL) {
+ if (CHECK_FLAG (old_select->flags,
BGP_INFO_NEW_NHOP_INVALID))
{
+ attr = old_select->attr;
+ old_select->attr = (bgp_info_extra_get(old_select))->attr;
And this patch is trying to workaround that by stashing /another/
reference to the attributes in (struct bgp_info_extra). A reference which
whatever other bogus bit of bgpd that is trampling over the (struct
bgp_info).attr doesn't know about, and so it stays valid and pointing to
the original, correct attr. And then retrieving that reference here to
temporarily set the old_select->attr pointer back to.
That's a cute hack, I have to admit :), but it doesn't address the root
cause of this issue, does it? Something is screwing with (struct bgp_info
*)->attr when it should not be, or at least in a bogus manner, right?
Martin, how long does it take you to test for this? Is there any chance
you could do a binary search (ala git bisect) starting with a much earlier
version Quagga and locate the commit that introduces this bug?
This needs fixing properly.
regards,
--
Paul Jakma [email protected] @pjakma Key ID: 64A2FF6A
Fortune:
"Necessity is the mother of invention" is a silly proverb. "Necessity
is the mother of futile dodges" is much nearer the truth.
-- Alfred North Whitehead
_______________________________________________
Quagga-dev mailing list
[email protected]
https://lists.quagga.net/mailman/listinfo/quagga-dev
