On 06/08/17(Sun) 17:04, Florian Obser wrote:
> On Sun, Aug 06, 2017 at 10:29:09AM -0400, Martin Pieuchot wrote:
> > On 03/08/17(Thu) 09:13, Florian Obser wrote:
> > > 
> > > as mpi pointed out in "nd6 address expiration & NET_LOCK() contention"
> > > we run nd6_expire every second. That seems a bit silly considering
> > > that we normally have a pltime of a day.
> > > 
> > > With a bit of math we can work out when the timer should fire when we
> > > set pltime/vltime and when we walk the list in nd6_expire.
> > > 
> > > This intentionally only lowers the timeout in in6_update_ifa(). That
> > > means in the case of no privacy addresses and default pltime/vltime
> > > and no router advertisements getting lost the timer fires every week
> > > and notices that it has nothing to do. I think that is acceptable
> > > since it fires 600.000 times less...
> > > 
> > > Tests, comments, OKs?
> > 
> > I like it, comments below.
> 
> next try, but I probably missunderstood your comments.

A bit, more comments below :)

> If I try to printf-debug this in nd6_expire_timer_update()

Are you trying to print something at an invalid address?

> panic(ffff8000221a6480,ffff8000221bd800,ffff8000221b9000,ffffffff8197bca5,0,fff
> f8000221bd698) at panic+0xab
> __mtx_enter(6,9791cee0ccf8fbcd,ffffff013f60f800,ffff8000221b9000,15000,9791cee0
> ccf8fbcd) at __mtx_enter+0x62

Here trap() calls printf() which tries to grab a mutex.  However the
current CPU is already holding this mutex because it faulted *inside*
printf().  Now __mtx_enter() will want to call panic() which wants to
call printf(), hence the loop.

> trap() at trap+0x51f
> --- trap (number 6) ---
> strlen(ffff800000106290,ffff8000003e8c00,ffff8000221bdda0,1,ffffffff81a558f6,97
> 91cee0ccf8fbcd) at strlen+0x20

strlen() faulted means that you gave something invalid to printf().

> printf(0,9791cee0ccf8fbcd,1,ffff8000221bd9b0,ffff8000221bda20,3000000010) at 
> pr
> intf+0x6b
> in6_update_ifa(ffff8000221bdda0,0,ffff800000106290,8080691a,0,9791cee0ccf8fbcd)
>  at in6_update_ifa+0xa5f
> in6_ioctl(6,8080691a,ffff800000106290,ffff8000221a6480,ffff8000221bdda0,ffff800
> 000106290) at in6_ioctl+0x4ed
> ifioctl(ffff8000221a6480,ffff8000221bdda0,8004667d,ffffff013f2a4bc0,8080691a,97
> 91cee0ccf8fbcd) at ifioctl+0x5e0
> soo_ioctl(80,ffffff01207d3850,8004667d,ffff8000221bde90,ffff8000221bdda0,ffffff
> ff81ab4200) at soo_ioctl+0x163
> sys_ioctl(ffff8000221bdf20,360,ffff8000221a6480,0,7f7ffffe0370,9791cee0ccf8fbcd
> ) at sys_ioctl+0x353
> syscall() at syscall+0x251
> --- syscall (number 54) ---
> end of kernel
> end trace frame: 0x7f7ffffe0310, count: -303
> diff --git sys/netinet6/in6.c sys/netinet6/in6.c
> index cafdd9fe36f..6f37a0a3418 100644
> --- sys/netinet6/in6.c
> +++ sys/netinet6/in6.c
> @@ -497,6 +497,7 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq 
> *ifra,
>       struct in6_addrlifetime *lt;
>       struct in6_multi_mship *imm;
>       struct rtentry *rt;
> +     time_t expire_time = -1;
>       char addr[INET6_ADDRSTRLEN];
>  
>       NET_ASSERT_LOCKED();
> @@ -686,6 +687,10 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq 
> *ifra,
>        */
>       ia6->ia6_flags = ifra->ifra_flags;
>  
> +     nd6_next_expire_time(ia6, &expire_time);
> +
> +     nd6_expire_timer_update(expire_time, 0);

I was suggesting to put everything in one function.

Reply via email to