Date: Mon, 13 Jul 2026 14:58:00 +0000 (UTC) From: "Bjoern A. Zeeb" <bz@FreeBSD.org> To: Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org> Cc: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: Re: git: d05d1f256082 - main - routing: Subscribe nhops to ifnet link events Message-ID: <3p674769-n07-n27p-o386-531r93405499@mnoonqbm.arg> In-Reply-To: <6a5298de.25070.b0bcc3b@gitrepo.freebsd.org>
index | next in thread | previous in thread | raw e-mail
On Sat, 11 Jul 2026, Pouria Mousavizadeh Tehrani wrote: > The branch main has been updated by pouria: > > URL: https://cgit.FreeBSD.org/src/commit/?id=d05d1f25608230edd300d59b96da6521b409d4f3 > > commit d05d1f25608230edd300d59b96da6521b409d4f3 > Author: Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org> > AuthorDate: 2026-07-08 19:55:46 +0000 > Commit: Pouria Mousavizadeh Tehrani <pouria@FreeBSD.org> > CommitDate: 2026-07-11 18:23:05 +0000 > > routing: Subscribe nhops to ifnet link events > > Update nexthop flags with interface link status events and > instead of checking link status of interface for every packet > only check the reachability flag of the final nexthop. > > Reviewed by: glebius > Discussed with: markj > Differential Revision: https://reviews.freebsd.org/D57375 > --- > sys/net/route.h | 4 +-- > sys/net/route/nhop.h | 8 +----- > sys/net/route/nhop_ctl.c | 68 +++++++++++++++++++++++++++++++++++++++++++++-- > sys/net/route/route_ctl.h | 1 + > sys/netinet/in_fib.c | 6 ++--- > sys/netinet6/in6_fib.c | 6 ++--- > 6 files changed, 75 insertions(+), 18 deletions(-) > > diff --git a/sys/net/route.h b/sys/net/route.h > index d8cc12a39c61..b4440de19d96 100644 > --- a/sys/net/route.h > +++ b/sys/net/route.h > @@ -205,6 +205,7 @@ EVENTHANDLER_DECLARE(rtnumfibs_change, rtnumfibs_change_t); > #define NHF_BROADCAST 0x0100 /* RTF_BROADCAST */ > #define NHF_GATEWAY 0x0200 /* RTF_GATEWAY */ > #define NHF_HOST 0x0400 /* RTF_HOST */ > +#define NHF_INVALID 0x0800 /* Nexthop is unreachable */ > > /* Nexthop request flags */ > #define NHR_NONE 0x00 /* empty flags field */ > @@ -368,9 +369,6 @@ struct rt_addrinfo { > > #ifdef _KERNEL > > -#define RT_LINK_IS_UP(ifp) (!((ifp)->if_capabilities & IFCAP_LINKSTATE) \ > - || (ifp)->if_link_state == LINK_STATE_UP) > - > #define RO_NHFREE(_ro) do { \ > if ((_ro)->ro_nh) { \ > NH_FREE((_ro)->ro_nh); \ > diff --git a/sys/net/route/nhop.h b/sys/net/route/nhop.h > index 6c62ae2f2f5f..0ae41cbf8292 100644 > --- a/sys/net/route/nhop.h > +++ b/sys/net/route/nhop.h > @@ -145,14 +145,8 @@ struct nhop_object { > > /* > * Nhop validness. > - * > - * Currently we verify whether link is up or not on every packet, which can be > - * quite costy. > - * TODO: subscribe for the interface notifications and update the nexthops > - * with NHF_INVALID flag. > */ > - > -#define NH_IS_VALID(_nh) RT_LINK_IS_UP((_nh)->nh_ifp) > +#define NH_IS_VALID(_nh) (!((_nh)->nh_flags & NHF_INVALID)) > #define NH_IS_NHGRP(_nh) ((_nh)->nh_flags & NHF_MULTIPATH) > > #define NH_FREE(_nh) do { \ > diff --git a/sys/net/route/nhop_ctl.c b/sys/net/route/nhop_ctl.c > index 9ef5bbc74a92..4f013908b338 100644 > --- a/sys/net/route/nhop_ctl.c > +++ b/sys/net/route/nhop_ctl.c You need to include eventhandler.h or what it is (as it's currently only leaked in through vnet code): 24 /sys/net/route/nhop_ctl.c:114:2: error: call to undeclared function 'EVENTHANDLER_REGISTER'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration] 25 114 | EVENTHANDLER_REGISTER(ifnet_event, nhops_ifnet_event, NULL, 26 | ^ 27 /sys/net/route/nhop_ctl.c:114:24: error: use of undeclared identifier 'ifnet_event' 28 114 | EVENTHANDLER_REGISTER(ifnet_event, nhops_ifnet_event, NULL, 29 | ^~~~~~~~~~~ 30 /sys/net/route/nhop_ctl.c:115:6: error: use of undeclared identifier 'EVENTHANDLER_PRI_ANY' 31 115 | EVENTHANDLER_PRI_ANY); 32 | ^~~~~~~~~~~~~~~~~~~~ 33 /sys/net/route/nhop_ctl.c:116:24: error: use of undeclared identifier 'ifnet_link_event' 34 116 | EVENTHANDLER_REGISTER(ifnet_link_event, nhops_ifnet_link_event, NULL, 35 | ^~~~~~~~~~~~~~~~ 36 /sys/net/route/nhop_ctl.c:117:6: error: use of undeclared identifier 'EVENTHANDLER_PRI_ANY' 37 117 | EVENTHANDLER_PRI_ANY); 38 | ^~~~~~~~~~~~~~~~~~~~ 39 5 errors generated. 40 --- nhop_ctl.o --- 41 *** [nhop_ctl.o] Error code 1 > @@ -90,6 +90,8 @@ static void fill_sdl_from_ifp(struct sockaddr_dl_short *sdl, const struct ifnet > > static void destroy_nhop_epoch(epoch_context_t ctx); > static void destroy_nhop(struct nhop_object *nh); > +static void nhops_ifnet_event(void *arg, struct ifnet *ifp, int state); > +static void nhops_ifnet_link_event(void *arg, struct ifnet *ifp, int state); > > _Static_assert(__offsetof(struct nhop_object, nh_ifp) == 32, > "nhop_object: wrong nh_ifp offset"); > @@ -109,6 +111,10 @@ nhops_init(void) > nhops_zone = uma_zcreate("routing nhops", > NHOP_OBJECT_ALIGNED_SIZE + NHOP_PRIV_ALIGNED_SIZE, > NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, 0); > + EVENTHANDLER_REGISTER(ifnet_event, nhops_ifnet_event, NULL, > + EVENTHANDLER_PRI_ANY); > + EVENTHANDLER_REGISTER(ifnet_link_event, nhops_ifnet_link_event, NULL, > + EVENTHANDLER_PRI_ANY); > } > > /* > @@ -1109,7 +1115,10 @@ nhops_iter_start(struct nhop_iter *iter) > if (iter->rh != NULL) { > struct nh_control *ctl = iter->rh->nh_control; > > - NHOPS_RLOCK(ctl); > + if (iter->wlock) > + NHOPS_WLOCK(ctl); > + else > + NHOPS_RLOCK(ctl); > > iter->_i = 0; > iter->_next = CHT_FIRST(&ctl->nh_head, iter->_i); > @@ -1147,7 +1156,10 @@ nhops_iter_stop(struct nhop_iter *iter) > if (iter->rh != NULL) { > struct nh_control *ctl = iter->rh->nh_control; > > - NHOPS_RUNLOCK(ctl); > + if (iter->wlock) > + NHOPS_WUNLOCK(ctl); > + else > + NHOPS_RUNLOCK(ctl); > } > } > > @@ -1317,3 +1329,55 @@ nhops_dump_sysctl(struct rib_head *rh, struct sysctl_req *w) > > return (0); > } > + > +static void > +nhops_ifnet_state_changed(struct ifnet *ifp, bool status) > +{ > + struct nhop_object *nh; > + struct nhop_iter iter = { .fibnum = ifp->if_fib, .wlock = true }; > + > + for (iter.family = 1; iter.family <= AF_MAX; iter.family++) { > + iter.rh = rt_tables_get_rnh_safe(iter.fibnum, iter.family); > + for (nh = nhops_iter_start(&iter); nh != NULL; > + nh = nhops_iter_next(&iter)) { > + if (nh->nh_ifp != ifp) > + continue; > + > + if (status) > + nh->nh_flags &= ~NHF_INVALID; > + else > + nh->nh_flags |= NHF_INVALID; > + } > + nhops_iter_stop(&iter); > + } > +} > + > +static void > +nhops_ifnet_event(void *arg __unused, struct ifnet *ifp, int state) > +{ > + > + if ((ifp->if_flags & IFF_DYING) != 0 || > + (state != IFNET_EVENT_UP && state != IFNET_EVENT_DOWN)) > + return; > + > + nhops_ifnet_state_changed(ifp, state == IFNET_EVENT_UP); > +} > + > +static void > +nhops_ifnet_link_event(void *arg __unused, struct ifnet *ifp, int state) > +{ > +#ifdef VIMAGE > + /* > + * rib_head will be calculated from V_tables in rt_tables_get_rnh > + * and the VNET is destroyed. > + */ > + if (VNET_IS_SHUTTING_DOWN(ifp->if_vnet)) > + return; > +#endif > + > + if ((ifp->if_flags & IFF_DYING) != 0 || > + (state != LINK_STATE_UP && state != LINK_STATE_DOWN)) > + return; > + > + nhops_ifnet_state_changed(ifp, state == LINK_STATE_UP); > +} > diff --git a/sys/net/route/route_ctl.h b/sys/net/route/route_ctl.h > index 845df8ce1fbe..2e34b2a35c25 100644 > --- a/sys/net/route/route_ctl.h > +++ b/sys/net/route/route_ctl.h > @@ -164,6 +164,7 @@ struct nhop_iter { > struct rib_head *rh; > int _i; > struct nhop_priv *_next; > + const bool wlock; > }; > > struct nhop_object *nhops_iter_start(struct nhop_iter *iter); > diff --git a/sys/netinet/in_fib.c b/sys/netinet/in_fib.c > index d8e373bf3dc2..d864bce07315 100644 > --- a/sys/netinet/in_fib.c > +++ b/sys/netinet/in_fib.c > @@ -117,7 +117,7 @@ fib4_lookup(uint32_t fibnum, struct in_addr dst, uint32_t scopeid, > if (nh != NULL) { > nh = nhop_select(nh, flowid); > /* Ensure route & ifp is UP */ > - if (RT_LINK_IS_UP(nh->nh_ifp)) { > + if (NH_IS_VALID(nh)) { > if (flags & NHR_REF) > nhop_ref_object(nh); > return (nh); > @@ -154,7 +154,7 @@ fib4_lookup(uint32_t fibnum, struct in_addr dst, uint32_t scopeid, > if (rn != NULL && ((rn->rn_flags & RNF_ROOT) == 0)) { > nh = nhop_select((RNTORT(rn))->rt_nhop, flowid); > /* Ensure route & ifp is UP */ > - if (RT_LINK_IS_UP(nh->nh_ifp)) { > + if (NH_IS_VALID(nh)) { > if (flags & NHR_REF) > nhop_ref_object(nh); > RIB_RUNLOCK(rh); > @@ -320,7 +320,7 @@ fib4_lookup_debugnet(uint32_t fibnum, struct in_addr dst, uint32_t scopeid, > if (rt != NULL) { > struct nhop_object *nh = nhop_select(rnd.rnd_nhop, 0); > /* Ensure route & ifp is UP */ > - if (RT_LINK_IS_UP(nh->nh_ifp)) > + if (NH_IS_VALID(nh)) > return (nh); > } > > diff --git a/sys/netinet6/in6_fib.c b/sys/netinet6/in6_fib.c > index c851a5bd14c5..594a9dc0a24e 100644 > --- a/sys/netinet6/in6_fib.c > +++ b/sys/netinet6/in6_fib.c > @@ -125,7 +125,7 @@ fib6_lookup(uint32_t fibnum, const struct in6_addr *dst6, > if (nh != NULL) { > nh = nhop_select(nh, flowid); > /* Ensure route & ifp is UP */ > - if (RT_LINK_IS_UP(nh->nh_ifp)) { > + if (NH_IS_VALID(nh)) { > if (flags & NHR_REF) > nhop_ref_object(nh); > return (nh); > @@ -163,7 +163,7 @@ fib6_lookup(uint32_t fibnum, const struct in6_addr *dst6, > if (rn != NULL && ((rn->rn_flags & RNF_ROOT) == 0)) { > nh = nhop_select((RNTORT(rn))->rt_nhop, flowid); > /* Ensure route & ifp is UP */ > - if (RT_LINK_IS_UP(nh->nh_ifp)) { > + if (NH_IS_VALID(nh)) { > if (flags & NHR_REF) > nhop_ref_object(nh); > RIB_RUNLOCK(rh); > @@ -335,7 +335,7 @@ fib6_lookup_debugnet(uint32_t fibnum, const struct in6_addr *dst6, > if (rt != NULL) { > struct nhop_object *nh = nhop_select(rnd.rnd_nhop, 0); > /* Ensure route & ifp is UP */ > - if (RT_LINK_IS_UP(nh->nh_ifp)) > + if (NH_IS_VALID(nh)) > return (nh); > } > > > -- Bjoern A. Zeeb r15:7home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3p674769-n07-n27p-o386-531r93405499>