The following reply was made to PR kern/32644; it has been noted by GNATS.

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: kern/32644: gsip(4) sends byte-swapped vlan tags
Date: Thu, 26 Jan 2006 22:30:14 -0500

 On Jan 26, 10:25pm, pcah8322@artax.karlin.mff.cuni.cz (Pavel Cahyna) wrote:
 -- Subject: kern/32644: gsip(4) sends byte-swapped vlan tags
 
 | >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);
 |  		}
 |  
 -- End of excerpt from Pavel Cahyna
 
 
 This does not seem right because in the first part of the patch you do:
 
 +			    htole32(EXTSTS_VPKT | 
 +					(bswap16(VLAN_TAG_VALUE(mtag)) &
 +					 EXTSTS_VTCI));
 
 and in the second:
 
 | +			VLAN_INPUT_TAG(ifp, m, bswap16(extsts & EXTSTS_VTCI),
 
 because in the first patch you are not bswapping16 EXTSTS_VTCI and in the
 second you do?
 
 christos