Subject: kern/32644: gsip(4) sends byte-swapped vlan tags
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Pavel Cahyna <pcah8322@artax.karlin.mff.cuni.cz>
List: netbsd-bugs
Date: 01/26/2006 22:25:00
>Number:         32644
>Category:       kern
>Synopsis:       gsip(4) sends byte-swapped vlan tags
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jan 26 22:25:00 +0000 2006
>Originator:     Pavel Cahyna
>Release:        NetBSD 3.0
>Organization:
>Environment:
System: NetBSD beta.imc.cas.cz 3.0 NetBSD 3.0 (GENERIC-RLPHY) #3: Thu Jan 26 16:18:14 CET 2006 cahyna@beta.imc.cas.cz:/usr/src/sys/arch/i386/compile/GENERIC-RLPHY i386
Architecture: i386
Machine: i386
>Description:
When using a tagged VLAN on a gsip(4) NIC, I discovered that the VLAN
tag is sent byte-swapped. If I configure it as 4, I see 1024 on the
wire. If I configure 1024, I see 4.  If I configure 2, I see 512.

See also http://mail-index.netbsd.org/tech-net/2006/01/24/0003.html
and several following messages for attempts at analyzing the problem.
>How-To-Repeat:
Configure a vlan on a gsip interface, watch with tcpdump on another
machine in the same Ethernet segment.
>Fix:
(tested on both i386 - little endian and sgimips - big endian)
--- if_sip.c.~1.101.~	Sun Feb 27 01:27:33 2005
+++ if_sip.c	Wed Jan 25 00:51:11 2006
@@ -1354,10 +1354,20 @@
 		 * This apparently has to be on the last descriptor of
 		 * the packet.
 		 */
+
+		/*
+		 * Byte swapping is tricky. We need to provide the tag
+		 * in a network byte order. On a big-endian machine,
+		 * the byteorder is correct, but we need to swap it
+		 * anyway, because this will be undone by the outside
+		 * htole32(). That's why there must be an
+		 * unconditional swap instead of htons() inside.
+		 */
 		if ((mtag = VLAN_OUTPUT_TAG(&sc->sc_ethercom, m0)) != NULL) {
 			sc->sc_txdescs[lasttx].sipd_extsts |=
-			    htole32(EXTSTS_VPKT |
-				    (VLAN_TAG_VALUE(mtag) & EXTSTS_VTCI));
+			    htole32(EXTSTS_VPKT | 
+					(bswap16(VLAN_TAG_VALUE(mtag)) &
+					 EXTSTS_VTCI));
 		}
 
 		/*
@@ -1969,8 +1979,19 @@
 		 * If VLANs are enabled, VLAN packets have been unwrapped
 		 * for us.  Associate the tag with the packet.
 		 */
+
+		/*
+		 * Again, byte swapping is tricky. Hardware provided
+		 * the tag in the network byte order, but extsts was
+		 * passed through le32toh() in the meantime. On a
+		 * big-endian machine, we need to swap it again. On a
+		 * little-endian machine, we need to convert from the
+		 * network to host byte order. This means that we must
+		 * swap it in any case, so unconditional swap instead
+		 * of htons() is used.
+		 */
 		if ((extsts & EXTSTS_VPKT) != 0) {
-			VLAN_INPUT_TAG(ifp, m, ntohs(extsts & EXTSTS_VTCI),
+			VLAN_INPUT_TAG(ifp, m, bswap16(extsts & EXTSTS_VTCI),
 			    continue);
 		}