From owner-freebsd-net@FreeBSD.ORG Mon Oct 25 13:05:53 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4A89A16A4CE; Mon, 25 Oct 2004 13:05:53 +0000 (GMT) Received: from outbound0.sv.meer.net (outbound0.sv.meer.net [205.217.152.13]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2457C43D49; Mon, 25 Oct 2004 13:05:53 +0000 (GMT) (envelope-from gnn@neville-neil.com) Received: from mail.meer.net (mail.meer.net [209.157.152.14]) i9PD4aa2035437; Mon, 25 Oct 2004 06:04:37 -0700 (PDT) (envelope-from gnn@neville-neil.com) Received: from minion.local.neville-neil.com (pc1.oakwoodazabu1-unet.ocn.ne.jp [220.110.140.201]) by mail.meer.net (8.12.10/8.12.2/meer) with ESMTP id i9PD4X0E006877; Mon, 25 Oct 2004 06:04:34 -0700 (PDT) (envelope-from gnn@neville-neil.com) Date: Mon, 25 Oct 2004 22:04:32 +0900 Message-ID: From: gnn@freebsd.org To: rwatson@freebsd.org User-Agent: Wanderlust/2.10.1 (Watching The Wheels) SEMI/1.14.5 (Awara-Onsen) FLIM/1.14.5 (Demachiyanagi) APEL/10.5 Emacs/21.2 (powerpc-apple-darwin) MULE/5.0 (SAKAKI) MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII cc: freebsd-net@freebsd.org cc: kame Subject: Updated scope6.c locking patch... X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Oct 2004 13:05:53 -0000 Hi Folks, Enclosed please find my updated patch for scope6 locking for the IPv6 code in FreeBSD-CURRENT. The major change was to remove the IFNET list locking because this actually needs to be done outside of the IPv6 code and to add address family (AF) data locking because that's something we can and should do within the scope6 code. I have tested this locally in the following way: 1) Open a shell on the machine (shell1) and do: shell1> ifconfig lo1 create shell1> while 1 shell1> scope6config -l 1 -s 2 -o 3 lo1 shell1> end 2) Open a second shell on the same machine (shell2) and do: shell2> ifconfig lo1 destroy shell2> ifconfig lo1 create etc. The messages in the shell1 window should switch between: lo1: 0, 6, 1, 0, 0, 2, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0 and scope6config: ioctl(SIOCSSCOPE6): Device not configured and the kernel should not panic ;-) Later, George --- sys/netinet6/scope6.c.orig +++ sys/netinet6/scope6.c @@ -71,7 +71,6 @@ scope6_ifattach(ifp) struct ifnet *ifp; { - int s = splnet(); struct scope6_id *sid; sid = (struct scope6_id *)malloc(sizeof(*sid), M_IFADDR, M_WAITOK); @@ -89,7 +88,6 @@ sid->s6id_list[IPV6_ADDR_SCOPE_ORGLOCAL] = 1; #endif - splx(s); return sid; } @@ -106,12 +104,17 @@ struct ifnet *ifp; struct scope6_id *idlist; { - int i, s; + int i; int error = 0; - struct scope6_id *sid = SID(ifp); + struct scope6_id *sid = NULL; + + IF_AFDATA_LOCK(ifp); + sid = SID(ifp); - if (!sid) /* paranoid? */ + if (!sid) { /* paranoid? */ + IF_AFDATA_UNLOCK(ifp); return (EINVAL); + } /* * XXX: We need more consistency checks of the relationship among @@ -123,8 +126,6 @@ * interface addresses, routing table entries, PCB entries... */ - s = splnet(); - SCOPE6_LOCK(); for (i = 0; i < 16; i++) { if (idlist->s6id_list[i] && @@ -135,7 +136,8 @@ */ if (i == IPV6_ADDR_SCOPE_INTFACELOCAL && idlist->s6id_list[i] != ifp->if_index) { - splx(s); + IF_AFDATA_UNLOCK(ifp); + SCOPE6_UNLOCK(); return (EINVAL); } @@ -147,7 +149,8 @@ * IDs, but we check the consistency for * safety in later use. */ - splx(s); + IF_AFDATA_UNLOCK(ifp); + SCOPE6_UNLOCK(); return (EINVAL); } @@ -160,7 +163,7 @@ } } SCOPE6_UNLOCK(); - splx(s); + IF_AFDATA_UNLOCK(ifp); return (error); } @@ -170,15 +173,20 @@ struct ifnet *ifp; struct scope6_id *idlist; { + /* We only need to lock the interface's afdata for SID() to work. */ + IF_AFDATA_LOCK(ifp); struct scope6_id *sid = SID(ifp); - if (sid == NULL) /* paranoid? */ + if (sid == NULL) { /* paranoid? */ + IF_AFDATA_UNLOCK(ifp); return (EINVAL); + } SCOPE6_LOCK(); *idlist = *sid; SCOPE6_UNLOCK(); + IF_AFDATA_UNLOCK(ifp); return (0); } @@ -259,7 +267,11 @@ { int scope; u_int32_t zoneid = 0; - struct scope6_id *sid = SID(ifp); + struct scope6_id *sid = NULL; + + IF_AFDATA_LOCK(ifp); + + sid = SID(ifp); #ifdef DIAGNOSTIC if (sid == NULL) { /* should not happen */ @@ -277,10 +289,12 @@ * interface. */ if (IN6_IS_ADDR_LOOPBACK(addr)) { - if (!(ifp->if_flags & IFF_LOOPBACK)) + if (!(ifp->if_flags & IFF_LOOPBACK)) { + IF_AFDATA_UNLOCK(ifp); return (-1); - else { + } else { *ret_id = 0; /* there's no ambiguity */ + IF_AFDATA_UNLOCK(ifp); return (0); } } @@ -315,6 +329,9 @@ SCOPE6_UNLOCK(); *ret_id = zoneid; + + IF_AFDATA_UNLOCK(ifp); + return (0); } @@ -323,7 +340,7 @@ struct ifnet *ifp; /* note that this might be NULL */ { /* - * Currently, this function just set the default "interfaces" + * Currently, this function just sets the default "interfaces" * and "links" according to the given interface. * We might eventually have to separate the notion of "link" from * "interface" and provide a user interface to set the default.