Date: Wed, 17 Jul 2002 14:25:29 -0400 From: "Jim McGrath" <jimmcgra@bellatlantic.net> To: "Kelly Yancey" <kbyanc@posi.net>, "Bill Baumann" <bbaumann@isilon.com> Cc: "Jim McGrath" <jimmcgra@bellatlantic.net>, <freebsd-net@FreeBSD.ORG> Subject: RE: Inconsistency between net/if.c and several ethernet drivers Message-ID: <NDBBKKEELKBCJJBEGDECGEJECEAA.jimmcgra@bellatlantic.net> In-Reply-To: <Pine.BSF.4.21.0207170942280.35007-100000@isilon.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Any driver that uses miibus_attach() is broken if struct arpcom is not at
the beginning of the softc structure. There was some discussion of this
more than a year ago when this bug showed up in the wx driver.
Jim
> -----Original Message-----
> From: owner-freebsd-net@FreeBSD.ORG
> [mailto:owner-freebsd-net@FreeBSD.ORG]On Behalf Of Bill Baumann
> Sent: Wednesday, July 17, 2002 1:58 PM
> To: Kelly Yancey
> Cc: freebsd-net@FreeBSD.ORG
> Subject: Re: Inconsistency between net/if.c and several ethernet drivers
>
>
>
> Pointers to interface and arpcom are clearly equivalent as arpcom contains
> the interface structure. The one definition for the combined structure
> makes it very safe. However, there are many definitions of the softc
> structure. Requiring arpcom to be at the beginning of all softc structs
> requires every device driver writer to either track this obscure detail,
> or risk obscure bugs.
>
>
> Why bother with a if_softc field when the interface and softc pointer are
> supposed to be the same? Also, the very old Lance driver (lnc) has this
> problem. It makes me wonder how true we are to TCP/IP Illustrated...
>
> So while this wouldn't really be a bug fix, perhaps we should do my
> suggested change anyway to avoid modifying the drivers.
>
> Regards,
> Bill Baumann
>
>
> On Tue, 16 Jul 2002, Kelly Yancey wrote:
>
> > On Tue, 16 Jul 2002, Bill Baumann wrote:
> >
> > >
> > > In net/if.c in a couple of places, the ethernet address is
> needed. This
> > > is stored in the arpcom structure. A couple lines of code in
> if.c require
> > > struct arpcom be at the very begining of device softc
> structures. Nearly
> > > all drivers observe this. However, several do not. Sadly,
> this includes
> > > the one I'm working on.
> > >
> > > net/if.c routines if_findindex() and if_setlladdr() gain access to the
> > > ethernet address via the following expression:
> > >
> > > ((struct arpcom *)ifp->if_softc)->ac_enaddr
> > >
> > > The above code assumes that the if_softc pointer is equivalent to an
> > > struct arpcom pointer. The awi, ray, lnc and pdq drivers have other
> > > fields at the beginning of their softc structures. Attempts
> to set the
> > > ethernet address of these devices may cause corruption.
> > >
> > >
> > > Shouldn't access of arpcom be via ifp instead?
> > >
> > > ((struct arpcom *)ifp)->ac_enaddr
> > >
> > >
> > > For example, if_ethersubr.c uses the following macro:
> > > #define IFP2AC(IFP) ((struct arpcom *)IFP)
> > >
> > > It looked to me like the other code in net, like
> if_ethersubr.c use ifp
> > > rather than if_softc to find struct arpcom.
> > >
> > > Bug?
> > >
> >
> > Design. :) See page 77 of Stevens' TCP/IP Illustrated Volume
> 2. By putting
> > the structures at the beginning of the softc, the networking
> code can access
> > them without any explicit knowledge of the driver's softc
> itself (i.e. it can
> > use the softc as an opaque encapsulated version of either the
> arpcom or ifnet
> > structures). The bug, then, would seem to be in the network
> drivers that
> > don't follow this convention. But I'm not familiar with those
> particular
> > drivers, so I cannot comment on them; perhaps they employ some
> cleverness to
> > circumvent the requirement (by why?). Anyway, it should be obvious that
> > accessing the arpcom structure via casting from the ifnet
> structure or the
> > softc structure are supposed to have the same results, so the
> code your quoted
> > above is fine.
> >
> > Kelly
> >
> > --
> > Kelly Yancey -- kbyanc@{posi.net,FreeBSD.org}
> > "The worst sin towards our fellow creatures is not to hate
> them, but to be
> > indifferent to them; that's the essence of inhumanity."
> > -- George Bernard Shaw
> >
> >
> >
> > To Unsubscribe: send mail to majordomo@FreeBSD.org
> > with "unsubscribe freebsd-net" in the body of the message
> >
>
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-net" in the body of the message
>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?NDBBKKEELKBCJJBEGDECGEJECEAA.jimmcgra>