From owner-freebsd-bugs@FreeBSD.ORG Thu Nov 2 21:50:31 2006 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2101316A40F for ; Thu, 2 Nov 2006 21:50:31 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id A9A6C43D58 for ; Thu, 2 Nov 2006 21:50:30 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id kA2LoShX089846 for ; Thu, 2 Nov 2006 21:50:28 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id kA2LoSkb089845; Thu, 2 Nov 2006 21:50:28 GMT (envelope-from gnats) Date: Thu, 2 Nov 2006 21:50:28 GMT Message-Id: <200611022150.kA2LoSkb089845@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Yo Its Me Remember Cc: Subject: Re: kern/105054: [vge] [patch] vge driver sends VLAN IDs in wrong endian. X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Yo Its Me Remember List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Nov 2006 21:50:31 -0000 The following reply was made to PR kern/105054; it has been noted by GNATS. From: Yo Its Me Remember To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/105054: [vge] [patch] vge driver sends VLAN IDs in wrong endian. Date: Thu, 2 Nov 2006 13:43:34 -0800 (PST) --0-15573433-1162503814=:18302 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Yeah, I thought it was pretty strange too, but it's up and running with my patch. However, now that i think about it, htole16() shouldn't be there, since we're doing bitwise OR we should just leave them both in their native endians and then let the htole32() convert the final product to the correct (little) endian. Only affects big-endian systems, but would end up causing the same bug that we're trying to fix on them. Coinsidentally, the original code would work fine on big-endian systems. ;) If no one on the FBSD team has a vge card, I'd be more than happy to donate money to buy one (they're ~$15 shipped). Or perhaps we could ask VIA for details... The final patch that I have working (and should work for big-endian systems) looks like this: *** 1819,1825 **** mtag = VLAN_OUTPUT_TAG(sc->vge_ifp, m_head); if (mtag != NULL) sc->vge_ldata.vge_tx_list[idx].vge_ctl |= ! htole32(htons(VLAN_TAG_VALUE(mtag)) | VGE_TDCTL_VTAG); sc->vge_ldata.vge_tx_list[idx].vge_sts |= htole32(VGE_TDSTS_OWN); --- 1819,1825 ---- mtag = VLAN_OUTPUT_TAG(sc->vge_ifp, m_head); if (mtag != NULL) sc->vge_ldata.vge_tx_list[idx].vge_ctl |= ! htole32(VLAN_TAG_VALUE(mtag) | VGE_TDCTL_VTAG); sc->vge_ldata.vge_tx_list[idx].vge_sts |= htole32(VGE_TDSTS_OWN); ...looks similar for HEAD, just strip out that one htons() on that line. Thanks, Ian --------------------------------- Low, Low, Low Rates! Check out Yahoo! Messenger's cheap PC-to-Phone call rates. --0-15573433-1162503814=:18302 Content-Type: text/html; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Yeah, I thought it was pretty strange too, but it's up and running with my patch.  However, now that i think about it, htole16() shouldn't be there, since we're doing bitwise OR we should just leave them both in their native endians and then let the htole32() convert the final product to the correct (little) endian.  Only affects big-endian systems, but would end up causing the same bug that we're trying to fix on them.  Coinsidentally, the original code would work fine on big-endian systems . ;)

If no one on the FBSD team has a vge card, I'd be more than happy to donate money to buy one (they're ~$15 shipped).  Or perhaps we could ask VIA for details...

The final patch that I have working (and should work for big-endian systems) looks like this:

*** 1819,1825 ****
         mtag = VLAN_OUTPUT_TAG(sc->vge_ifp, m_head);
         if (mtag != NULL)
                
  sc->vge_ldata.vge_tx_list[idx].vge_ctl |=
!                    htole32(htons(VLAN_TAG_VALUE(mtag)) | VGE_TDCTL_VTAG);
         sc->vge_ldata.vge_tx_list[idx].vge_sts |= htole32(VGE_TDSTS_OWN);
--- 1819,1825 ----
         mtag = VLAN_OUTPUT_TAG(sc->vge_ifp, m_head);
         if (mtag != NULL)
                 sc->vge_ldata.vge_tx_list[idx].vge_ctl |=
!                     htole32(VLAN_TAG_VALUE(mtag) | VGE_TDCTL_VTAG);
         sc->vge_ldata.vge_tx_list[idx].vge_sts |= htole32(VGE_TDSTS_OWN);
...looks similar for HEAD, just strip out that one htons() on that line.

Thanks,
Ian

Low, Low, Low Rates! Check out Yahoo! Messenger's cheap PC-to-Phone call rates. --0-15573433-1162503814=:18302--