On Wed, Aug 25, 1999 at 09:19:01PM +0200, Ignatios Souvatzis wrote:
> On Wed, Aug 25, 1999 at 11:01:04AM +0200, Andreas Johansson wrote:
> > Hello!
> > 
> > I have discovered a potential problem in NetBSD 1.3 & 1.4. The problem is
> > that if_attach() in net/if.c uses the mac address size ifp->if_addrlen to
> > setup the size of the interface's sockaddr_dl structure like this:
> > 
> >         namelen = strlen(ifp->if_xname);
> >         masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + namelen;
> >         socksize = masklen + ifp->if_addrlen;
> > #define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1)))
> >         if (socksize < sizeof(*sdl))
> >                 socksize = sizeof(*sdl);
> >         socksize = ROUNDUP(socksize);
> > 	[...]
> >         sdl->sdl_len = socksize;
> > 
> > But unfortunately, ifp->if_addrlen is setup in ether_ifattach() (or the
> > corresponding function for other interface types). This function must be 
> > called after if_attach(), and therefore if_addrlen is uninitialized by the
> > time if_attach() uses it.
> 
> Looks like I'm the culprit, from back then when I reworked the ARP stuff.
> 
> Hm.
> 
> we need the sizes before if_attach(), and to write back the link level
> address after it.
> 
> I see three possibilities... 
> 
> - call ether_ifattach before AND after (won't do any harm)
> - split it into two, and call the before part before, and something
>   like ether_writelladdr() after
> - _only_ call ether_ifattach(), and make ether_ifattach() call if_attach()
>   at an appropriate moment.

I strongly suspect we need a seperate function for xxx_storelladdr().
At least for common ARCnet hardware, the ll addr used by the chip is read
from DIP switches each time we "ifconfig ... up" (when the if was down 
before).

Currently the if_bah driver stores this address into the right place by calling
arc_ifattach() again.

This might not be necessary _currently_ for Ethernet hardware, but given that
some people are working on DECnet, having a convenience function to store
the current link level address would be nice.

(If people object, I'll need to add one for ARCnet; and I don't see why 
 it can't be made general).

if_storelladdr(ifp, lla)
	struct ifnet *ifp;
	caddr_t lla;
{
	/* check whether the address length has changed. */
	/* if it has grown enough to make this necessary,
	   allocate a new address structure,
	   copy the string interface name etc, and free the old */
	/* store ifp->if_addrlen bytes off lla into it */
	/* INSERTHERE: add additional code to deal with changed link level
	   addresses, if necessary in the future */
}

Regards,
	Ignatios

-- 
 * Progress (n.): The process through which Usenet has evolved from
   smart people in front of dumb terminals to dumb people in front of
   smart terminals.  -- obs@burnout.demon.co.uk (obscurity)