Skip to content

Commit 5dbb7ff

Browse files
mjstappmjstapp
authored
Merge pull request #19488 from raja-rajasekar/rajasekarr/improve_nhg_debuggability
Some improvement in NHG debuggability and code
2 parents 7a24f66 + ef8d966 commit 5dbb7ff

File tree

6 files changed

+99
-44
lines changed

6 files changed

+99
-44
lines changed

‎lib/nexthop_group.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ extern void nexthop_group_json_nexthop(json_object *j,
152152
extern uint16_t nexthop_group_nexthop_num(const struct nexthop_group *nhg);
153153
extern uint16_t nexthop_group_active_nexthop_num(const struct nexthop_group *nhg);
154154

155+
/* Return TRUE if the NHG is Singleton (has only one nexthop) */
156+
#define NHG_IS_SINGLETON(nhg) ((nhg)->nexthop && !(nhg)->nexthop->next)
157+
155158
extern bool nexthop_group_has_label(const struct nexthop_group *nhg);
156159
extern uint16_t nexthop_group_nexthop_num_no_recurse(const struct nexthop_group *nhg);
157160

‎zebra/rib.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,8 @@ void route_entry_dump_nh(const struct route_entry *re, const char *straddr,
660660
/* Name of hook calls */
661661
#define ZEBRA_ON_RIB_PROCESS_HOOK_CALL "on_rib_process_dplane_results"
662662

663+
extern char *zebra_rib_dump_re_status(const struct route_entry *re, char *buf, size_t len);
664+
663665
#ifdef __cplusplus
664666
}
665667
#endif

‎zebra/zebra_nhg.c‎

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -454,16 +454,16 @@ static void *zebra_nhg_hash_alloc(void *arg)
454454
*
455455
* A proto-owned ID is always a group.
456456
*/
457-
if (!PROTO_OWNED(nhe) && nhe->nhg.nexthop && !nhe->nhg.nexthop->next
458-
&& !nhe->nhg.nexthop->resolved && nhe->nhg.nexthop->ifindex) {
457+
if (!PROTO_OWNED(nhe) && (ZEBRA_NHG_IS_SINGLETON(nhe) && nhe->nhg.nexthop->ifindex &&
458+
!CHECK_FLAG(nhe->nhg.nexthop->flags, NEXTHOP_FLAG_RECURSIVE))) {
459459
struct interface *ifp = NULL;
460460

461461
ifp = if_lookup_by_index(nhe->nhg.nexthop->ifindex,
462462
nhe->nhg.nexthop->vrf_id);
463463
if (ifp)
464464
zebra_nhg_set_if(nhe, ifp);
465465
else {
466-
if (IS_ZEBRA_DEBUG_NHG)
466+
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
467467
zlog_debug(
468468
"Failed to lookup an interface with ifindex=%d in vrf=%u for NHE %pNG",
469469
nhe->nhg.nexthop->ifindex,
@@ -830,8 +830,7 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */
830830
nh = backup_nhe->nhg.nexthop;
831831

832832
/* Singleton recursive NH */
833-
if (nh->next == NULL &&
834-
CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE)) {
833+
if (ZEBRA_NHG_IS_SINGLETON(backup_nhe) && CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE)) {
835834
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
836835
zlog_debug("%s: backup depend NH %pNHv (R)",
837836
__func__, nh);
@@ -1111,13 +1110,8 @@ void zebra_nhg_check_valid(struct nhg_hash_entry *nhe)
11111110
struct nhg_connected *rb_node_dep = NULL;
11121111
bool valid = false;
11131112

1114-
/*
1115-
* If I have other nhe's depending on me, or I have nothing
1116-
* I am depending on then this is a
1117-
* singleton nhe so set this nexthops flag as appropriate.
1118-
*/
1119-
if (nhg_connected_tree_count(&nhe->nhg_depends) ||
1120-
nhg_connected_tree_count(&nhe->nhg_dependents) == 0) {
1113+
/* Singleton means it has no depends and has only dependents */
1114+
if (ZEBRA_NHG_IS_SINGLETON(nhe)) {
11211115
UNSET_FLAG(nhe->nhg.nexthop->flags, NEXTHOP_FLAG_FIB);
11221116
UNSET_FLAG(nhe->nhg.nexthop->flags, NEXTHOP_FLAG_ACTIVE);
11231117
}
@@ -1700,14 +1694,13 @@ static void zebra_nhg_free_members(struct nhg_hash_entry *nhe)
17001694
void zebra_nhg_free(struct nhg_hash_entry *nhe)
17011695
{
17021696
if (IS_ZEBRA_DEBUG_NHG_DETAIL) {
1703-
/* Group or singleton? */
1704-
if (nhe->nhg.nexthop && nhe->nhg.nexthop->next)
1705-
zlog_debug("%s: nhe %p (%pNG), refcnt %d", __func__,
1706-
nhe, nhe, nhe->refcnt);
1707-
else
1697+
if (ZEBRA_NHG_IS_SINGLETON(nhe))
17081698
zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv",
17091699
__func__, nhe, nhe, nhe->refcnt,
17101700
nhe->nhg.nexthop);
1701+
else
1702+
zlog_debug("%s: nhe %p (%pNG) flags (0x%x), refcnt %d", __func__, nhe, nhe,
1703+
nhe->flags, nhe->refcnt);
17111704
}
17121705

17131706
event_cancel(&nhe->timer);
@@ -1725,14 +1718,13 @@ void zebra_nhg_hash_free(void *p)
17251718
struct nhg_hash_entry *nhe = p;
17261719

17271720
if (IS_ZEBRA_DEBUG_NHG_DETAIL) {
1728-
/* Group or singleton? */
1729-
if (nhe->nhg.nexthop && nhe->nhg.nexthop->next)
1730-
zlog_debug("%s: nhe %p (%u), refcnt %d", __func__, nhe,
1731-
nhe->id, nhe->refcnt);
1732-
else
1721+
if (ZEBRA_NHG_IS_SINGLETON(nhe))
17331722
zlog_debug("%s: nhe %p (%pNG), refcnt %d, NH %pNHv",
17341723
__func__, nhe, nhe, nhe->refcnt,
17351724
nhe->nhg.nexthop);
1725+
else
1726+
zlog_debug("%s: nhe %p (%u), refcnt %d", __func__, nhe, nhe->id,
1727+
nhe->refcnt);
17361728
}
17371729

17381730
event_cancel(&nhe->timer);
@@ -3265,8 +3257,9 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re,
32653257
old_re->nhe);
32663258

32673259
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
3268-
zlog_debug("%s: re %p CHANGED: nhe %p (%pNG) => new_nhe %p (%pNG) rib_find_nhe returned %p (%pNG) refcnt: %d",
3269-
__func__, re, re->nhe, re->nhe, new_nhe, new_nhe, remove, remove,
3260+
zlog_debug("%s: re %p CHANGED: nhe %p (%pNG) flags (0x%x) => new_nhe %p (%pNG) flags (0x%x) rib_find_nhe returned %p (%pNG) flags (0x%x) refcnt: %d",
3261+
__func__, re, re->nhe, re->nhe, re->nhe->flags, new_nhe,
3262+
new_nhe, new_nhe->flags, remove, remove, remove->flags,
32703263
remove ? remove->refcnt : 0);
32713264

32723265
/*
@@ -3452,8 +3445,8 @@ void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe, uint8_t type)
34523445

34533446
if (zebra_nhg_set_valid_if_active(nhe)) {
34543447
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
3455-
zlog_debug("%s: valid flag set for nh %pNG", __func__,
3456-
nhe);
3448+
zlog_debug("%s: valid flag set for nh %pNG flags (0x%x)", __func__, nhe,
3449+
nhe->flags);
34573450
}
34583451

34593452
if ((type != ZEBRA_ROUTE_CONNECT && type != ZEBRA_ROUTE_LOCAL &&
@@ -3557,7 +3550,7 @@ void zebra_nhg_dplane_result(struct zebra_dplane_ctx *ctx)
35573550
nhe = zebra_nhg_lookup_id(id);
35583551

35593552
if (!nhe) {
3560-
if (IS_ZEBRA_DEBUG_NHG)
3553+
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
35613554
zlog_debug("%s operation performed on Nexthop ID (%u) in the kernel, that we no longer have in our table",
35623555
dplane_op2str(op), id);
35633556

@@ -4083,17 +4076,16 @@ void zebra_interface_nhg_reinstall(struct interface *ifp)
40834076

40844077
if (zebra_nhg_set_valid_if_active(rb_node_dep->nhe)) {
40854078
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
4086-
zlog_debug(
4087-
"%s: Setting the valid flag for nhe %pNG, interface: %s",
4088-
__func__, rb_node_dep->nhe, ifp->name);
4079+
zlog_debug("%s: Setting the valid flag for nhe %pNG flags (0x%x), interface: %s",
4080+
__func__, rb_node_dep->nhe, rb_node_dep->nhe->flags,
4081+
ifp->name);
40894082
}
40904083

40914084
/* Check for singleton NHG associated to interface */
4092-
if (!nexthop_is_blackhole(nh) &&
4093-
zebra_nhg_depends_is_empty(rb_node_dep->nhe)) {
4085+
if (!nexthop_is_blackhole(nh) && ZEBRA_NHG_IS_SINGLETON(rb_node_dep->nhe)) {
40944086
struct nhg_connected *rb_node_dependent;
40954087

4096-
if (IS_ZEBRA_DEBUG_NHG)
4088+
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
40974089
zlog_debug(
40984090
"%s install nhe %pNG nh type %u flags 0x%x",
40994091
__func__, rb_node_dep->nhe, nh->type,
@@ -4122,10 +4114,11 @@ void zebra_interface_nhg_reinstall(struct interface *ifp)
41224114
SET_FLAG(nhop_dependent->flags,
41234115
NEXTHOP_FLAG_ACTIVE);
41244116

4125-
if (IS_ZEBRA_DEBUG_NHG)
4126-
zlog_debug("%s dependent nhe %pNG Setting Reinstall flag",
4127-
__func__,
4128-
rb_node_dependent->nhe);
4117+
if (IS_ZEBRA_DEBUG_NHG_DETAIL)
4118+
zlog_debug("%s dependent nhe (%pNG) flags (0x%x) Setting Reinstall flag",
4119+
__func__, rb_node_dependent->nhe,
4120+
rb_node_dependent->nhe->flags);
4121+
41294122
SET_FLAG(rb_node_dependent->nhe->flags,
41304123
NEXTHOP_GROUP_REINSTALL);
41314124
}

‎zebra/zebra_nhg.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ enum nhg_type {
190190

191191
#define PROTO_OWNED(NHE) (NHE->id >= ZEBRA_NHG_PROTO_LOWER)
192192

193+
/* Wrapper macro for zebra-specific usage */
194+
#define ZEBRA_NHG_IS_SINGLETON(NHE) NHG_IS_SINGLETON(&((NHE)->nhg))
195+
193196
/*
194197
* Backup nexthops: this is a group object itself, so
195198
* that the backup nexthops can use the same code as a normal object.

‎zebra/zebra_rib.c‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,7 @@ static ssize_t printfrr_zebra_node(struct fbuf *buf, struct printfrr_eargs *ea,
388388
VRF_LOGNAME(vrf), node, node, ##__VA_ARGS__); \
389389
} while (0)
390390

391-
static char *_dump_re_status(const struct route_entry *re, char *buf,
392-
size_t len)
391+
char *zebra_rib_dump_re_status(const struct route_entry *re, char *buf, size_t len)
393392
{
394393
if (re->status == 0) {
395394
snprintfrr(buf, len, "None ");
@@ -1314,8 +1313,9 @@ static void rib_process(struct route_node *rn)
13141313
zlog_debug("%s(%u:%u:%u):%pRN: Examine re %p (%s) status: %sflags: %sdist %d metric %d",
13151314
VRF_LOGNAME(vrf), vrf_id, re->table, safi, rn, re,
13161315
zebra_route_string(re->type),
1317-
_dump_re_status(re, status_buf, sizeof(status_buf)),
1318-
zclient_dump_route_flags(re->flags, flags_buf, sizeof(flags_buf)),
1316+
zebra_rib_dump_re_status(re, status_buf, sizeof(status_buf)),
1317+
zclient_dump_route_flags(re->flags, flags_buf,
1318+
sizeof(flags_buf)),
13191319
re->distance, re->metric);
13201320
}
13211321

@@ -4376,9 +4376,8 @@ void _route_entry_dump(const char *func, union prefixconstptr pp,
43764376
re->type, re->instance, re->table);
43774377
zlog_debug("%s(%s): metric == %u, mtu == %u, distance == %u, flags == %sstatus == %s",
43784378
straddr, VRF_LOGNAME(vrf), re->metric, re->mtu, re->distance,
4379-
zclient_dump_route_flags(re->flags, flags_buf,
4380-
sizeof(flags_buf)),
4381-
_dump_re_status(re, status_buf, sizeof(status_buf)));
4379+
zclient_dump_route_flags(re->flags, flags_buf, sizeof(flags_buf)),
4380+
zebra_rib_dump_re_status(re, status_buf, sizeof(status_buf)));
43824381
zlog_debug("%s(%s): tag == %u, nexthop_num == %u, nexthop_active_num == %u",
43834382
straddr, VRF_LOGNAME(vrf), re->tag,
43844383
nexthop_group_nexthop_num(&(re->nhe->nhg)),

‎zebra/zebra_vty.c‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,8 @@ static void vty_show_ip_route_detail(struct vty *vty, struct route_node *rn,
419419
char buf[SRCDEST2STR_BUFFER];
420420
struct zebra_vrf *zvrf;
421421
rib_dest_t *dest;
422+
char flags_buf[128];
423+
char status_buf[128];
422424

423425
dest = rib_dest_from_rnode(rn);
424426

@@ -468,6 +470,13 @@ static void vty_show_ip_route_detail(struct vty *vty, struct route_node *rn,
468470

469471
vty_out(vty, " Last update %s ago\n", buf);
470472

473+
zclient_dump_route_flags(re->flags, flags_buf, sizeof(flags_buf));
474+
zebra_rib_dump_re_status(re, status_buf, sizeof(status_buf));
475+
if (flags_buf[0] != '\0')
476+
vty_out(vty, " Flags: %s\n", flags_buf);
477+
if (status_buf[0] != '\0')
478+
vty_out(vty, " Status: %s\n", status_buf);
479+
471480
if (show_ng) {
472481
vty_out(vty, " Nexthop Group ID: %u\n", re->nhe_id);
473482
if (re->nhe_installed_id != 0
@@ -744,6 +753,14 @@ static void vty_show_ip_route_detail_json(struct vty *vty,
744753
if (use_fib && re != dest->selected_fib)
745754
continue;
746755
vty_show_ip_route(vty, rn, re, json_prefix, use_fib, false);
756+
757+
/* Add flags and status to the last object */
758+
json_object *json_route =
759+
json_object_array_get_idx(json_prefix,
760+
json_object_array_length(json_prefix) - 1);
761+
762+
json_object_int_add(json_route, "flags", re->flags);
763+
json_object_int_add(json_route, "status", re->status);
747764
}
748765

749766
prefix2str(&rn->p, buf, sizeof(buf));
@@ -1100,6 +1117,7 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe,
11001117
vrf_id_to_name(nhe->vrf_id));
11011118
json_object_string_add(json, "afi", zebra_nhg_afi2str(nhe));
11021119
json_object_int_add(json, "nexthopCount", nexthop_count);
1120+
json_object_int_add(json, "flags", nhe->flags);
11031121

11041122
} else {
11051123
vty_out(vty, "ID: %u (%s)\n", nhe->id,
@@ -1116,6 +1134,7 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe,
11161134
vty_out(vty, " VRF: %s(%s)\n", vrf_id_to_name(nhe->vrf_id),
11171135
zebra_nhg_afi2str(nhe));
11181136
vty_out(vty, " Nexthop Count: %u\n", nexthop_count);
1137+
vty_out(vty, " Flags: 0x%x\n", nhe->flags);
11191138
}
11201139

11211140
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_VALID)) {
@@ -1142,6 +1161,42 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe,
11421161
else
11431162
vty_out(vty, ", Initial Delay");
11441163
}
1164+
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_QUEUED)) {
1165+
if (json)
1166+
json_object_boolean_true_add(json, "queued");
1167+
else
1168+
vty_out(vty, ", Queued");
1169+
}
1170+
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_RECURSIVE)) {
1171+
if (json)
1172+
json_object_boolean_true_add(json, "recursive");
1173+
else
1174+
vty_out(vty, ", Recursive");
1175+
}
1176+
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_BACKUP)) {
1177+
if (json)
1178+
json_object_boolean_true_add(json, "backup");
1179+
else
1180+
vty_out(vty, ", Backup");
1181+
}
1182+
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_PROTO_RELEASED)) {
1183+
if (json)
1184+
json_object_boolean_true_add(json, "protoReleased");
1185+
else
1186+
vty_out(vty, ", Proto Released");
1187+
}
1188+
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_KEEP_AROUND)) {
1189+
if (json)
1190+
json_object_boolean_true_add(json, "keepAround");
1191+
else
1192+
vty_out(vty, ", Keep Around");
1193+
}
1194+
if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_FPM)) {
1195+
if (json)
1196+
json_object_boolean_true_add(json, "fpm");
1197+
else
1198+
vty_out(vty, ", FPM");
1199+
}
11451200
if (!json)
11461201
vty_out(vty, "\n");
11471202
}

0 commit comments

Comments
 (0)