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

From: Pavel Cahyna <pcah8322@artax.karlin.mff.cuni.cz>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: kern/32644: gsip(4) sends byte-swapped vlan tags
Date: Mon, 30 Jan 2006 16:15:54 +0100

 On Fri, Jan 27, 2006 at 03:35:02AM +0000, Christos Zoulas wrote:
 >  | --- 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?
 
 I think it can be justified algebraically:
 
 The firts patched part does:
 
 mtag(host byte order) -> bswap16 -> &= EXTSTS_VTCI -> |= EXTSTS_VPKT ->
 -> htole32 -> tx descriptor
 
 while the second does:
 
 rx descriptor -> le32toh -> &= EXTSTS_VTCI -> bswap16 -> mtag(host byte
 order)
 
 So, the second sequence is inverse to the first.
 
 Pavel