From owner-freebsd-net@FreeBSD.ORG Tue Nov 24 23:07:21 2009 Return-Path: Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 66A591065670; Tue, 24 Nov 2009 23:07:21 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 400018FC12; Tue, 24 Nov 2009 23:07:21 +0000 (UTC) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTPS id F2BEA46B2A; Tue, 24 Nov 2009 18:07:20 -0500 (EST) Date: Tue, 24 Nov 2009 23:07:20 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: John Baldwin In-Reply-To: <200911241356.05528.jhb@freebsd.org> Message-ID: References: <200911241356.05528.jhb@freebsd.org> User-Agent: Alpine 2.00 (BSF 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: net@freebsd.org Subject: Re: [PATCH] Remove if_watchdog and if_timer X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Nov 2009 23:07:21 -0000 On Tue, 24 Nov 2009, John Baldwin wrote: > Now that no drivers in the tree use if_watchdog and if_timer, this patch > just removes them completely. Since if_timer was a short that was adjacent > to if_index, removing if_timer would have still left a padding hole in the > form of a short on all of our current architectures. After discussing this > briefly with Brooks I changed if_index to be an int rather than leaving the > padding hole. Comments? The if_watchdog/if_timer changes sound good to me, but let's do the if_index change separately, and just add padding in this change. The if_index -> 32-bit chnge is one that's been overdue for a long time, but it's going to have a lot of side effects, so keeping it to its own changesets (which can be backed out if needed more easily) is probably a good idea. Thanks for taking on the watchdog changes! Robert N M Watson Computer Laboratory University of Cambridge > > --- //depot/vendor/freebsd/src/sys/net/if.c 2009/11/12 19:05:14 > +++ //depot/user/jhb/cleanup/sys/net/if.c 2009/11/19 22:35:58 > @@ -125,10 +125,8 @@ > static void if_freemulti(struct ifmultiaddr *); > static void if_init(void *); > static void if_grow(void); > -static void if_check(void *); > static void if_route(struct ifnet *, int flag, int fam); > static int if_setflag(struct ifnet *, int, int, int *, int); > -static void if_slowtimo(void *); > static int if_transmit(struct ifnet *ifp, struct mbuf *m); > static void if_unroute(struct ifnet *, int flag, int fam); > static void link_rtrequest(int, struct rtentry *, struct rt_addrinfo *); > @@ -185,11 +183,6 @@ > static if_com_alloc_t *if_com_alloc[256]; > static if_com_free_t *if_com_free[256]; > > -/* > - * System initialization > - */ > -SYSINIT(interface_check, SI_SUB_PROTO_IF, SI_ORDER_FIRST, if_check, NULL); > - > MALLOC_DEFINE(M_IFNET, "ifnet", "interface internals"); > MALLOC_DEFINE(M_IFADDR, "ifaddr", "interface address"); > MALLOC_DEFINE(M_IFMADDR, "ether_multi", "link-level multicast address"); > @@ -375,18 +368,6 @@ > V_ifindex_table = e; > } > > -static void > -if_check(void *dummy __unused) > -{ > - > - /* > - * If at least one interface added during boot uses > - * if_watchdog then start the timer. > - */ > - if (slowtimo_started) > - if_slowtimo(0); > -} > - > /* > * Allocate a struct ifnet and an index for an interface. A layer 2 > * common structure will also be allocated if an allocation routine is > @@ -670,18 +651,6 @@ > > /* Announce the interface. */ > rt_ifannouncemsg(ifp, IFAN_ARRIVAL); > - > - if (!vmove && ifp->if_watchdog != NULL) { > - if_printf(ifp, > - "WARNING: using obsoleted if_watchdog interface\n"); > - > - /* > - * Note that we need if_slowtimo(). If this happens after > - * boot, then call if_slowtimo() directly. > - */ > - if (atomic_cmpset_int(&slowtimo_started, 0, 1) && !cold) > - if_slowtimo(0); > - } > } > > static void > @@ -1973,39 +1942,6 @@ > } > > /* > - * Handle interface watchdog timer routines. Called > - * from softclock, we decrement timers (if set) and > - * call the appropriate interface routine on expiration. > - * > - * XXXRW: Note that because timeouts run with Giant, if_watchdog() is called > - * holding Giant. > - */ > -static void > -if_slowtimo(void *arg) > -{ > - VNET_ITERATOR_DECL(vnet_iter); > - struct ifnet *ifp; > - int s = splimp(); > - > - VNET_LIST_RLOCK_NOSLEEP(); > - IFNET_RLOCK_NOSLEEP(); > - VNET_FOREACH(vnet_iter) { > - CURVNET_SET(vnet_iter); > - TAILQ_FOREACH(ifp, &V_ifnet, if_link) { > - if (ifp->if_timer == 0 || --ifp->if_timer) > - continue; > - if (ifp->if_watchdog) > - (*ifp->if_watchdog)(ifp); > - } > - CURVNET_RESTORE(); > - } > - IFNET_RUNLOCK_NOSLEEP(); > - VNET_LIST_RUNLOCK_NOSLEEP(); > - splx(s); > - timeout(if_slowtimo, (void *)0, hz / IFNET_SLOWHZ); > -} > - > -/* > * Map interface name to interface structure pointer, with or without > * returning a reference. > */ > --- //depot/vendor/freebsd/src/sys/net/if_dead.c 2009/04/23 11:55:13 > +++ //depot/user/jhb/cleanup/sys/net/if_dead.c 2009/11/19 22:35:58 > @@ -70,12 +70,6 @@ > return (ENXIO); > } > > -static void > -ifdead_watchdog(struct ifnet *ifp) > -{ > - > -} > - > static int > ifdead_resolvemulti(struct ifnet *ifp, struct sockaddr **llsa, > struct sockaddr *sa) > @@ -107,7 +101,6 @@ > ifp->if_input = ifdead_input; > ifp->if_start = ifdead_start; > ifp->if_ioctl = ifdead_ioctl; > - ifp->if_watchdog = ifdead_watchdog; > ifp->if_resolvemulti = ifdead_resolvemulti; > ifp->if_qflush = ifdead_qflush; > ifp->if_transmit = ifdead_transmit; > --- //depot/vendor/freebsd/src/sys/net/if_var.h 2009/11/12 19:05:14 > +++ //depot/user/jhb/cleanup/sys/net/if_var.h 2009/11/19 22:35:58 > @@ -140,8 +140,7 @@ > int if_pcount; /* number of promiscuous listeners */ > struct carp_if *if_carp; /* carp interface structure */ > 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 */ > + u_int if_index; /* numeric abbreviation for this if */ > struct ifvlantrunk *if_vlantrunk; /* pointer to 802.1q data */ > int if_flags; /* up/down, broadcast, etc. */ > int if_capabilities; /* interface features & capabilities */ > @@ -161,8 +160,6 @@ > (struct ifnet *); > int (*if_ioctl) /* ioctl routine */ > (struct ifnet *, u_long, caddr_t); > - void (*if_watchdog) /* timer routine */ > - (struct ifnet *); > void (*if_init) /* Init routine */ > (void *); > int (*if_resolvemulti) /* validate/resolve multicast */ > > -- > John Baldwin > _______________________________________________ > freebsd-net@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-net > To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >