Date: Mon, 22 Sep 2025 02:26:12 +0200 From: Guido Falsi <madpilot@FreeBSD.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: "Herbert J. Skuhra" <herbert@gojira.at>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 31ec8b6407fd - main - sys/netinet6: Implement RFC 7217 Message-ID: <63c07846-cd1b-4ff2-8c35-c0a6db6954cc@FreeBSD.org> In-Reply-To: <aNCDmk7dcv8jE7yi@cell.glebi.us> References: <202509201231.58KCVqBC047480@gitrepo.freebsd.org> <874iswhip4.wl-herbert@gojira.at> <bad8cb94-8243-468a-9919-a713a9426eae@FreeBSD.org> <07503de1-785e-4e4d-b4e4-0524aeb064e1@FreeBSD.org> <87jz1sc9fr.wl-herbert@gojira.at> <31da7dd5-ae67-4fb4-aa47-81e57f460c9d@FreeBSD.org> <aNBKeJwyBFpkt77b@cell.glebi.us> <909b3e33-0639-4abc-915f-073cd0f304da@FreeBSD.org> <aNCDmk7dcv8jE7yi@cell.glebi.us>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
On 9/22/25 01:00, Gleb Smirnoff wrote:
> On Sun, Sep 21, 2025 at 09:17:15PM +0200, Guido Falsi wrote:
> G> > Is struct nd_ifinfo something that is used as argument to ioctl(2)?
> G> > If so, then adding counter_u64_t into it is not correct.
> G> >
> G> > I would suggest to not revert the change as a whole, but remove the
> G> > counter only and bring struct nd_ifinfo back to original. The counter
> G> > should probably go into struct in6_ifextra that is pointed by
> G> > if->if_afdata[AF_INET6].
> G>
> G> Thanks for the feedback and suggestion!
> G>
> G> You are definitely right! I'm in the process to create a review for such a
> G> change shortly, I'll post it as soon as I'm able to have the code tested.
>
> As long as Herbert confirms that the patch fixes the issue for him, please
> consider the change as approved by an src committer and please push it. We
> want to enter the stabweek that starts tomorrow without a ABI breakage.
>
@Herbert I'm attaching a patch against base sources for you to test.
It moves the new counter to in6_ifextra structure, removing the ABI
breakage I introduced. Everything should be back to working with this.
Can you confirm it fixes the issue for you?
Thanks in advance!
--
Guido Falsi <madpilot@FreeBSD.org>
[-- Attachment #2 --]
From b9b814a6bfc84818affce9243172e4b591e02215 Mon Sep 17 00:00:00 2001
From: Guido Falsi <madpilot@FreeBSD.org>
Date: Sun, 21 Sep 2025 19:23:28 +0200
Subject: [PATCH] Move dad_failures to in6_ifextra
---
sys/netinet6/in6.c | 3 +++
sys/netinet6/in6_ifattach.c | 2 +-
sys/netinet6/in6_var.h | 2 ++
sys/netinet6/nd6.c | 4 ----
sys/netinet6/nd6.h | 1 -
sys/netinet6/nd6_nbr.c | 6 +++---
sys/netinet6/nd6_rtr.c | 2 +-
7 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
index 4f756a75fac7..8ef755e2dc0a 100644
--- a/sys/netinet6/in6.c
+++ b/sys/netinet6/in6.c
@@ -2604,6 +2604,8 @@ in6_domifattach(struct ifnet *ifp)
COUNTER_ARRAY_ALLOC(ext->icmp6_ifstat,
sizeof(struct icmp6_ifstat) / sizeof(uint64_t), M_WAITOK);
+ ext->dad_failures = counter_u64_alloc(M_WAITOK);
+
ext->nd_ifinfo = nd6_ifattach(ifp);
ext->scope6_id = scope6_ifattach(ifp);
ext->lltable = in6_lltattach(ifp);
@@ -2639,6 +2641,7 @@ in6_domifdetach(struct ifnet *ifp, void *aux)
COUNTER_ARRAY_FREE(ext->icmp6_ifstat,
sizeof(struct icmp6_ifstat) / sizeof(uint64_t));
free(ext->icmp6_ifstat, M_IFADDR);
+ counter_u64_free(ext->dad_failures);
free(ext, M_IFADDR);
}
diff --git a/sys/netinet6/in6_ifattach.c b/sys/netinet6/in6_ifattach.c
index 57fe12a1c93b..4fde346fb691 100644
--- a/sys/netinet6/in6_ifattach.c
+++ b/sys/netinet6/in6_ifattach.c
@@ -377,7 +377,7 @@ in6_get_stableifid(struct ifnet *ifp, struct in6_addr *in6, int prefixlen)
}
hostuuid_len = strlen(hostuuid);
- dad_failures = counter_u64_fetch(ND_IFINFO(ifp)->dad_failures);
+ dad_failures = counter_u64_fetch(DAD_FAILURES(ifp));
/*
* RFC 7217 section 7
diff --git a/sys/netinet6/in6_var.h b/sys/netinet6/in6_var.h
index e5ab83e6a2a1..e511ead24f08 100644
--- a/sys/netinet6/in6_var.h
+++ b/sys/netinet6/in6_var.h
@@ -106,9 +106,11 @@ struct in6_ifextra {
struct scope6_id *scope6_id;
struct lltable *lltable;
struct mld_ifsoftc *mld_ifinfo;
+ counter_u64_t dad_failures; /* DAD failures when using RFC 7217 stable addresses */
};
#define LLTABLE6(ifp) (((struct in6_ifextra *)(ifp)->if_afdata[AF_INET6])->lltable)
+#define DAD_FAILURES(ifp) (((struct in6_ifextra *)(ifp)->if_afdata[AF_INET6])->dad_failures)
#ifdef _KERNEL
diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index 938d411711f0..00df5efcef92 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -329,8 +329,6 @@ nd6_ifattach(struct ifnet *ifp)
nd->flags |= ND6_IFF_STABLEADDR;
}
- nd->dad_failures = counter_u64_alloc(M_WAITOK);
-
return nd;
}
@@ -350,8 +348,6 @@ nd6_ifdetach(struct ifnet *ifp, struct nd_ifinfo *nd)
}
NET_EPOCH_EXIT(et);
- counter_u64_free(nd->dad_failures);
-
free(nd, M_IP6NDP);
}
diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
index 1de2a77ddf6d..5fe027ac5e7c 100644
--- a/sys/netinet6/nd6.h
+++ b/sys/netinet6/nd6.h
@@ -76,7 +76,6 @@ struct nd_ifinfo {
u_int8_t randomseed0[8]; /* upper 64 bits of MD5 digest */
u_int8_t randomseed1[8]; /* lower 64 bits (usually the EUI64 IFID) */
u_int8_t randomid[8]; /* current random ID */
- counter_u64_t dad_failures; /* DAD failures when using RFC 7217 stable addresses */
};
#define ND6_IFF_PERFORMNUD 0x1
diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
index 76b1fd86ee08..cc17b4e1a402 100644
--- a/sys/netinet6/nd6_nbr.c
+++ b/sys/netinet6/nd6_nbr.c
@@ -1473,7 +1473,7 @@ nd6_dad_timer(void *arg)
if ((ND_IFINFO(ifp)->flags & ND6_IFF_IFDISABLED) == 0) {
ia->ia6_flags &= ~IN6_IFF_TENTATIVE;
if ((ND_IFINFO(ifp)->flags & ND6_IFF_STABLEADDR) && !(ia->ia6_flags & IN6_IFF_TEMPORARY))
- counter_u64_zero(ND_IFINFO(ifp)->dad_failures);
+ counter_u64_zero(DAD_FAILURES(ifp));
}
nd6log((LOG_DEBUG,
@@ -1522,10 +1522,10 @@ nd6_dad_duplicated(struct ifaddr *ifa, struct dadq *dp)
* More addresses will be generated as long as retries are not exhausted.
*/
if ((ND_IFINFO(ifp)->flags & ND6_IFF_STABLEADDR) && !(ia->ia6_flags & IN6_IFF_TEMPORARY)) {
- uint64_t dad_failures = counter_u64_fetch(ND_IFINFO(ifp)->dad_failures);
+ uint64_t dad_failures = counter_u64_fetch(DAD_FAILURES(ifp));
if (dad_failures <= V_ip6_stableaddr_maxretries) {
- counter_u64_add(ND_IFINFO(ifp)->dad_failures, 1);
+ counter_u64_add(DAD_FAILURES(ifp), 1);
/* if retries exhausted, output an informative error message */
if (dad_failures == V_ip6_stableaddr_maxretries)
log(LOG_ERR, "%s: manual intervention required, consider disabling \"stableaddr\" on the interface"
diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index 01623a4506be..78dc55dd292f 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1757,7 +1757,7 @@ prelist_update(struct nd_prefixctl *new, struct nd_defrouter *dr,
* to fail and no further retries should happen.
*/
if (ND_IFINFO(ifp)->flags & ND6_IFF_STABLEADDR &&
- counter_u64_fetch(ND_IFINFO(ifp)->dad_failures) <= V_ip6_stableaddr_maxretries &&
+ counter_u64_fetch(DAD_FAILURES(ifp)) <= V_ip6_stableaddr_maxretries &&
ifa6->ia6_flags & (IN6_IFF_DUPLICATED | IN6_IFF_TEMPORARY))
continue;
--
2.51.0
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?63c07846-cd1b-4ff2-8c35-c0a6db6954cc>