Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 16 Dec 2003 00:10:22 +0100
From:      Wilko Bulte <wkb@freebie.xs4all.nl>
To:        freebsd-current@freebsd.org
Subject:   FW: [PATCH] SysKonnect Yukon driver promiscuous mode fix
Message-ID:  <20031215231022.GA15082@freebie.xs4all.nl>

next in thread | raw e-mail | index | archive | help

--0F1p//8PRICkK4MW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

----- Forwarded message from Jung-uk Kim <jkim@niksun.com> -----

Date: Wed, 15 Oct 2003 19:40:02 -0400
From: Jung-uk Kim <jkim@niksun.com> (by way of Jung-uk Kim <jkim@niksun.com>)
Subject: [PATCH] SysKonnect Yukon driver promiscuous mode fix
To: Wilko Bulte <wkb@freebie.xs4all.nl>

I am re-sending it to your private e-mail just in case you missed 
@FreeBSD.Org e-mail. ;-)
--------------
(I am CC'ing Nathan L. Binkert because my patch was originally from
OpenBSD.)

Here is the first patch for the bug.

It is very simple to reproduce the bug.  Run tcpdump on the
 interface, e.g., 'tcpdump -i sk0'.  You will see all packets on the
 segment. While in promiscuous mode (i.e., don't quit tcpdump), try
 'ifconfig <iface> down', e.g., 'ifconfig sk0 down', and bring it
 back up, e.g., 'ifconfig sk0 up'.  You will only see your packets or
 broadcast packets.  This bug also causes multicast filter to be
 cleaned up.

This patch simplifies IFF_PROMISC checking from sk_ioctl() and
prevents the inteface from getting re-initialized when you change
other flags, e.g., arp.

Thanks,

JK

PS: Actually I found another bug, which was originally from Bill
Paul's code.  I need more test but I will let you know.

On Tuesday 14 October 2003 07:38 pm, Jung-uk Kim wrote:
> I thought you were going to MFC it after 4.9-RELEASE but...
>
> Any way, I think you have to mention that miivar.h has partial MFC
> from the following:
>
> http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/mii/miivar.h.diff
>?r1=1.10&r2=1.11
>
> Actually I got it from NetBSD.
>
> BTW, there is a buglet in the driver with promiscuous mode. ;-)
> When the card is in promiscuous mode, trying changing any flags.
> Promiscuous mode WILL not work properly because of a bug in the
> original patch.
>
> Nathan also added checksum offloading feature recently.  I am going
> to add the feature, too.
>
> So stay tuned for more patches. ;-)
>
> Thanks,
>
> JK

--
_______________________________________________________________
Jung-uk Kim			NIKSUN, Inc.
jkim@niksun.com			http://www.niksun.com
Tel: +1 732 821-5000 x3331	1100 Cornwall Road
Fax: +1 732 821-6000		Monmouth Junction, NJ 08852 USA
_______________________________________________________________






----- End forwarded message -----

-- 
|   / o / /_  _   		
|/|/ / / /(  (_)  Bulte		wilko@FreeBSD.org

--0F1p//8PRICkK4MW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sk-stable.diff"

--- src/sys/pci/if_sk.c.orig	Tue Oct 14 19:22:55 2003
+++ src/sys/pci/if_sk.c	Wed Oct 15 18:58:12 2003
@@ -217,6 +217,7 @@
 static u_int32_t sk_calchash	__P((caddr_t));
 static void sk_setfilt		__P((struct sk_if_softc *, caddr_t, int));
 static void sk_setmulti		__P((struct sk_if_softc *));
+static void sk_setpromisc	__P((struct sk_if_softc *));
 
 #ifdef SK_USEIOSPACE
 #define SK_RES		SYS_RES_IOPORT
@@ -787,6 +788,34 @@
 	return;
 }
 
+static void sk_setpromisc(sc_if)
+	struct sk_if_softc	*sc_if;
+{
+	struct sk_softc		*sc = sc_if->sk_softc;
+	struct ifnet		*ifp = &sc_if->arpcom.ac_if;
+
+	switch(sc->sk_type) {
+	case SK_GENESIS:
+		if (ifp->if_flags & IFF_PROMISC) {
+			SK_XM_SETBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
+		} else {
+			SK_XM_CLRBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
+		}
+		break;
+	case SK_YUKON:
+		if (ifp->if_flags & IFF_PROMISC) {
+			SK_YU_CLRBIT_2(sc_if, YUKON_RCR,
+			    YU_RCR_UFLEN | YU_RCR_MUFLEN);
+		} else {
+			SK_YU_SETBIT_2(sc_if, YUKON_RCR,
+			    YU_RCR_UFLEN | YU_RCR_MUFLEN);
+		}
+		break;
+	}
+
+	return;
+}
+
 static int sk_init_rx_ring(sc_if)
 	struct sk_if_softc	*sc_if;
 {
@@ -1125,7 +1154,6 @@
 	caddr_t			data;
 {
 	struct sk_if_softc	*sc_if = ifp->if_softc;
-	struct sk_softc		*sc = sc_if->sk_softc;
 	struct ifreq		*ifr = (struct ifreq *) data;
 	int			s, error = 0;
 	struct mii_data		*mii;
@@ -1147,34 +1175,12 @@
 		break;
 	case SIOCSIFFLAGS:
 		if (ifp->if_flags & IFF_UP) {
-			if (ifp->if_flags & IFF_RUNNING &&
-			    ifp->if_flags & IFF_PROMISC &&
-			    !(sc_if->sk_if_flags & IFF_PROMISC)) {
-				switch(sc->sk_type) {
-				case SK_GENESIS:
-					SK_XM_SETBIT_4(sc_if, XM_MODE,
-					    XM_MODE_RX_PROMISC);
-					break;
-				case SK_YUKON:
-					SK_YU_CLRBIT_2(sc_if, YUKON_RCR,
-					    YU_RCR_UFLEN | YU_RCR_MUFLEN);
-					break;
-				}
-				sk_setmulti(sc_if);
-			} else if (ifp->if_flags & IFF_RUNNING &&
-			    !(ifp->if_flags & IFF_PROMISC) &&
-			    sc_if->sk_if_flags & IFF_PROMISC) {
-				switch(sc->sk_type) {
-				case SK_GENESIS:
-					SK_XM_CLRBIT_4(sc_if, XM_MODE,
-					    XM_MODE_RX_PROMISC);
-					break;
-				case SK_YUKON:
-					SK_YU_SETBIT_2(sc_if, YUKON_RCR,
-					    YU_RCR_UFLEN | YU_RCR_MUFLEN);
-					break;
+			if (ifp->if_flags & IFF_RUNNING) {
+				if ((ifp->if_flags ^ sc_if->sk_if_flags)
+				    & IFF_PROMISC) {
+					sk_setpromisc(sc_if);
+					sk_setmulti(sc_if);
 				}
-				sk_setmulti(sc_if);
 			} else
 				sk_init(sc_if);
 		} else {
@@ -2258,12 +2264,6 @@
 	    *(u_int16_t *)(&sc_if->arpcom.ac_enaddr[4]));
 	SK_XM_SETBIT_4(sc_if, XM_MODE, XM_MODE_RX_USE_STATION);
 
-	if (ifp->if_flags & IFF_PROMISC) {
-		SK_XM_SETBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
-	} else {
-		SK_XM_CLRBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
-	}
-
 	if (ifp->if_flags & IFF_BROADCAST) {
 		SK_XM_CLRBIT_4(sc_if, XM_MODE, XM_MODE_RX_NOBROAD);
 	} else {
@@ -2305,6 +2305,9 @@
 	 */
 	SK_XM_WRITE_2(sc_if, XM_TX_REQTHRESH, SK_XM_TX_FIFOTHRESH);
 
+	/* Set promiscuous mode */
+	sk_setpromisc(sc_if);
+
 	/* Set multicast filter */
 	sk_setmulti(sc_if);
 
@@ -2400,8 +2403,7 @@
 	SK_YU_WRITE_2(sc_if, YUKON_PAR, reg);
 
 	/* receive control reg */
-	SK_YU_WRITE_2(sc_if, YUKON_RCR, YU_RCR_UFLEN | YU_RCR_MUFLEN |
-		      YU_RCR_CRCR);
+	SK_YU_WRITE_2(sc_if, YUKON_RCR, YU_RCR_CRCR);
 
 	/* transmit parameter register */
 	SK_YU_WRITE_2(sc_if, YUKON_TPR, YU_TPR_JAM_LEN(0x3) |
@@ -2425,11 +2427,11 @@
 		SK_YU_WRITE_2(sc_if, YUKON_SAL2 + i * 4, reg);
 	}
 
-	/* clear all Multicast filter hash registers */
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH1, 0);
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH2, 0);
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH3, 0);
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH4, 0);
+	/* Set promiscuous mode */
+	sk_setpromisc(sc_if);
+
+	/* Set multicast filter */
+	sk_setmulti(sc_if);
 
 	/* enable interrupt mask for counter overflows */
 	SK_YU_WRITE_2(sc_if, YUKON_TIMR, 0);

--0F1p//8PRICkK4MW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sk-current.diff"

--- src/sys/pci/if_sk.c.orig	Sat Sep 20 06:53:08 2003
+++ src/sys/pci/if_sk.c	Wed Oct 15 19:10:32 2003
@@ -219,6 +219,7 @@
 static u_int32_t sk_calchash	(caddr_t);
 static void sk_setfilt		(struct sk_if_softc *, caddr_t, int);
 static void sk_setmulti		(struct sk_if_softc *);
+static void sk_setpromisc	(struct sk_if_softc *);
 
 #ifdef SK_USEIOSPACE
 #define SK_RES		SYS_RES_IOPORT
@@ -815,6 +816,35 @@
 	return;
 }
 
+static void
+sk_setpromisc(sc_if)
+	struct sk_if_softc	*sc_if;
+{
+	struct sk_softc		*sc = sc_if->sk_softc;
+	struct ifnet		*ifp = &sc_if->arpcom.ac_if;
+
+	switch(sc->sk_type) {
+	case SK_GENESIS:
+		if (ifp->if_flags & IFF_PROMISC) {
+			SK_XM_SETBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
+		} else {
+			SK_XM_CLRBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
+		}
+		break;
+	case SK_YUKON:
+		if (ifp->if_flags & IFF_PROMISC) {
+			SK_YU_CLRBIT_2(sc_if, YUKON_RCR,
+			    YU_RCR_UFLEN | YU_RCR_MUFLEN);
+		} else {
+			SK_YU_SETBIT_2(sc_if, YUKON_RCR,
+			    YU_RCR_UFLEN | YU_RCR_MUFLEN);
+		}
+		break;
+	}
+
+	return;
+}
+
 static int
 sk_init_rx_ring(sc_if)
 	struct sk_if_softc	*sc_if;
@@ -1097,7 +1127,6 @@
 	caddr_t			data;
 {
 	struct sk_if_softc	*sc_if = ifp->if_softc;
-	struct sk_softc		*sc = sc_if->sk_softc;
 	struct ifreq		*ifr = (struct ifreq *) data;
 	int			error = 0;
 	struct mii_data		*mii;
@@ -1115,34 +1144,12 @@
 		break;
 	case SIOCSIFFLAGS:
 		if (ifp->if_flags & IFF_UP) {
-			if (ifp->if_flags & IFF_RUNNING &&
-			    ifp->if_flags & IFF_PROMISC &&
-			    !(sc_if->sk_if_flags & IFF_PROMISC)) {
-				switch(sc->sk_type) {
-				case SK_GENESIS:
-					SK_XM_SETBIT_4(sc_if, XM_MODE,
-					    XM_MODE_RX_PROMISC);
-					break;
-				case SK_YUKON:
-					SK_YU_CLRBIT_2(sc_if, YUKON_RCR,
-					    YU_RCR_UFLEN | YU_RCR_MUFLEN);
-					break;
-				}
-				sk_setmulti(sc_if);
-			} else if (ifp->if_flags & IFF_RUNNING &&
-			    !(ifp->if_flags & IFF_PROMISC) &&
-			    sc_if->sk_if_flags & IFF_PROMISC) {
-				switch(sc->sk_type) {
-				case SK_GENESIS:
-					SK_XM_CLRBIT_4(sc_if, XM_MODE,
-					    XM_MODE_RX_PROMISC);
-					break;
-				case SK_YUKON:
-					SK_YU_SETBIT_2(sc_if, YUKON_RCR,
-					    YU_RCR_UFLEN | YU_RCR_MUFLEN);
-					break;
+			if (ifp->if_flags & IFF_RUNNING) {
+				if ((ifp->if_flags ^ sc_if->sk_if_flags)
+				    & IFF_PROMISC) {
+					sk_setpromisc(sc_if);
+					sk_setmulti(sc_if);
 				}
-				sk_setmulti(sc_if);
 			} else
 				sk_init(sc_if);
 		} else {
@@ -2242,12 +2249,6 @@
 	    *(u_int16_t *)(&sc_if->arpcom.ac_enaddr[4]));
 	SK_XM_SETBIT_4(sc_if, XM_MODE, XM_MODE_RX_USE_STATION);
 
-	if (ifp->if_flags & IFF_PROMISC) {
-		SK_XM_SETBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
-	} else {
-		SK_XM_CLRBIT_4(sc_if, XM_MODE, XM_MODE_RX_PROMISC);
-	}
-
 	if (ifp->if_flags & IFF_BROADCAST) {
 		SK_XM_CLRBIT_4(sc_if, XM_MODE, XM_MODE_RX_NOBROAD);
 	} else {
@@ -2289,6 +2290,9 @@
 	 */
 	SK_XM_WRITE_2(sc_if, XM_TX_REQTHRESH, SK_XM_TX_FIFOTHRESH);
 
+	/* Set promiscuous mode */
+	sk_setpromisc(sc_if);
+
 	/* Set multicast filter */
 	sk_setmulti(sc_if);
 
@@ -2384,8 +2388,7 @@
 	SK_YU_WRITE_2(sc_if, YUKON_PAR, reg);
 
 	/* receive control reg */
-	SK_YU_WRITE_2(sc_if, YUKON_RCR, YU_RCR_UFLEN | YU_RCR_MUFLEN |
-		      YU_RCR_CRCR);
+	SK_YU_WRITE_2(sc_if, YUKON_RCR, YU_RCR_CRCR);
 
 	/* transmit parameter register */
 	SK_YU_WRITE_2(sc_if, YUKON_TPR, YU_TPR_JAM_LEN(0x3) |
@@ -2409,11 +2412,11 @@
 		SK_YU_WRITE_2(sc_if, YUKON_SAL2 + i * 4, reg);
 	}
 
-	/* clear all Multicast filter hash registers */
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH1, 0);
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH2, 0);
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH3, 0);
-	SK_YU_WRITE_2(sc_if, YUKON_MCAH4, 0);
+	/* Set promiscuous mode */
+	sk_setpromisc(sc_if);
+
+	/* Set multicast filter */
+	sk_setmulti(sc_if);
 
 	/* enable interrupt mask for counter overflows */
 	SK_YU_WRITE_2(sc_if, YUKON_TIMR, 0);

--0F1p//8PRICkK4MW--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031215231022.GA15082>