Re: ND6 and splsoftnet()

2016-12-23 Thread Alexander Bluhm
On Thu, Dec 22, 2016 at 02:03:51PM +0100, Alexander Bluhm wrote:
> Fine.  But let's do the other changes.  Move timer initialisation
> to nd6_init() and call timeout_set() only once during init.  Then
> I don't have to think about wether it is MP safe.

updated diff, parts have been commited

ok?

bluhm

Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.172
diff -u -p -r1.172 ip6_input.c
--- netinet6/ip6_input.c20 Dec 2016 18:33:43 -  1.172
+++ netinet6/ip6_input.c23 Dec 2016 17:57:38 -
@@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA
 
 struct ip6stat ip6stat;
 
-void ip6_init2(void *);
 int ip6_check_rh0hdr(struct mbuf *, int *);
 
 int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
@@ -157,19 +156,8 @@ ip6_init(void)
ip6_randomid_init();
nd6_init();
frag6_init();
-   ip6_init2(NULL);
 
mq_init(_mq, 64, IPL_SOFTNET);
-}
-
-void
-ip6_init2(void *dummy)
-{
-
-   /* nd6_timer_init */
-   bzero(_timer_ch, sizeof(nd6_timer_ch));
-   timeout_set(_timer_ch, nd6_timer, NULL);
-   timeout_add_sec(_timer_ch, 1);
 }
 
 /*
Index: netinet6/nd6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.201
diff -u -p -r1.201 nd6.c
--- netinet6/nd6.c  23 Dec 2016 15:08:54 -  1.201
+++ netinet6/nd6.c  23 Dec 2016 17:59:30 -
@@ -93,6 +93,8 @@ struct nd_prhead nd_prefix = { 0 };
 int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
 
 void nd6_slowtimo(void *);
+void nd6_timer_work(void *);
+void nd6_timer(void *);
 void nd6_invalidate(struct rtentry *);
 struct llinfo_nd6 *nd6_free(struct rtentry *, int);
 void nd6_llinfo_timer(void *);
@@ -100,7 +102,6 @@ void nd6_llinfo_timer(void *);
 struct timeout nd6_slowtimo_ch;
 struct timeout nd6_timer_ch;
 struct task nd6_timer_task;
-void nd6_timer_work(void *);
 
 int fill_drlist(void *, size_t *, size_t);
 int fill_prlist(void *, size_t *, size_t);
@@ -129,6 +130,8 @@ nd6_init(void)
/* start timer */
timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
+   timeout_set(_timer_ch, nd6_timer, NULL);
+   timeout_add_sec(_timer_ch, nd6_prune);
 
nd6_rs_init();
 }
@@ -437,7 +440,6 @@ nd6_timer_work(void *null)
 
NET_LOCK(s);
 
-   timeout_set(_timer_ch, nd6_timer, NULL);
timeout_add_sec(_timer_ch, nd6_prune);
 
/* expire default router list */
Index: netinet6/nd6.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.65
diff -u -p -r1.65 nd6.h
--- netinet6/nd6.h  28 Nov 2016 13:59:51 -  1.65
+++ netinet6/nd6.h  23 Dec 2016 17:57:38 -
@@ -223,8 +223,6 @@ extern int nd6_debug;
 
 #define nd6log(x)  do { if (nd6_debug) log x; } while (0)
 
-extern struct timeout nd6_timer_ch;
-
 union nd_opts {
struct nd_opt_hdr *nd_opt_array[9];
struct {
@@ -260,7 +258,6 @@ int nd6_options(union nd_opts *);
 struct rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, u_int);
 void nd6_setmtu(struct ifnet *);
 void nd6_llinfo_settimer(struct llinfo_nd6 *, int);
-void nd6_timer(void *);
 void nd6_purge(struct ifnet *);
 void nd6_nud_hint(struct rtentry *);
 void nd6_rtrequest(struct ifnet *, int, struct rtentry *);



Re: ND6 and splsoftnet()

2016-12-22 Thread Alexander Bluhm
On Wed, Dec 21, 2016 at 07:51:03PM +0100, Martin Pieuchot wrote:
> > This diff replaces the nd6_timer timeout and task with a timeout
> > proc.
> 
> Can we wait until the experiment is complete?  Until then I'd like
> to keep timeout_set_proc() only for timeout converted because they
> need the NET_LOCK().  

Fine.  But let's do the other changes.  Move timer initialisation
to nd6_init() and call timeout_set() only once during init.  Then
I don't have to think about wether it is MP safe.

ok?

bluhm

Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.172
diff -u -p -r1.172 ip6_input.c
--- netinet6/ip6_input.c20 Dec 2016 18:33:43 -  1.172
+++ netinet6/ip6_input.c22 Dec 2016 12:51:04 -
@@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA
 
 struct ip6stat ip6stat;
 
-void ip6_init2(void *);
 int ip6_check_rh0hdr(struct mbuf *, int *);
 
 int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
@@ -157,19 +156,8 @@ ip6_init(void)
ip6_randomid_init();
nd6_init();
frag6_init();
-   ip6_init2(NULL);
 
mq_init(_mq, 64, IPL_SOFTNET);
-}
-
-void
-ip6_init2(void *dummy)
-{
-
-   /* nd6_timer_init */
-   bzero(_timer_ch, sizeof(nd6_timer_ch));
-   timeout_set(_timer_ch, nd6_timer, NULL);
-   timeout_add_sec(_timer_ch, 1);
 }
 
 /*
Index: netinet6/nd6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.199
diff -u -p -r1.199 nd6.c
--- netinet6/nd6.c  20 Dec 2016 18:33:43 -  1.199
+++ netinet6/nd6.c  22 Dec 2016 12:59:31 -
@@ -93,6 +93,8 @@ struct nd_prhead nd_prefix = { 0 };
 int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
 
 void nd6_slowtimo(void *);
+void nd6_timer_work(void *);
+void nd6_timer(void *);
 void nd6_invalidate(struct rtentry *);
 struct llinfo_nd6 *nd6_free(struct rtentry *, int);
 void nd6_llinfo_timer(void *);
@@ -100,7 +102,6 @@ void nd6_llinfo_timer(void *);
 struct timeout nd6_slowtimo_ch;
 struct timeout nd6_timer_ch;
 struct task nd6_timer_task;
-void nd6_timer_work(void *);
 
 int fill_drlist(void *, size_t *, size_t);
 int fill_prlist(void *, size_t *, size_t);
@@ -129,6 +130,8 @@ nd6_init(void)
/* start timer */
timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
+   timeout_set(_timer_ch, nd6_timer, NULL);
+   timeout_add_sec(_timer_ch, nd6_prune);
 
nd6_rs_init();
 }
@@ -436,7 +439,6 @@ nd6_timer_work(void *null)
struct in6_ifaddr *ia6, *nia6;
 
s = splsoftnet();
-   timeout_set(_timer_ch, nd6_timer, NULL);
timeout_add_sec(_timer_ch, nd6_prune);
 
/* expire default router list */
@@ -1482,7 +1484,6 @@ nd6_slowtimo(void *ignored_arg)
struct nd_ifinfo *nd6if;
struct ifnet *ifp;
 
-   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
TAILQ_FOREACH(ifp, , if_list) {
nd6if = ND_IFINFO(ifp);
Index: netinet6/nd6.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.65
diff -u -p -r1.65 nd6.h
--- netinet6/nd6.h  28 Nov 2016 13:59:51 -  1.65
+++ netinet6/nd6.h  22 Dec 2016 12:52:54 -
@@ -223,8 +223,6 @@ extern int nd6_debug;
 
 #define nd6log(x)  do { if (nd6_debug) log x; } while (0)
 
-extern struct timeout nd6_timer_ch;
-
 union nd_opts {
struct nd_opt_hdr *nd_opt_array[9];
struct {
@@ -260,7 +258,6 @@ int nd6_options(union nd_opts *);
 struct rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, u_int);
 void nd6_setmtu(struct ifnet *);
 void nd6_llinfo_settimer(struct llinfo_nd6 *, int);
-void nd6_timer(void *);
 void nd6_purge(struct ifnet *);
 void nd6_nud_hint(struct rtentry *);
 void nd6_rtrequest(struct ifnet *, int, struct rtentry *);



Re: ND6 and splsoftnet()

2016-12-21 Thread Martin Pieuchot
On 21/12/16(Wed) 18:22, Alexander Bluhm wrote:
> On Wed, Dec 21, 2016 at 05:21:53PM +0100, Alexander Bluhm wrote:
> > > @@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
> > >  void
> > >  nd6_timer_work(void *null)
> > >  {
> > > - int s;
> > >   struct nd_defrouter *dr, *ndr;
> > >   struct nd_prefix *pr, *npr;
> > >   struct in6_ifaddr *ia6, *nia6;
> > > + int s;
> > >  
> > > - s = splsoftnet();
> > >   timeout_set(_timer_ch, nd6_timer, NULL);
> > >   timeout_add_sec(_timer_ch, nd6_prune);
> > >  
> > > + NET_LOCK(s);
> > > +
> > 
> > Moving timeout_set() out of the splsoftnet() feels wrong.  The whole
> > task construct could be replaced with a timeout_set_proc(), I will
> > make a diff.
> 
> This diff replaces the nd6_timer timeout and task with a timeout
> proc.

Can we wait until the experiment is complete?  Until then I'd like
to keep timeout_set_proc() only for timeout converted because they
need the NET_LOCK().  

If we discover a flaw and need to "back out" we can simply do

#define timeout_set_proc(x) timeout_set

and

#define NET_LOCK(s) s = splsoftnet()

But if we start to convert other stuff, like the diff below does, this
wont work.

> 
> ok?
> 
> bluhm
> 
> Index: netinet6/ip6_input.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.172
> diff -u -p -r1.172 ip6_input.c
> --- netinet6/ip6_input.c  20 Dec 2016 18:33:43 -  1.172
> +++ netinet6/ip6_input.c  21 Dec 2016 16:49:17 -
> @@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA
>  
>  struct ip6stat ip6stat;
>  
> -void ip6_init2(void *);
>  int ip6_check_rh0hdr(struct mbuf *, int *);
>  
>  int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
> @@ -157,19 +156,8 @@ ip6_init(void)
>   ip6_randomid_init();
>   nd6_init();
>   frag6_init();
> - ip6_init2(NULL);
>  
>   mq_init(_mq, 64, IPL_SOFTNET);
> -}
> -
> -void
> -ip6_init2(void *dummy)
> -{
> -
> - /* nd6_timer_init */
> - bzero(_timer_ch, sizeof(nd6_timer_ch));
> - timeout_set(_timer_ch, nd6_timer, NULL);
> - timeout_add_sec(_timer_ch, 1);
>  }
>  
>  /*
> Index: netinet6/nd6.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.199
> diff -u -p -r1.199 nd6.c
> --- netinet6/nd6.c20 Dec 2016 18:33:43 -  1.199
> +++ netinet6/nd6.c21 Dec 2016 17:21:12 -
> @@ -93,14 +93,13 @@ struct nd_prhead nd_prefix = { 0 };
>  int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
>  
>  void nd6_slowtimo(void *);
> +void nd6_timer(void *);
>  void nd6_invalidate(struct rtentry *);
>  struct llinfo_nd6 *nd6_free(struct rtentry *, int);
>  void nd6_llinfo_timer(void *);
>  
>  struct timeout nd6_slowtimo_ch;
>  struct timeout nd6_timer_ch;
> -struct task nd6_timer_task;
> -void nd6_timer_work(void *);
>  
>  int fill_drlist(void *, size_t *, size_t);
>  int fill_prlist(void *, size_t *, size_t);
> @@ -122,13 +121,13 @@ nd6_init(void)
>   /* initialization of the default router list */
>   TAILQ_INIT(_defrouter);
>  
> - task_set(_timer_task, nd6_timer_work, NULL);
> -
>   nd6_init_done = 1;
>  
>   /* start timer */
>   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
>   timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
> + timeout_set_proc(_timer_ch, nd6_timer, NULL);
> + timeout_add_sec(_timer_ch, nd6_prune);
>  
>   nd6_rs_init();
>  }
> @@ -428,7 +427,7 @@ nd6_llinfo_timer(void *arg)
>   * ND6 timer routine to expire default route list and prefix list
>   */
>  void
> -nd6_timer_work(void *null)
> +nd6_timer(void *arg)
>  {
>   int s;
>   struct nd_defrouter *dr, *ndr;
> @@ -436,7 +435,6 @@ nd6_timer_work(void *null)
>   struct in6_ifaddr *ia6, *nia6;
>  
>   s = splsoftnet();
> - timeout_set(_timer_ch, nd6_timer, NULL);
>   timeout_add_sec(_timer_ch, nd6_prune);
>  
>   /* expire default router list */
> @@ -483,12 +481,6 @@ nd6_timer_work(void *null)
>   }
>   }
>   splx(s);
> -}
> -
> -void
> -nd6_timer(void *ignored_arg)
> -{
> - task_add(systq, _timer_task);
>  }
>  
>  /*
> Index: netinet6/nd6.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
> retrieving revision 1.65
> diff -u -p -r1.65 nd6.h
> --- netinet6/nd6.h28 Nov 2016 13:59:51 -  1.65
> +++ netinet6/nd6.h21 Dec 2016 16:48:46 -
> @@ -223,8 +223,6 @@ extern int nd6_debug;
>  
>  #define nd6log(x)do { if (nd6_debug) log x; } while (0)
>  
> -extern struct timeout nd6_timer_ch;
> -
>  union nd_opts {
>   struct nd_opt_hdr *nd_opt_array[9];
>   struct {
> @@ -260,7 +258,6 @@ int nd6_options(union nd_opts *);
>  struct   rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, 
> u_int);
>  

Re: ND6 and splsoftnet()

2016-12-21 Thread Alexander Bluhm
On Wed, Dec 21, 2016 at 05:21:53PM +0100, Alexander Bluhm wrote:
> > @@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
> >  void
> >  nd6_timer_work(void *null)
> >  {
> > -   int s;
> > struct nd_defrouter *dr, *ndr;
> > struct nd_prefix *pr, *npr;
> > struct in6_ifaddr *ia6, *nia6;
> > +   int s;
> >  
> > -   s = splsoftnet();
> > timeout_set(_timer_ch, nd6_timer, NULL);
> > timeout_add_sec(_timer_ch, nd6_prune);
> >  
> > +   NET_LOCK(s);
> > +
> 
> Moving timeout_set() out of the splsoftnet() feels wrong.  The whole
> task construct could be replaced with a timeout_set_proc(), I will
> make a diff.

This diff replaces the nd6_timer timeout and task with a timeout
proc.

ok?

bluhm

Index: netinet6/ip6_input.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.172
diff -u -p -r1.172 ip6_input.c
--- netinet6/ip6_input.c20 Dec 2016 18:33:43 -  1.172
+++ netinet6/ip6_input.c21 Dec 2016 16:49:17 -
@@ -119,7 +119,6 @@ struct niqueue ip6intrq = NIQUEUE_INITIA
 
 struct ip6stat ip6stat;
 
-void ip6_init2(void *);
 int ip6_check_rh0hdr(struct mbuf *, int *);
 
 int ip6_hbhchcheck(struct mbuf *, int *, int *, int *);
@@ -157,19 +156,8 @@ ip6_init(void)
ip6_randomid_init();
nd6_init();
frag6_init();
-   ip6_init2(NULL);
 
mq_init(_mq, 64, IPL_SOFTNET);
-}
-
-void
-ip6_init2(void *dummy)
-{
-
-   /* nd6_timer_init */
-   bzero(_timer_ch, sizeof(nd6_timer_ch));
-   timeout_set(_timer_ch, nd6_timer, NULL);
-   timeout_add_sec(_timer_ch, 1);
 }
 
 /*
Index: netinet6/nd6.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.199
diff -u -p -r1.199 nd6.c
--- netinet6/nd6.c  20 Dec 2016 18:33:43 -  1.199
+++ netinet6/nd6.c  21 Dec 2016 17:21:12 -
@@ -93,14 +93,13 @@ struct nd_prhead nd_prefix = { 0 };
 int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
 
 void nd6_slowtimo(void *);
+void nd6_timer(void *);
 void nd6_invalidate(struct rtentry *);
 struct llinfo_nd6 *nd6_free(struct rtentry *, int);
 void nd6_llinfo_timer(void *);
 
 struct timeout nd6_slowtimo_ch;
 struct timeout nd6_timer_ch;
-struct task nd6_timer_task;
-void nd6_timer_work(void *);
 
 int fill_drlist(void *, size_t *, size_t);
 int fill_prlist(void *, size_t *, size_t);
@@ -122,13 +121,13 @@ nd6_init(void)
/* initialization of the default router list */
TAILQ_INIT(_defrouter);
 
-   task_set(_timer_task, nd6_timer_work, NULL);
-
nd6_init_done = 1;
 
/* start timer */
timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
+   timeout_set_proc(_timer_ch, nd6_timer, NULL);
+   timeout_add_sec(_timer_ch, nd6_prune);
 
nd6_rs_init();
 }
@@ -428,7 +427,7 @@ nd6_llinfo_timer(void *arg)
  * ND6 timer routine to expire default route list and prefix list
  */
 void
-nd6_timer_work(void *null)
+nd6_timer(void *arg)
 {
int s;
struct nd_defrouter *dr, *ndr;
@@ -436,7 +435,6 @@ nd6_timer_work(void *null)
struct in6_ifaddr *ia6, *nia6;
 
s = splsoftnet();
-   timeout_set(_timer_ch, nd6_timer, NULL);
timeout_add_sec(_timer_ch, nd6_prune);
 
/* expire default router list */
@@ -483,12 +481,6 @@ nd6_timer_work(void *null)
}
}
splx(s);
-}
-
-void
-nd6_timer(void *ignored_arg)
-{
-   task_add(systq, _timer_task);
 }
 
 /*
Index: netinet6/nd6.h
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.65
diff -u -p -r1.65 nd6.h
--- netinet6/nd6.h  28 Nov 2016 13:59:51 -  1.65
+++ netinet6/nd6.h  21 Dec 2016 16:48:46 -
@@ -223,8 +223,6 @@ extern int nd6_debug;
 
 #define nd6log(x)  do { if (nd6_debug) log x; } while (0)
 
-extern struct timeout nd6_timer_ch;
-
 union nd_opts {
struct nd_opt_hdr *nd_opt_array[9];
struct {
@@ -260,7 +258,6 @@ int nd6_options(union nd_opts *);
 struct rtentry *nd6_lookup(struct in6_addr *, int, struct ifnet *, u_int);
 void nd6_setmtu(struct ifnet *);
 void nd6_llinfo_settimer(struct llinfo_nd6 *, int);
-void nd6_timer(void *);
 void nd6_purge(struct ifnet *);
 void nd6_nud_hint(struct rtentry *);
 void nd6_rtrequest(struct ifnet *, int, struct rtentry *);



Re: ND6 and splsoftnet()

2016-12-21 Thread Alexander Bluhm
On Wed, Dec 21, 2016 at 01:52:45PM +0100, Martin Pieuchot wrote:
> ok?

OK bluhm@, but comments inline

> @@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
>  void
>  nd6_timer_work(void *null)
>  {
> - int s;
>   struct nd_defrouter *dr, *ndr;
>   struct nd_prefix *pr, *npr;
>   struct in6_ifaddr *ia6, *nia6;
> + int s;
>  
> - s = splsoftnet();
>   timeout_set(_timer_ch, nd6_timer, NULL);
>   timeout_add_sec(_timer_ch, nd6_prune);
>  
> + NET_LOCK(s);
> +

Moving timeout_set() out of the splsoftnet() feels wrong.  The whole
task construct could be replaced with a timeout_set_proc(), I will
make a diff.


> @@ -1120,7 +1121,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
>   struct in6_nbrinfo *nbi = (struct in6_nbrinfo *)data;
>   struct rtentry *rt;
>   int error = 0;
> - int s;
>  

Could you add an assert here when you remove the splsoftnet().  This
makes further review easier and we see bugs during runtime.

> @@ -1478,12 +1471,14 @@ fail:
>  void
>  nd6_slowtimo(void *ignored_arg)
>  {
> - int s = splsoftnet();
>   struct nd_ifinfo *nd6if;
>   struct ifnet *ifp;
> + int s;
>  
>   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
>   timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
> +
> + NET_LOCK(s);

Again timeout_set() without lock feels wrong.  It has been done in
nd6_init() already.  And there it is a timeout_set_proc() now.
Remove the line.

> @@ -825,7 +825,6 @@ nd6_na_input(struct mbuf *m, int off, in
>*/
>   struct nd_defrouter *dr;
>   struct in6_addr *in6;
> - int s;

Please add an assert.

>  nd6_dad_starttimer(struct dadq *dp, int ticks)
>  {
>  
> - timeout_set(>dad_timer_ch, nd6_dad_timer, dp->dad_ifa);
> + timeout_set_proc(>dad_timer_ch, nd6_dad_timer, dp->dad_ifa);
>   timeout_add(>dad_timer_ch, ticks);

Are we sure that timeout_set() is not called when the timer is
already running?  I will have a look.  You diff does not make it
worse.

> @@ -202,7 +202,7 @@ nd6_rs_output(struct ifnet* ifp, struct 
>   struct nd_router_solicit *rs;
>   struct ip6_moptions im6o;
>   caddr_t mac;
> - int icmp6len, maxlen, s;
> + int icmp6len, maxlen;

Can you put an assert here?

> @@ -1592,9 +1592,8 @@ pfxlist_onlink_check(void)
>   if ((pr->ndpr_stateflags & NDPRF_DETACHED) != 0 &&
>   (pr->ndpr_stateflags & NDPRF_ONLINK) != 0) {
>   if ((e = nd6_prefix_offlink(pr)) != 0) {
> - nd6log((LOG_ERR,
> - "pfxlist_onlink_check: failed to "
> - "make %s/%d offlink, errno=%d\n",
> + nd6log((LOG_ERR, "%s: failed to make %s/%d"
> + " offlink, errno=%d\n", __func__,
>   inet_ntop(AF_INET6,
>   >ndpr_prefix.sin6_addr,
>   addr, sizeof(addr)),
> @@ -1605,9 +1604,8 @@ pfxlist_onlink_check(void)
>   (pr->ndpr_stateflags & NDPRF_ONLINK) == 0 &&
>   pr->ndpr_raf_onlink) {
>   if ((e = nd6_prefix_onlink(pr)) != 0) {
> - nd6log((LOG_ERR,
> - "pfxlist_onlink_check: failed to "
> - "make %s/%d offlink, errno=%d\n",
> + nd6log((LOG_ERR, "%s: failed to make %s/%d"
> + " offlink, errno=%d\n", __func__,
>   inet_ntop(AF_INET6,
>   >ndpr_prefix.sin6_addr,
>   addr, sizeof(addr)),
> @@ -2000,7 +1998,7 @@ in6_init_address_ltimes(struct nd_prefix

Plaese keep the style that the space is before the " at the end
of the line.



ND6 and splsoftnet()

2016-12-21 Thread Martin Pieuchot
This diff get rids of all splsoftet() in nd6 code.

I converted timers to the NET_LOCK() even if ip6_output() doesn't assert
for it for the moment.  That's simply the way to go.  Also because these
functions iterates on global data structures that will need the NET_LOCK()
or a rewrite.

ok?

Index: netinet6/nd6.c
===
RCS file: /cvs/src/sys/netinet6/nd6.c,v
retrieving revision 1.199
diff -u -p -r1.199 nd6.c
--- netinet6/nd6.c  20 Dec 2016 18:33:43 -  1.199
+++ netinet6/nd6.c  21 Dec 2016 12:47:31 -
@@ -127,7 +127,7 @@ nd6_init(void)
nd6_init_done = 1;
 
/* start timer */
-   timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
+   timeout_set_proc(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
 
nd6_rs_init();
@@ -430,15 +430,16 @@ nd6_llinfo_timer(void *arg)
 void
 nd6_timer_work(void *null)
 {
-   int s;
struct nd_defrouter *dr, *ndr;
struct nd_prefix *pr, *npr;
struct in6_ifaddr *ia6, *nia6;
+   int s;
 
-   s = splsoftnet();
timeout_set(_timer_ch, nd6_timer, NULL);
timeout_add_sec(_timer_ch, nd6_prune);
 
+   NET_LOCK(s);
+
/* expire default router list */
TAILQ_FOREACH_SAFE(dr, _defrouter, dr_entry, ndr)
if (dr->expire && dr->expire < time_uptime)
@@ -482,7 +483,7 @@ nd6_timer_work(void *null)
prelist_remove(pr);
}
}
-   splx(s);
+   NET_UNLOCK(s);
 }
 
 void
@@ -1120,7 +1121,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
struct in6_nbrinfo *nbi = (struct in6_nbrinfo *)data;
struct rtentry *rt;
int error = 0;
-   int s;
 
switch (cmd) {
case SIOCGIFINFO_IN6:
@@ -1142,7 +1142,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
/* flush all the prefix advertised by routers */
struct nd_prefix *pr, *npr;
 
-   s = splsoftnet();
/* First purge the addresses referenced by a prefix. */
LIST_FOREACH_SAFE(pr, _prefix, ndpr_entry, npr) {
struct in6_ifaddr *ia6, *ia6_next;
@@ -1170,7 +1169,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
 
prelist_remove(pr);
}
-   splx(s);
break;
}
case SIOCSRTRFLUSH_IN6:
@@ -1178,12 +1176,10 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
/* flush all the default routers */
struct nd_defrouter *dr, *ndr;
 
-   s = splsoftnet();
defrouter_reset();
TAILQ_FOREACH_SAFE(dr, _defrouter, dr_entry, ndr)
defrtrlist_del(dr);
defrouter_select();
-   splx(s);
break;
}
case SIOCGNBRINFO_IN6:
@@ -1204,13 +1200,11 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
*idp = htons(ifp->if_index);
}
 
-   s = splsoftnet();
rt = nd6_lookup(_addr, 0, ifp, ifp->if_rdomain);
if (rt == NULL ||
(ln = (struct llinfo_nd6 *)rt->rt_llinfo) == NULL) {
error = EINVAL;
rtfree(rt);
-   splx(s);
break;
}
expire = ln->ln_rt->rt_expire;
@@ -1224,7 +1218,6 @@ nd6_ioctl(u_long cmd, caddr_t data, stru
nbi->isrouter = ln->ln_router;
nbi->expire = expire;
rtfree(rt);
-   splx(s);
 
break;
}
@@ -1478,12 +1471,14 @@ fail:
 void
 nd6_slowtimo(void *ignored_arg)
 {
-   int s = splsoftnet();
struct nd_ifinfo *nd6if;
struct ifnet *ifp;
+   int s;
 
timeout_set(_slowtimo_ch, nd6_slowtimo, NULL);
timeout_add_sec(_slowtimo_ch, ND6_SLOWTIMER_INTERVAL);
+
+   NET_LOCK(s);
TAILQ_FOREACH(ifp, , if_list) {
nd6if = ND_IFINFO(ifp);
if (nd6if->basereachable && /* already initialized */
@@ -1498,7 +1493,7 @@ nd6_slowtimo(void *ignored_arg)
nd6if->reachable = 
ND_COMPUTE_RTIME(nd6if->basereachable);
}
}
-   splx(s);
+   NET_UNLOCK(s);
 }
 
 int
Index: netinet6/nd6_nbr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_nbr.c,v
retrieving revision 1.112
diff -u -p -r1.112 nd6_nbr.c
--- netinet6/nd6_nbr.c  21 Dec 2016 12:30:19 -  1.112
+++ netinet6/nd6_nbr.c  21 Dec 2016 12:47:31 -
@@ -825,7 +825,6 @@ nd6_na_input(struct mbuf *m, int off, in
 */
struct nd_defrouter *dr;
struct in6_addr *in6;
-   int s;
 
in6 = (rt_key(rt))->sin6_addr;
 
@@ -835,7