|
| 1 | +From 3996843162d6b5625edd831895dfe4cec9aa1f2e Mon Sep 17 00:00:00 2001 |
| 2 | +From: Deepak Singhal <deepsinghal@microsoft.com> |
| 3 | +Date: Thu, 25 Jun 2026 05:14:26 +0000 |
| 4 | +Subject: [PATCH] zebra: keep route_node lock in NB RIB route lookup_next |
| 5 | + |
| 6 | +The northbound oper-state iterator for /frr-vrf:lib/vrf/frr-zebra:zebra/ |
| 7 | +ribs/rib/route uses lookup_next() to resume a yielded walk. lookup_next() |
| 8 | +called route_table_get_next(), which returns a route_node with the lock it |
| 9 | +acquired held, and then immediately released it with route_unlock_node() |
| 10 | +before returning the node. |
| 11 | + |
| 12 | +That node is carried back into the walk as args->list_entry and passed to |
| 13 | +lib_vrf_zebra_ribs_rib_route_get_next() -> srcdest_route_next() -> |
| 14 | +route_next(), which unlocks its input node as part of advancing. Because |
| 15 | +lookup_next() had already dropped the lock, route_next() under-runs the |
| 16 | +lock count to zero and trips: |
| 17 | + |
| 18 | + zebra: assertion (node->lock > 0) failed |
| 19 | + |
| 20 | +aborting zebra. In SONiC this is hit during the post-restart RIB sweep when |
| 21 | +the FRR VRF oper-state notification walk (enabled by default since FRR 10.3) |
| 22 | +traverses a large RIB; the abort wedges vtysh and stalls bgpd on the zapi |
| 23 | +socket. |
| 24 | + |
| 25 | +lookup_entry() is a self-contained point lookup and correctly unlocks the |
| 26 | +node it returns. lookup_next(), by contrast, seeds an iteration whose first |
| 27 | +step (route_next) expects the carried node to still be locked. Remove the |
| 28 | +erroneous route_unlock_node() so the iterator lock contract holds. |
| 29 | + |
| 30 | +Fixes: https://github.com/sonic-net/sonic-buildimage/issues/27788 |
| 31 | +Signed-off-by: Deepak Singhal <deepsinghal@microsoft.com> |
| 32 | +--- |
| 33 | + zebra/zebra_nb_state.c | 10 ++++++++-- |
| 34 | + 1 file changed, 8 insertions(+), 2 deletions(-) |
| 35 | + |
| 36 | +diff --git a/zebra/zebra_nb_state.c b/zebra/zebra_nb_state.c |
| 37 | +index 543d4b38b3..1f660f5550 100644 |
| 38 | +--- a/zebra/zebra_nb_state.c |
| 39 | ++++ b/zebra/zebra_nb_state.c |
| 40 | +@@ -399,8 +399,14 @@ lib_vrf_zebra_ribs_rib_route_lookup_next(struct nb_cb_lookup_entry_args *args) |
| 41 | + if (!rn) |
| 42 | + return NULL; |
| 43 | + |
| 44 | +- route_unlock_node(rn); |
| 45 | +- |
| 46 | ++ /* |
| 47 | ++ * Unlike lookup_entry (a self-contained point lookup), lookup_next |
| 48 | ++ * seeds the get_next iteration: the returned node is carried as |
| 49 | ++ * args->list_entry into get_next() -> route_next(), which unlocks its |
| 50 | ++ * input. The node returned by route_table_get_next() therefore must |
| 51 | ++ * keep the lock it carries; releasing it here under-runs the lock and |
| 52 | ++ * trips the route_unlock_node() assert when the walk resumes. |
| 53 | ++ */ |
| 54 | + return rn; |
| 55 | + } |
| 56 | + |
| 57 | +-- |
| 58 | +2.34.1 |
| 59 | + |
0 commit comments