Skip to content

Commit 5d6894b

Browse files
routing: improve debug logging
Use standard logging (FIB_XX_LOG) across nhg code instead of using old-style DPRINTFs. Add debug object printer for nhgs (`nhgrp_print_buf`). Example: ``` Jun 19 20:17:09 devel2 kernel: [nhgrp] inet.0 nhgrp_ctl_alloc_default: multipath init done Jun 19 20:17:09 devel2 kernel: [nhg_ctl] inet.0 alloc_nhgrp: num_nhops: 2, compiled_nhop: 2 Jun 19 20:17:26 devel2 kernel: [nhg_ctl] inet.0 alloc_nhgrp: num_nhops: 3, compiled_nhop: 3 Jun 19 20:17:26 devel2 kernel: [nhg_ctl] inet.0 destroy_nhgrp: destroying nhg#0/sz=2:[#6:1,#5:1] ``` Differential Revision: https://reviews.freebsd.org/D35525 MFC after: 2 weeks
1 parent f6ed05f commit 5d6894b

File tree

4 files changed

+94
-22
lines changed

4 files changed

+94
-22
lines changed

sys/net/route/nhgrp.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@
6060
#include <net/route/nhop_var.h>
6161
#include <net/route/nhgrp_var.h>
6262

63+
#define DEBUG_MOD_NAME nhgrp
64+
#define DEBUG_MAX_LEVEL LOG_DEBUG
65+
#include <net/route/route_debug.h>
66+
_DECLARE_DEBUG(LOG_INFO);
67+
6368
/*
6469
* This file contains data structures management logic for the nexthop
6570
* groups ("nhgrp") route subsystem.
@@ -163,7 +168,7 @@ link_nhgrp(struct nh_control *ctl, struct nhgrp_priv *grp_priv)
163168

164169
if (bitmask_alloc_idx(&ctl->nh_idx_head, &idx) != 0) {
165170
NHOPS_WUNLOCK(ctl);
166-
DPRINTF("Unable to allocate mpath index");
171+
FIB_RH_LOG(LOG_INFO, ctl->ctl_rh, "Unable to allocate nhg index");
167172
consider_resize(ctl, new_num_buckets, new_num_items);
168173
return (0);
169174
}
@@ -190,7 +195,7 @@ unlink_nhgrp(struct nh_control *ctl, struct nhgrp_priv *key)
190195
CHT_SLIST_REMOVE(&ctl->gr_head, mpath, key, nhg_priv_ret);
191196

192197
if (nhg_priv_ret == NULL) {
193-
DPRINTF("Unable to find nhop group!");
198+
FIB_RH_LOG(LOG_DEBUG, ctl->ctl_rh, "Unable to find nhg");
194199
NHOPS_WUNLOCK(ctl);
195200
return (NULL);
196201
}
@@ -233,7 +238,8 @@ consider_resize(struct nh_control *ctl, uint32_t new_gr_bucket, uint32_t new_idx
233238
return;
234239
}
235240

236-
DPRINTF("mp: going to resize: gr:[ptr:%p sz:%u] idx:[ptr:%p sz:%u]",
241+
FIB_RH_LOG(LOG_DEBUG, ctl->ctl_rh,
242+
"going to resize nhg hash: [ptr:%p sz:%u] idx:[ptr:%p sz:%u]",
237243
gr_ptr, new_gr_bucket, gr_idx_ptr, new_idx_items);
238244

239245
old_idx_ptr = NULL;
@@ -271,7 +277,7 @@ nhgrp_ctl_alloc_default(struct nh_control *ctl, int malloc_flags)
271277
cht_ptr = malloc(alloc_size, M_NHOP, malloc_flags);
272278

273279
if (cht_ptr == NULL) {
274-
DPRINTF("mpath init failed");
280+
FIB_RH_LOG(LOG_WARNING, ctl->ctl_rh, "multipath init failed");
275281
return (false);
276282
}
277283

@@ -287,8 +293,7 @@ nhgrp_ctl_alloc_default(struct nh_control *ctl, int malloc_flags)
287293
free(cht_ptr, M_NHOP);
288294
}
289295

290-
DPRINTF("mpath init done for fib/af %d/%d", ctl->rh->rib_fibnum,
291-
ctl->rh->rib_family);
296+
FIB_RH_LOG(LOG_DEBUG, ctl->ctl_rh, "multipath init done");
292297

293298
return (true);
294299
}
@@ -320,7 +325,11 @@ nhgrp_ctl_unlink_all(struct nh_control *ctl)
320325
NHOPS_WLOCK_ASSERT(ctl);
321326

322327
CHT_SLIST_FOREACH(&ctl->gr_head, mpath, nhg_priv) {
323-
DPRINTF("Marking nhgrp %u unlinked", nhg_priv->nhg_idx);
328+
#if DEBUG_MAX_LEVEL >= LOG_DEBUG
329+
char nhgbuf[NHOP_PRINT_BUFSIZE];
330+
FIB_RH_LOG(LOG_DEBUG, ctl->ctl_rh, "marking %s unlinked",
331+
nhgrp_print_buf(nhg_priv->nhg, nhgbuf, sizeof(nhgbuf)));
332+
#endif
324333
refcount_release(&nhg_priv->nhg_linked);
325334
} CHT_SLIST_FOREACH_END;
326335
}

sys/net/route/nhgrp_ctl.c

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@
5858
#include <net/route/nhop_var.h>
5959
#include <net/route/nhgrp_var.h>
6060

61+
#define DEBUG_MOD_NAME nhgrp_ctl
62+
#define DEBUG_MAX_LEVEL LOG_DEBUG
63+
#include <net/route/route_debug.h>
64+
_DECLARE_DEBUG(LOG_INFO);
65+
6166
/*
6267
* This file contains the supporting functions for creating multipath groups
6368
* and compiling their dataplane parts.
@@ -222,7 +227,8 @@ compile_nhgrp(struct nhgrp_priv *dst_priv, const struct weightened_nhop *x,
222227
for (i = 0; i < dst_priv->nhg_nh_count; i++)
223228
remaining_sum += x[i].weight;
224229
remaining_slots = num_slots;
225-
DPRINTF("O: %u/%u", (uint32_t)remaining_sum, remaining_slots);
230+
FIB_NH_LOG(LOG_DEBUG3, x[0].nh, "sum: %lu, slots: %d",
231+
remaining_sum, remaining_slots);
226232
for (i = 0; i < dst_priv->nhg_nh_count; i++) {
227233
/* Calculate number of slots for the current nexthop */
228234
if (remaining_sum > 0) {
@@ -234,9 +240,9 @@ compile_nhgrp(struct nhgrp_priv *dst_priv, const struct weightened_nhop *x,
234240
remaining_sum -= x[i].weight;
235241
remaining_slots -= nh_slots;
236242

237-
DPRINTF(" OO[%d]: %u/%u curr=%d slot_idx=%d", i,
238-
(uint32_t)remaining_sum, remaining_slots,
239-
(int)nh_slots, slot_idx);
243+
FIB_NH_LOG(LOG_DEBUG3, x[0].nh,
244+
" rem_sum: %lu, rem_slots: %d nh_slots: %d, slot_idx: %d",
245+
remaining_sum, remaining_slots, (int)nh_slots, slot_idx);
240246

241247
KASSERT((slot_idx + nh_slots <= num_slots),
242248
("index overflow during nhg compilation"));
@@ -267,12 +273,14 @@ alloc_nhgrp(struct weightened_nhop *wn, int num_nhops)
267273
size_t sz = get_nhgrp_alloc_size(nhgrp_size, num_nhops);
268274
nhg = malloc(sz, M_NHOP, M_NOWAIT | M_ZERO);
269275
if (nhg == NULL) {
276+
FIB_NH_LOG(LOG_INFO, wn[0].nh,
277+
"unable to allocate group with num_nhops %d (compiled %u)",
278+
num_nhops, nhgrp_size);
270279
return (NULL);
271280
}
272281

273282
/* Has to be the first to make NHGRP_PRIV() work */
274283
nhg->nhg_size = nhgrp_size;
275-
DPRINTF("new mpath group: num_nhops: %u", (uint32_t)nhgrp_size);
276284
nhg->nhg_flags = MPF_MULTIPATH;
277285

278286
nhg_priv = NHGRP_PRIV(nhg);
@@ -286,6 +294,9 @@ alloc_nhgrp(struct weightened_nhop *wn, int num_nhops)
286294
memcpy(&nhg_priv->nhg_nh_weights[0], wn,
287295
num_nhops * sizeof(struct weightened_nhop));
288296

297+
FIB_NH_LOG(LOG_DEBUG, wn[0].nh, "num_nhops: %d, compiled_nhop: %u",
298+
num_nhops, nhgrp_size);
299+
289300
compile_nhgrp(nhg_priv, wn, nhg->nhg_size);
290301

291302
return (nhg_priv);
@@ -345,7 +356,8 @@ nhgrp_free(struct nhgrp_object *nhg)
345356
ctl = nhg_priv->nh_control;
346357
if (unlink_nhgrp(ctl, nhg_priv) == NULL) {
347358
/* Do not try to reclaim */
348-
DPRINTF("Failed to unlink nexhop group %p", nhg_priv);
359+
RT_LOG(LOG_INFO, "Failed to unlink nexhop group %p",
360+
nhg_priv);
349361
NET_EPOCH_EXIT(et);
350362
return;
351363
}
@@ -371,13 +383,16 @@ destroy_nhgrp(struct nhgrp_priv *nhg_priv)
371383
{
372384

373385
KASSERT((nhg_priv->nhg_refcount == 0), ("nhg_refcount != 0"));
374-
375-
DPRINTF("DEL MPATH %p", nhg_priv);
376-
377386
KASSERT((nhg_priv->nhg_idx == 0), ("gr_idx != 0"));
378387

379-
free_nhgrp_nhops(nhg_priv);
388+
#if DEBUG_MAX_LEVEL >= LOG_DEBUG
389+
char nhgbuf[NHOP_PRINT_BUFSIZE];
390+
FIB_NH_LOG(LOG_DEBUG, nhg_priv->nhg_nh_weights[0].nh,
391+
"destroying %s", nhgrp_print_buf(nhg_priv->nhg,
392+
nhgbuf, sizeof(nhgbuf)));
393+
#endif
380394

395+
free_nhgrp_nhops(nhg_priv);
381396
destroy_nhgrp_int(nhg_priv);
382397
}
383398

@@ -695,6 +710,37 @@ nhgrp_get_nhops(struct nhgrp_object *nhg, uint32_t *pnum_nhops)
695710
return (nhg_priv->nhg_nh_weights);
696711
}
697712

713+
/*
714+
* Prints nexhop group @nhg data in the provided @buf.
715+
* Example: nhg#33/sz=3:[#1:100,#2:100,#3:100]
716+
* Example: nhg#33/sz=5:[#1:100,#2:100,..]
717+
*/
718+
char *
719+
nhgrp_print_buf(const struct nhgrp_object *nhg, char *buf, size_t bufsize)
720+
{
721+
const struct nhgrp_priv *nhg_priv = NHGRP_PRIV_CONST(nhg);
722+
723+
int off = snprintf(buf, bufsize, "nhg#%u/sz=%u:[", nhg_priv->nhg_idx,
724+
nhg_priv->nhg_nh_count);
725+
726+
for (int i = 0; i < nhg_priv->nhg_nh_count; i++) {
727+
const struct weightened_nhop *wn = &nhg_priv->nhg_nh_weights[i];
728+
int len = snprintf(&buf[off], bufsize - off, "#%u:%u,",
729+
wn->nh->nh_priv->nh_idx, wn->weight);
730+
if (len + off + 3 >= bufsize) {
731+
int len = snprintf(&buf[off], bufsize - off, "...");
732+
off += len;
733+
break;
734+
}
735+
off += len;
736+
}
737+
if (off > 0)
738+
off--; // remove last ","
739+
if (off + 1 < bufsize)
740+
snprintf(&buf[off], bufsize - off, "]");
741+
return buf;
742+
}
743+
698744
__noinline static int
699745
dump_nhgrp_entry(struct rib_head *rh, const struct nhgrp_priv *nhg_priv,
700746
char *buffer, size_t buffer_size, struct sysctl_req *w)

sys/net/route/nhop_ctl.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ finalize_nhop(struct nh_control *ctl, struct rt_addrinfo *info,
584584
* the epoch end, as nexthop is not used
585585
* and return.
586586
*/
587-
char nhbuf[48];
587+
char nhbuf[NHOP_PRINT_BUFSIZE];
588588
FIB_NH_LOG(LOG_WARNING, nh, "failed to link %s",
589589
nhop_print_buf(nh, nhbuf, sizeof(nhbuf)));
590590
destroy_nhop(nh_priv);
@@ -593,7 +593,7 @@ finalize_nhop(struct nh_control *ctl, struct rt_addrinfo *info,
593593
}
594594

595595
#if DEBUG_MAX_LEVEL >= LOG_DEBUG
596-
char nhbuf[48];
596+
char nhbuf[NHOP_PRINT_BUFSIZE];
597597
FIB_NH_LOG(LOG_DEBUG, nh, "finalized: %s", nhop_print_buf(nh, nhbuf, sizeof(nhbuf)));
598598
#endif
599599

@@ -655,7 +655,7 @@ nhop_free(struct nhop_object *nh)
655655
return;
656656

657657
#if DEBUG_MAX_LEVEL >= LOG_DEBUG
658-
char nhbuf[48];
658+
char nhbuf[NHOP_PRINT_BUFSIZE];
659659
FIB_NH_LOG(LOG_DEBUG, nh, "deleting %s", nhop_print_buf(nh, nhbuf, sizeof(nhbuf)));
660660
#endif
661661

@@ -683,7 +683,7 @@ nhop_free(struct nhop_object *nh)
683683
ctl = nh_priv->nh_control;
684684
if (unlink_nhop(ctl, nh_priv) == NULL) {
685685
/* Do not try to reclaim */
686-
char nhbuf[48];
686+
char nhbuf[NHOP_PRINT_BUFSIZE];
687687
FIB_NH_LOG(LOG_WARNING, nh, "failed to unlink %s",
688688
nhop_print_buf(nh, nhbuf, sizeof(nhbuf)));
689689
NET_EPOCH_EXIT(et);
@@ -864,6 +864,17 @@ nhop_print_buf(const struct nhop_object *nh, char *buf, size_t bufsize)
864864
return (buf);
865865
}
866866

867+
char *
868+
nhop_print_buf_any(const struct nhop_object *nh, char *buf, size_t bufsize)
869+
{
870+
#ifdef ROUTE_MPATH
871+
if (NH_IS_NHGRP(nh))
872+
return (nhgrp_print_buf((const struct nhgrp_object *)nh, buf, bufsize));
873+
else
874+
#endif
875+
return (nhop_print_buf(nh, buf, bufsize));
876+
}
877+
867878
/*
868879
* Dumps a single entry to sysctl buffer.
869880
*

sys/net/route/route_debug.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@
7676

7777
/* Same as FIB_LOG, but uses nhop to get fib and family */
7878
#define FIB_NH_LOG(_l, _nh, _fmt, ...) FIB_LOG_##_l(_l, nhop_get_fibnum(_nh), nhop_get_upper_family(_nh), _fmt, ## __VA_ARGS__)
79+
/* Same as FIB_LOG, but uses rib_head to get fib and family */
80+
#define FIB_RH_LOG(_l, _rh, _fmt, ...) FIB_LOG_##_l(_l, (_rh)->rib_fibnum, (_rh)->rib_family, _fmt, ## __VA_ARGS__)
7981

8082
/*
8183
* Generic logging for routing subsystem
@@ -90,7 +92,7 @@
9092
/*
9193
* Wrapper logic to avoid compiling high levels of debugging messages for production systems.
9294
*/
93-
#if DEBUG_MAX_LEVEL>=LOG_DEBUG2
95+
#if DEBUG_MAX_LEVEL>=LOG_DEBUG3
9496
#define FIB_LOG_LOG_DEBUG3 _FIB_LOG
9597
#define RT_LOG_LOG_DEBUG3 _RT_LOG
9698
#else
@@ -129,10 +131,14 @@
129131

130132
/* Helpers for fancy-printing various objects */
131133
struct nhop_object;
134+
struct nhgrp_object;
132135
struct llentry;
133136
struct nhop_neigh;
134137

138+
#define NHOP_PRINT_BUFSIZE 48
135139
char *nhop_print_buf(const struct nhop_object *nh, char *buf, size_t bufsize);
140+
char *nhop_print_buf_any(const struct nhop_object *nh, char *buf, size_t bufsize);
141+
char *nhgrp_print_buf(const struct nhgrp_object *nhg, char *buf, size_t bufsize);
136142
char *llentry_print_buf(const struct llentry *lle, struct ifnet *ifp, int family, char *buf,
137143
size_t bufsize);
138144
char *llentry_print_buf_lltable(const struct llentry *lle, char *buf, size_t bufsize);

0 commit comments

Comments
 (0)