FreeBSD-5.2 BETA & Davicom ethernet

[Maxime Henrion]

Marius Strobl wrote:
> On Tue, Dec 02, 2003 at 04:21:16PM +0100, Maxime Henrion wrote:
> > Aditya wrote:
> > > > On Tue, 2 Dec 2003 15:41:42 +0300, Vyacheslav Silakov <seal at inar.ru> said:
> > > > I've read in October that problem with Davicom ethernet card (Sun
> > > > Netra X1) is fixed under FreeBSD-5-CURRENT. Now I've booted fresh
> > > > FreeBSD-5.2-BETA and found old bugs:
> > > >
> > > > [..]  FreeBSD 5.2-BETA #0: Wed Nov 26 23:53:05 GMT 2003
> > > > root at luthien.local:/usr/obj/usr/src/sys/GENERIC [..]  dc0: <Davicom
> > > > DM9102A 10/100BaseTX> port 0x10000-0x100ff at device 12.0 on pci0
> > > > dc0: Ethernet address: 00:00:00:00:00:00
> > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [..]  dc1: <Davicom DM9102A
> > > > 10/100BaseTX> port 0x10100-0x101ff mem 0x2000-0x20ff at device 5.0
> > > > on pci0 dc1: Ethernet address: 00:00:00:00:00:00
> > > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > 
> > > I can confirm this on a Netra X1 that I just installed 5.2-BETA
> > > freshly using a FTP Passive install:
> > > 
> > > FreeBSD some.lan.grot.org 5.2-BETA FreeBSD 5.2-BETA #0: Wed Nov 26
> > > 23:53:05 GMT 2003 root at luthien.local:/usr/obj/usr/src/sys/GENERIC
> > > sparc64
> > > 
> > > dc0: <Davicom DM9102A 10/100BaseTX> port 0x10000-0x100ff at device 12.0 on pci0
> > > dc0: Ethernet address: 00:00:00:00:00:00
> > > 
> > > dc1: <Davicom DM9102A 10/100BaseTX> port 0x10100-0x101ff mem 0x2000-0x20ff at device 5.0 on pci0
> > > dc1: Ethernet address: 00:00:00:00:00:00
> > 
> > This is expected for now.  I coudln't find the time to add support for
> > getting the MAC address from the OpenFirmware in dc(4) yet.  With a bit
> > of luck, this will be working soon, though probably not before
> > 5.3-RELEASE, unless someone else tackles this issue.
> > 
> 
> A while ago I already sent you a patch similar to the attached one which
> gets the MAC address for Sun onboard NICs via OpenFirmware in dc(4). But
> you didn't like the MD ifdefs in if_dc.c and wanted to implement it some
> layers above.

Yes, I remember it.  But as I said, it's an ugly hack, and needs to be
done better.  If people really need a MAC address they could use an
"ifconfig dc0 ether xx:xx:xx:xx:xx:xx" command.  Besides, there have
been reports of crashes with dc(4) cards under sparc64 at high load.  So
I'm really reluctant to commit this.  Which makes me wonder : do you
experience these crashes?

> The patch has the problem that because of the check for OFW_NEWPCI dc(4)
> no longer can be built as a module on FreeBSD/sparc64. I tried to move
> the check for local-mac-address into OF_getetheraddr() so it would also
> work for hme(4) and gem(4) while dc(4) can be built as module but failed
> to do a version that works for both, sbus and pci, devices.

See below for another problem with this patch.

> Index: if_dc.c
> ===================================================================
> RCS file: /usr/data/bsd/cvs/fbsd/src/sys/pci/if_dc.c,v
> retrieving revision 1.135
> diff -u -r1.135 if_dc.c
> --- if_dc.c	14 Nov 2003 19:00:31 -0000	1.135
> +++ if_dc.c	19 Nov 2003 22:50:26 -0000
> @@ -124,6 +124,13 @@
>  #include <dev/pci/pcireg.h>
>  #include <dev/pci/pcivar.h>
>  
> +#ifdef __sparc64__
> +#include "opt_ofw_pci.h"
> +#include <dev/ofw/openfirm.h>
> +#include <machine/ofw_machdep.h>
> +#include <sparc64/pci/ofw_pci.h>
> +#endif
> +
>  #define DC_USEIOSPACE
>  #ifdef __alpha__
>  #define SRM_MEDIA
> @@ -2102,7 +2109,6 @@
>  	case DC_TYPE_PNIC:
>  		dc_read_eeprom(sc, (caddr_t)&eaddr, 0, 3, 1);
>  		break;
> -	case DC_TYPE_DM9102:
>  	case DC_TYPE_21143:
>  	case DC_TYPE_ASIX:
>  		dc_read_eeprom(sc, (caddr_t)&eaddr, DC_EE_NODEADDR, 3, 0);
> @@ -2115,6 +2121,25 @@
>  	case DC_TYPE_CONEXANT:
>  		bcopy(sc->dc_srom + DC_CONEXANT_EE_NODEADDR, &eaddr,
>  		    ETHER_ADDR_LEN);
> +		break;
> +	case DC_TYPE_DM9102:
> +		dc_read_eeprom(sc, (caddr_t)&eaddr, DC_EE_NODEADDR, 3, 0);
> +#ifdef __sparc64__
> +		/*
> +		 * If this is an onboard dc(4) the station address read from
> +		 * the EEPROM is all zero and we have to get it from the fcode.
> +		 */
> +		for (i = 0; i < ETHER_ADDR_LEN; i++)
> +			if (eaddr[i] != 0x00)
> +				break;

AFAICT, the "break" here breaks out of the for loop, not out of the case
statement, which is what you probably wanted.  So, whether or not the
MAC address is all zeros, you'll try to get the MAC address from
OpenFirmware.

You'd need a Davicom card which is not onboard in a sparc64 box to
trigger the bug, so it's harmless in most cases, which is probably why
you didn't notice it.

You should probably add an "if (i == ETHER_ADDR_LEN) {" after the for
loop and enclose all the below code in it.

> +#ifdef OFW_NEWPCI
> +		if (OF_getprop(ofw_pci_get_node(dev), "local-mac-address",
> +#else
> +		if (OF_getprop(ofw_pci_node(dev), "local-mac-address",
> +#endif
> +		    eaddr, sizeof(eaddr)) == -1)
> +			OF_getetheraddr(dev, eaddr);
> +#endif
>  		break;
>  	case DC_TYPE_XIRCOM:
>  		/* The MAC comes from the CIS. */

Cheers,
Maxime