From owner-freebsd-current@FreeBSD.ORG Thu Jun 9 08:25:33 2005 Return-Path: X-Original-To: current@FreeBSD.ORG Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4FE3B16A41C; Thu, 9 Jun 2005 08:25:33 +0000 (GMT) (envelope-from sobomax@web.portaone.com) Received: from www.portaone.com (web.portaone.com [195.70.151.35]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6353B43D1F; Thu, 9 Jun 2005 08:25:32 +0000 (GMT) (envelope-from sobomax@web.portaone.com) Received: from www.portaone.com (localhost [127.0.0.1]) by www.portaone.com (8.12.11/8.12.11) with ESMTP id j598PVnV053729 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 9 Jun 2005 10:25:31 +0200 (CEST) (envelope-from sobomax@web.portaone.com) Received: (from sobomax@localhost) by www.portaone.com (8.12.11/8.12.11/Submit) id j598PUdT053728; Thu, 9 Jun 2005 10:25:30 +0200 (CEST) (envelope-from sobomax) Date: Thu, 9 Jun 2005 10:25:30 +0200 From: Maxim Sobolev To: Brooks Davis Message-ID: <20050609082530.GA44274@www.portaone.com> References: <20050609064452.GC1595@odin.ac.hmc.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050609064452.GC1595@odin.ac.hmc.edu> User-Agent: Mutt/1.5.8i X-Virus-Scanned: ClamAV version 0.83, clamav-milter version 0.83 on www.portaone.com X-Virus-Status: Clean Cc: current@FreeBSD.ORG, net@FreeBSD.ORG Subject: Re: HEADSUP: internal network interface changes X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2005 08:25:33 -0000 Hi, I've noticed that in some cases you have removed bcopy() into arpcom.ac_enaddr completely, while in some others have modified it to use IFP2AC(). I wonder if it's a mistake or if there is some logic behind that. Also, it looks like in cdce(4) driver you are referencing if_softc before it's been assigned by if_alloc(): @@ -282,9 +283,13 @@ } } - bcopy(eaddr, (char *)&sc->arpcom.ac_enaddr, ETHER_ADDR_LEN); + bcopy(eaddr, (char *)&GET_ARPCOM(sc)->ac_enaddr, ETHER_ADDR_LEN); - ifp = GET_IFP(sc); + ifp = GET_IFP(sc) = if_alloc(IFT_ETHER); + if (ifp == NULL) { + printf("%s: can not if_alloc()\n", USBDEVNAME(sc->cdce_dev)); + USB_ATTACH_ERROR_RETURN; + } ifp->if_softc = sc; if_initname(ifp, "cdce", sc->cdce_unit); ifp->if_mtu = ETHERMTU; @@ -323,6 +328,7 @@ GET_ARPCOM(sc) basically dereferences sc->cdce_ifp, which isn't initialized before if_alloc() on the next line. -Maxim On Wed, Jun 08, 2005 at 11:44:52PM -0700, Brooks Davis wrote: > I plan to commit a major rework of network interface related storage > Friday morning PDT. This is a massive change touching every network > driver in the system. This change was discussed at the BSDCan dev > summit and derives from discussions at EuroBSDCon on dealing with > dynamic network devices. You can view the diff at: > > http://people.freebsd.org/~brooks/patches/ifnet.diff > > I'm not posting the diff to the list as it is nearly 700K. Below, you > will find the diff to ifnet(9) for a more technical view of the API > change. > > In short, the change removes the embedded struct ifnet and layer 2 common > structures (struct arpcom, struct ifatm, struct sppp, etc) from driver > softcs and replaces them with a struct ifnet pointer which is allocated > with a new function, if_alloc which takes an interface type. For > certain types, if_alloc also fills in the new struct ifnet member, > if_l2com with an initialized layer 2 common structure. > > The benefits of this change are: > - The size of struct ifnet and the layer 2 common structures is no > longer part of the network interface ABI. This means we add > features to the generic interface code so long as they do not require > action on the part of the driver without breaking the ABI. > - Since storage is no longer tied to the softc, we will able to > reference count struct ifnet more easily which is a prerequisite for > fixing the panics on removing an interface which is configured with > dummynet. > - This patch eliminates many ugly casts and the historically weakly > documented requirement that softc's be castable to ifnet's and > arpcom's. > > Things to note about this change: > - External drivers including those in ports will panic if loaded until > they are converted to the new API. > > Things to note about this patch: > - There are nearly 100 drivers in the tree and I only use a small set > of them so there are likely to be some small bugs in this patch. The > changes were mostly mechanical, but varying naming conventions, plus > the occasional driver written entirely from scratch introduce the > possibility of errors. Use care when updating, particularly with > remote systems. > - In most cases, this patch does not address the issue of keeping > source compatible with previous releases or other systems. I will > supply patches to do so in any case where there is a need, but I > intend to wait until after committing to do so. I hope the set of > drivers requiring these changes will be small. > > -- Brooks > > P.S. the posted patch contains a bug in the udav driver. It will be > fixed before commit. > > --- freebsd/share/man/man9/ifnet.9 Sun Jun 5 13:33:05 2005 > +++ ifnet/share/man/man9/ifnet.9 Sun Jun 5 20:20:03 2005 > @@ -46,9 +46,17 @@ > .In net/if_types.h > .\" > .Ss "Interface Manipulation Functions" > +.Ft "struct ifnet *" > +.Fn if_alloc "u_char type" > .Ft void > .Fn if_attach "struct ifnet *ifp" > .Ft void > +.Fn if_detach "struct ifnet *ifp" > +.Ft void > +.Fn if_free "struct ifnet *ifp" > +.Ft void > +.Fn if_free_type "struct ifnet *ifp" "u_char type" > +.Ft void > .Fn if_down "struct ifnet *ifp" > .Ft int > .Fn ifioctl "struct socket *so" "u_long cmd" "caddr_t data" "struct thread *td" > @@ -219,6 +227,11 @@ > .Pq Vt "void *" > A pointer to the driver's private state block. > (Initialized by driver.) > +.It Va if_l2com > +.Pq Vt "void *" > +A pointer to the common data for the interface's layer 2 protocol. > +(Initialized by > +.Fn if_alloc .) > .It Va if_link > .Pq Fn TAILQ_ENTRY ifnet > .Xr queue 3 > @@ -270,6 +283,8 @@ > to refer to a particular interface by index > (see > .Xr link_addr 3 ) . > +(Initialized by > +.Fn if_alloc .) > .It Va if_timer > .Pq Vt short > Number of seconds until the watchdog timer > @@ -988,6 +1003,14 @@ > .El > .Ss Interface Manipulation Functions > .Bl -ohang -offset indent > +.It Fn if_alloc > +Allocate and initalize an > +.Fa ifp . > +Initalization includes the allocation of an interface index and may > +include the allocation of a > +.Fa type > +specific structure in > +.Va if_l2com . > .It Fn if_attach > Link the specified interface > .Fa ifp > @@ -999,6 +1022,29 @@ > (A pointer to > this address structure is saved in the global array > .Va ifnet_addrs . ) > +The > +.Fa ifp > +must have been allocted by > +.Fn if_alloc . > +.It Fn if_detach > +Shutdown and unlink the specified > +.Fa ifp > +from the interface list. > +.It Fn if_free > +Free the given > +.Fa ifp > +back to the system. > +The interface must have been previously detached if it was ever attached. > +.It Fn if_free_type > +Identical to > +.Fn if_free > +except that the given > +.Fa type > + is used to free > + .Va if_l2com > + instead of the type in > + .Va if_type . > + This is intended for use with drivers that change their interface type. > .It Fn if_down > Mark the interface > .Fa ifp > > -- > Any statement of the form "X is the one, true Y" is FALSE. > PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4