Re: HEADSUP: internal network interface changes

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():

_at__at_ -282,9 +283,13 _at__at_
 		}
 	}
 
-	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;
_at__at_ -323,6 +328,7 _at__at_

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
> _at__at_ -46,9 +46,17 _at__at_
>  .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"
> _at__at_ -219,6 +227,11 _at__at_
>  .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
> _at__at_ -270,6 +283,8 _at__at_
>  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
> _at__at_ -988,6 +1003,14 _at__at_
>  .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
> _at__at_ -999,6 +1022,29 _at__at_
>  (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