On Wed, Nov 23, 2022 at 06:03:40PM +0000, Klemens Nanni wrote:
> "Push kernel lock into in6_ioctl()" on tech@ talks about ND6 locking,
> here is one cleanup and one documentation change (squashed) for this.
Complete diff with the missing ndp(8) bits and more background info.
Feedback? Objection? OK?
---
Remove struct nd_ifinfo's obsolete .initialized member
Added/set since 2002 sys/netinet6/nd6.c r1.42
attach nd_ifinfo structure to if_afdata.
split IPv6 MTU (advertised by RA) from real link MTU.
sync with kame
Read since 2002 usr.sbin/ndp/ndp.c r1.16
use new SIOCGIFINFO_IN6. random other cleanups. sync w/kame.
Obsolete since 2017 sys/netinet6/nd6.c r1.217
usr.sbin/ndp/ndp.c r1.85
Remove knob and always do neighbor unreachable detection.
SIOCGIFINFO_IN6 through ndp(8) is the only user of this.
nd6_ifattach() allocates and unconditionally initialises struct ifnet's
*if_nd member, early if_attachsetup() so that there is no way to query
unitialised Neighour Unreachable Detection bits.
So this is dead code from the 2017 commit that removed the possibility
to disable NUD.
===
Document struct nd_ifinfo protection
IPv6 Neighbour Discovery information is fully protected by the net lock.
All access to struct ifnet's member *if_nd is read-only, with the one
write exception being nd6_slowtimo() updating ND information.
The output of `grep -srw -e if_nd -e nd6if /sys/' shows these and fits
on a screen, probably with enough -C context to show locking details.
---
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index a3ae3b06dc4..fc404dce739 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -133,8 +133,6 @@ nd6_ifattach(struct ifnet *ifp)
nd = malloc(sizeof(*nd), M_IP6NDP, M_WAITOK | M_ZERO);
- nd->initialized = 1;
-
nd->basereachable = REACHABLE_TIME;
nd->reachable = ND_COMPUTE_RTIME(nd->basereachable);
nd->retrans = RETRANS_TIMER;
diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
index fe40b846180..45b1efdf0ec 100644
--- a/sys/netinet6/nd6.h
+++ b/sys/netinet6/nd6.h
@@ -43,12 +43,16 @@
#define ND6_LLINFO_DELAY 3
#define ND6_LLINFO_PROBE 4
+/*
+ * Locks used to protect struct members in this file:
+ * N net lock
+ */
+
struct nd_ifinfo {
- u_int32_t basereachable; /* BaseReachableTime */
- u_int32_t reachable; /* Reachable Time */
- u_int32_t retrans; /* Retrans Timer */
- int recalctm; /* BaseReacable re-calculation timer */
- u_int8_t initialized; /* Flag to see the entry is initialized */
+ u_int32_t basereachable; /* [N] BaseReachableTime */
+ u_int32_t reachable; /* [N] Reachable Time */
+ u_int32_t retrans; /* [N] Retrans Timer */
+ int recalctm; /* [N] BaseReachable recalc timer */
};
struct in6_nbrinfo {
diff --git a/usr.sbin/ndp/ndp.c b/usr.sbin/ndp/ndp.c
index 8b3cfeeb300..83147f7d748 100644
--- a/usr.sbin/ndp/ndp.c
+++ b/usr.sbin/ndp/ndp.c
@@ -888,9 +888,6 @@ ifinfo(const char *ifname)
if (ioctl(s, SIOCGIFINFO_IN6, (caddr_t)&nd) == -1)
err(1, "ioctl(SIOCGIFINFO_IN6)");
- if (!nd.ndi.initialized)
- errx(1, "%s: not initialized yet", ifname);
-
printf("basereachable=%ds%dms",
nd.ndi.basereachable / 1000, nd.ndi.basereachable % 1000);
printf(", reachable=%ds", nd.ndi.reachable);
