Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]

[Masao Uebayashi <uebayasi%gmail.com@localhost>]

http://marc.info/?t=141670552700001&r=1&w=2

Following the ideas raised in that thread:

- Allocate callout_t dynamically.  struct ifnet only has a pointer to struct
  callout, which will not be read by netstat(1) anyway.

- Prefer the name "slowtimo" to "watchdog", because those callbacks do a
  little more than what "watchdog" suggests.

Index: sys/net/if.c
===================================================================
RCS file: /cvsroot/src/sys/net/if.c,v
retrieving revision 1.293
diff -p -u -r1.293 if.c
--- sys/net/if.c	17 Nov 2014 13:58:53 -0000	1.293
+++ sys/net/if.c	24 Nov 2014 01:49:14 -0000
@@ -168,8 +168,6 @@ static kmutex_t			if_clone_mtx;
 
 static struct ifaddr **		ifnet_addrs = NULL;
 
-static callout_t		if_slowtimo_ch;
-
 struct ifnet *lo0ifp;
 int	ifqmaxlen = IFQ_MAXLEN;
 
@@ -194,6 +192,7 @@ static void ifnet_lock_exit(struct ifnet
 static void if_detach_queues(struct ifnet *, struct ifqueue *);
 static void sysctl_sndq_setup(struct sysctllog **, const char *,
     struct ifaltq *);
+static void if_slowtimo(void *);
 
 #if defined(INET) || defined(INET6)
 static void sysctl_net_pktq_setup(struct sysctllog **, int);
@@ -235,9 +234,6 @@ ifinit(void)
 	sysctl_net_pktq_setup(NULL, PF_INET6);
 #endif
 
-	callout_init(&if_slowtimo_ch, 0);
-	if_slowtimo(NULL);
-
 	if_listener = kauth_listen_scope(KAUTH_SCOPE_NETWORK,
 	    if_listener_cb, NULL);
 
@@ -337,7 +333,7 @@ if_nullstop(struct ifnet *ifp, int disab
 }
 
 void
-if_nullwatchdog(struct ifnet *ifp)
+if_nullslowtimo(struct ifnet *ifp)
 {
 
 	/* Nothing. */
@@ -637,6 +633,13 @@ if_attach(ifnet_t *ifp)
 
 	/* Announce the interface. */
 	rt_ifannouncemsg(ifp, IFAN_ARRIVAL);
+
+	if (ifp->if_slowtimo != NULL) {
+		ifp->if_slowtimo_ch =
+		    kmem_zalloc(sizeof(*ifp->if_slowtimo_ch), KM_SLEEP);
+		callout_init(ifp->if_slowtimo_ch, 0);
+		if_slowtimo(ifp);
+	}
 }
 
 void
@@ -687,7 +690,7 @@ if_deactivate(struct ifnet *ifp)
 	ifp->if_ioctl	 = if_nullioctl;
 	ifp->if_init	 = if_nullinit;
 	ifp->if_stop	 = if_nullstop;
-	ifp->if_watchdog = if_nullwatchdog;
+	ifp->if_slowtimo = if_nullslowtimo;
 	ifp->if_drain	 = if_nulldrain;
 
 	/* No more packets may be enqueued. */
@@ -736,6 +739,12 @@ if_detach(struct ifnet *ifp)
 
 	s = splnet();
 
+	if (ifp->if_slowtimo != NULL) {
+		callout_halt(ifp->if_slowtimo_ch, NULL);
+		callout_destroy(ifp->if_slowtimo_ch);
+		kmem_free(ifp->if_slowtimo_ch, sizeof(*ifp->if_slowtimo_ch));
+	}
+
 	/*
 	 * Do an if_down() to give protocols a chance to do something.
 	 */
@@ -1493,24 +1502,23 @@ if_up(struct ifnet *ifp)
 }
 
 /*
- * Handle interface watchdog timer routines.  Called
- * from softclock, we decrement timers (if set) and
+ * Handle interface slow timeout routine.  Called
+ * from softclock, we decrement timer (if set) and
  * call the appropriate interface routine on expiration.
  */
-void
+static void
 if_slowtimo(void *arg)
 {
-	struct ifnet *ifp;
+	struct ifnet *ifp = arg;
 	int s = splnet();
 
-	IFNET_FOREACH(ifp) {
-		if (ifp->if_timer == 0 || --ifp->if_timer)
-			continue;
-		if (ifp->if_watchdog != NULL)
-			(*ifp->if_watchdog)(ifp);
-	}
+	KASSERT(ifp->if_slowtimo != NULL);
+
+	if (ifp->if_timer != 0 && --ifp->if_timer == 0)
+		(*ifp->if_slowtimo)(ifp);
+
 	splx(s);
-	callout_reset(&if_slowtimo_ch, hz / IFNET_SLOWHZ, if_slowtimo, NULL);
+	callout_reset(ifp->if_slowtimo_ch, hz / IFNET_SLOWHZ, if_slowtimo, ifp);
 }
 
 /*
Index: sys/net/if.h
===================================================================
RCS file: /cvsroot/src/sys/net/if.h,v
retrieving revision 1.175
diff -p -u -r1.175 if.h
--- sys/net/if.h	9 Sep 2014 20:16:12 -0000	1.175
+++ sys/net/if.h	24 Nov 2014 01:49:14 -0000
@@ -244,6 +244,7 @@ TAILQ_HEAD(ifnet_head, ifnet);		/* the a
 
 struct bridge_softc;
 struct bridge_iflist;
+struct callout;
 
 typedef struct ifnet {
 	void	*if_softc;		/* lower-level data for this if */
@@ -253,7 +254,7 @@ typedef struct ifnet {
 	int	if_pcount;		/* number of promiscuous listeners */
 	struct bpf_if *if_bpf;		/* packet filter structure */
 	u_short	if_index;		/* numeric abbreviation for this if */
-	short	if_timer;		/* time 'til if_watchdog called */
+	short	if_timer;		/* time 'til if_slowtimo called */
 	short	if_flags;		/* up/down, broadcast, etc. */
 	short	if__pad1;		/* be nice to m68k ports */
 	struct	if_data if_data;	/* statistics and other data about if */
@@ -274,8 +275,9 @@ typedef struct ifnet {
 		    (struct ifnet *);
 	void	(*if_stop)		/* stop routine */
 		    (struct ifnet *, int);
-	void	(*if_watchdog)		/* timer routine */
+	void	(*if_slowtimo)		/* timer routine */
 		    (struct ifnet *);
+#define	if_watchdog	if_slowtimo
 	void	(*if_drain)		/* routine to release resources */
 		    (struct ifnet *);
 	struct ifaltq if_snd;		/* output queue (includes altq) */
@@ -341,6 +343,7 @@ typedef struct ifnet {
 	    const struct sockaddr *);
 	int (*if_setflags)(struct ifnet *, const short);
 	struct ifnet_lock *if_ioctl_lock;
+	struct callout *if_slowtimo_ch;
 } ifnet_t;
  
 #define	if_mtu		if_data.ifi_mtu
@@ -867,7 +870,6 @@ void	if_purgeaddrs(struct ifnet *, int, 
 void	if_detach(struct ifnet *);
 void	if_down(struct ifnet *);
 void	if_link_state_change(struct ifnet *, int);
-void	if_slowtimo(void *);
 void	if_up(struct ifnet *);
 int	ifconf(u_long, void *);
 void	ifinit(void);
@@ -926,7 +928,8 @@ void	if_nullstart(struct ifnet *);
 int	if_nullioctl(struct ifnet *, u_long, void *);
 int	if_nullinit(struct ifnet *);
 void	if_nullstop(struct ifnet *, int);
-void	if_nullwatchdog(struct ifnet *);
+void	if_nullslowtimo(struct ifnet *);
+#define	if_nullwatchdog	if_nullslowtimo
 void	if_nulldrain(struct ifnet *);
 #else
 struct if_nameindex {