On Sun, Sep 04, 2022 at 08:53:45AM +0200, Stefan Sperling wrote:
> On Sat, Aug 27, 2022 at 11:32:24PM +0300, Vitaliy Makkoveev wrote:
> > > On 27 Aug 2022, at 22:03, Alexander Bluhm <[email protected]> wrote:
> > >
> > > On Sat, Aug 27, 2022 at 03:14:15AM +0300, Vitaliy Makkoveev wrote:
> > >>> On 27 Aug 2022, at 00:04, Alexander Bluhm <[email protected]>
> > >>> wrote:
> > >>>
> > >>> Anyone willing to test or ok this?
> > >>>
> > >>
> > >> This fixes weird `ifa??? refcounting. I like this.
> > >>
> > >> Could the ifaref() and ifafree() names use the same notation? Like
> > >> ifaref() and ifarele() or ifaget() and ifafree() or something else?
> > >
> > > Refcount naming is very inconsistent.
> > >
> > > ifget(), ifput(), pf_state_key_ref(), pf_state_key_unref(), tdb_ref(),
> > > tdb_unref(), tdb_delete(), tdb_free(), vxlan_take(), vxlan_rele()
> > > all work in subtle different ways.
> > >
> > > I want to keep ifafree() as the name is established and called from
> > > many places. And giving ifaref() another name makes it different
> > > but not better.
> > >
> > > It would be easy to change something but hard to make it consistent.
> > > So I prefer to leave the diff as it is.
> > >
> > > bluhm
> >
> > I have no objections to commit this diff.
>
> The diff has been committed but the problem remains:
>
> OpenBSD 7.2-beta (GENERIC.MP) #2: Thu Sep 1 18:54:34 CEST 2022
>
> [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>
> login: kernel: protection fault trap, code=0
> Stopped at rt_ifa_del+0x39: movb 0x1b6(%rax),%bl
> ddb{3}> bt
> rt_ifa_del(ffff800000496c00,800100,dead0009dead4110,0) at rt_ifa_del+0x39
> in6_unlink_ifa(ffff800000496c00,ffff8000000da2a8) at in6_unlink_ifa+0xae
> in6_purgeaddr(ffff800000496c00) at in6_purgeaddr+0x127
> nd6_expire(0) at nd6_expire+0x96
> taskq_thread(ffff80000002c080) at taskq_thread+0x100
> end trace frame: 0x0, count: -5
> ddb{3}> show struct ifaddr 0xffff800000496c00
> struct ifaddr at 0xffff800000496c00 (64 bytes) {ifa_addr = (struct sockaddr
> *)0
> xdead0009dead4110, ifa_dstaddr = (struct sockaddr *)0x4002e6f6e3c87f50,
> ifa_net
> mask = (struct sockaddr *)0xdead4110dead4110, ifa_ifp = (struct ifnet
> *)0xdead4
> 110dead4110, ifa_list = {tqe_next = (struct ifaddr *)0xdead4110dead4110,
> tqe_pr
> ev = 0xdead4110dead4110}, ifa_flags = 0xdead4110, ifa_refcnt = {r_refs =
> 0xdead
> 4110, r_traceidx = 0xdead4110}, ifa_metric = 0xdead4110}
> ddb{3}>
>
Glancing at nd6_expire()... does this diff help?
Index: sys/netinet6/nd6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.246
diff -u -p -r1.246 nd6.c
--- sys/netinet6/nd6.c 9 Aug 2022 21:10:03 -0000 1.246
+++ sys/netinet6/nd6.c 4 Sep 2022 09:26:15 -0000
@@ -496,7 +496,7 @@ nd6_expire(void *unused)
TAILQ_FOREACH_SAFE(ifa, &ifp->if_addrlist, ifa_list, nifa) {
if (ifa->ifa_addr->sa_family != AF_INET6)
continue;
- ia6 = ifatoia6(ifa);
+ ia6 = ifatoia6(ifaref(ifa));
/* check address lifetime */
if (IFA6_IS_INVALID(ia6)) {
in6_purgeaddr(&ia6->ia_ifa);
