Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 4 Nov 2011 22:53:52 +0000 (UTC)
From:      Pyun YongHyeon <yongari@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r227095 - head/sys/dev/ti
Message-ID:  <201111042253.pA4Mrqkm037882@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: yongari
Date: Fri Nov  4 22:53:52 2011
New Revision: 227095
URL: http://svn.freebsd.org/changeset/base/227095

Log:
  Don't abuse if_hwassist and make sure enabling corresponding TX/RX
  checksum offloading and VLAN hardware tag insertion/stripping from
  the currently enabled hardware offloading capabilities.
  Previously if_hwassist, which was initialized to TX/RX checksum
  offloading, was blindly used to enable both TX and RX checksum
  offloading such that disabling either TX or RX checksum offloading
  was not possible.
  
  ti(4) controllers support TX/RX checksum offloading with VLAN
  tagging so announce TX/RX checksum offloading capability over VLAN
  to vlan(4).
  
  Make VLAN hardware tag insertion/stripping honors currently enabled
  interface capability instead of blindly enabling VLAN hardware
  tagging. This change allows disabling hardware support of VLAN tag.
  
  Because ti(4) supports VLAN oversized frames, make network stack
  know the capability by setting if_hdrlen.
  
  While I'm here, rewrite SIOCSIFCAP handler and make sure to
  reinitialize controller whenever TX/RX checksum offloading and VLAN
  hardware tagging option is changed.  The requirement of controller
  reinitialization comes from the limitation of Tigon I/II firmware.
  Tigon I/II firmware requires all related RCBs should be
  reinitialized whenever any of its hardware offloading capabilities
  change.
  
  vlan(4) is also notified whenever the parent interface's capability
  changes such that it can correctly handle TX/RX checksum offloading
  based on parent interface's enabled offloading capabilities.
  
  RX checksum offloading handler was changed to make upper stack use
  controller computed partial checksum value.  Previously, ti(4) just
  set the computed value for any frames(IPv4, IPv6) and the value was
  not used in upper stack because driver didn't set CSUM_DATA_VALID
  such that upper network stack had to recompute checksum of TCP/UDP
  packets. I have no idea how this was not noticed for a long time.
  With this change, upper network stack does not have to fully
  recompute the checksum such that calculating pseudo checksum based
  on partial checksum is sufficient to know whether received packet's
  checksum is correct or not. However, I don't know why ti(4) does
  not have controller compute pseudo checksum as controller has
  ability to do it. I'm just guessing enabling that feature could
  trigger a firmware bug or could be slower than computing it on host
  side so just leave it as it was.
  
  In order not to produce false positives, ti(4) now checks whether
  controller actually computed IP or TCP/UDP checksum by checking
  ti_flags field.

Modified:
  head/sys/dev/ti/if_ti.c

Modified: head/sys/dev/ti/if_ti.c
==============================================================================
--- head/sys/dev/ti/if_ti.c	Fri Nov  4 21:42:13 2011	(r227094)
+++ head/sys/dev/ti/if_ti.c	Fri Nov  4 22:53:52 2011	(r227095)
@@ -1252,7 +1252,7 @@ ti_newbuf_std(struct ti_softc *sc, int i
 	r->ti_len = segs.ds_len;
 	r->ti_type = TI_BDTYPE_RECV_BD;
 	r->ti_flags = 0;
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		r->ti_flags |= TI_BDFLAG_TCP_UDP_CKSUM | TI_BDFLAG_IP_CKSUM;
 	r->ti_idx = i;
 
@@ -1299,7 +1299,7 @@ ti_newbuf_mini(struct ti_softc *sc, int 
 	r->ti_len = segs.ds_len;
 	r->ti_type = TI_BDTYPE_RECV_BD;
 	r->ti_flags = TI_BDFLAG_MINI_RING;
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		r->ti_flags |= TI_BDFLAG_TCP_UDP_CKSUM | TI_BDFLAG_IP_CKSUM;
 	r->ti_idx = i;
 
@@ -1365,7 +1365,7 @@ ti_newbuf_jumbo(struct ti_softc *sc, int
 	r->ti_len = segs.ds_len;
 	r->ti_type = TI_BDTYPE_RECV_JUMBO_BD;
 	r->ti_flags = TI_BDFLAG_JUMBO_RING;
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		r->ti_flags |= TI_BDFLAG_TCP_UDP_CKSUM | TI_BDFLAG_IP_CKSUM;
 	r->ti_idx = i;
 
@@ -1507,7 +1507,7 @@ ti_newbuf_jumbo(struct ti_softc *sc, int
 
 	r->ti_flags = TI_BDFLAG_JUMBO_RING|TI_RCB_FLAG_USE_EXT_RX_BD;
 
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		r->ti_flags |= TI_BDFLAG_TCP_UDP_CKSUM|TI_BDFLAG_IP_CKSUM;
 
 	r->ti_idx = idx;
@@ -1860,11 +1860,6 @@ ti_chipinit(struct ti_softc *sc)
 	/* Initialize link to down state. */
 	sc->ti_linkstat = TI_EV_CODE_LINK_DOWN;
 
-	if (sc->ti_ifp->if_capenable & IFCAP_HWCSUM)
-		sc->ti_ifp->if_hwassist = TI_CSUM_FEATURES;
-	else
-		sc->ti_ifp->if_hwassist = 0;
-
 	/* Set endianness before we access any non-PCI registers. */
 #if 0 && BYTE_ORDER == BIG_ENDIAN
 	CSR_WRITE_4(sc, TI_MISC_HOST_CTL,
@@ -1983,7 +1978,7 @@ ti_chipinit(struct ti_softc *sc)
 	 * the firmware racks up lots of nicDmaReadRingFull
 	 * errors.  This is not compatible with hardware checksums.
 	 */
-	if (sc->ti_ifp->if_hwassist == 0)
+	if ((sc->ti_ifp->if_capenable & (IFCAP_TXCSUM | IFCAP_RXCSUM)) == 0)
 		TI_SETBIT(sc, TI_GCR_OPMODE, TI_OPMODE_1_DMA_ACTIVE);
 
 	/* Recommended settings from Tigon manual. */
@@ -2070,10 +2065,11 @@ ti_gibinit(struct ti_softc *sc)
 	TI_HOSTADDR(rcb->ti_hostaddr) = rdphys + TI_RD_OFF(ti_rx_std_ring);
 	rcb->ti_max_len = TI_FRAMELEN;
 	rcb->ti_flags = 0;
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		rcb->ti_flags |= TI_RCB_FLAG_TCP_UDP_CKSUM |
 		     TI_RCB_FLAG_IP_CKSUM | TI_RCB_FLAG_NO_PHDR_CKSUM;
-	rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
+	if (sc->ti_ifp->if_capenable & IFCAP_VLAN_HWTAGGING)
+		rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
 
 	/* Set up the jumbo receive ring. */
 	rcb = &sc->ti_rdata->ti_info.ti_jumbo_rx_rcb;
@@ -2086,10 +2082,11 @@ ti_gibinit(struct ti_softc *sc)
 	rcb->ti_max_len = PAGE_SIZE;
 	rcb->ti_flags = TI_RCB_FLAG_USE_EXT_RX_BD;
 #endif
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		rcb->ti_flags |= TI_RCB_FLAG_TCP_UDP_CKSUM |
 		     TI_RCB_FLAG_IP_CKSUM | TI_RCB_FLAG_NO_PHDR_CKSUM;
-	rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
+	if (sc->ti_ifp->if_capenable & IFCAP_VLAN_HWTAGGING)
+		rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
 
 	/*
 	 * Set up the mini ring. Only activated on the
@@ -2103,10 +2100,11 @@ ti_gibinit(struct ti_softc *sc)
 		rcb->ti_flags = TI_RCB_FLAG_RING_DISABLED;
 	else
 		rcb->ti_flags = 0;
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_RXCSUM)
 		rcb->ti_flags |= TI_RCB_FLAG_TCP_UDP_CKSUM |
 		     TI_RCB_FLAG_IP_CKSUM | TI_RCB_FLAG_NO_PHDR_CKSUM;
-	rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
+	if (sc->ti_ifp->if_capenable & IFCAP_VLAN_HWTAGGING)
+		rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
 
 	/*
 	 * Set up the receive return ring.
@@ -2135,8 +2133,9 @@ ti_gibinit(struct ti_softc *sc)
 		rcb->ti_flags = 0;
 	else
 		rcb->ti_flags = TI_RCB_FLAG_HOST_RING;
-	rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
-	if (sc->ti_ifp->if_hwassist)
+	if (sc->ti_ifp->if_capenable & IFCAP_VLAN_HWTAGGING)
+		rcb->ti_flags |= TI_RCB_FLAG_VLAN_ASSIST;
+	if (sc->ti_ifp->if_capenable & IFCAP_TXCSUM)
 		rcb->ti_flags |= TI_RCB_FLAG_TCP_UDP_CKSUM |
 		     TI_RCB_FLAG_IP_CKSUM | TI_RCB_FLAG_NO_PHDR_CKSUM;
 	rcb->ti_max_len = TI_TX_RING_CNT;
@@ -2236,8 +2235,8 @@ ti_attach(device_t dev)
 		error = ENOSPC;
 		goto fail;
 	}
-	sc->ti_ifp->if_capabilities = IFCAP_HWCSUM |
-	    IFCAP_VLAN_HWTAGGING | IFCAP_VLAN_MTU;
+	sc->ti_ifp->if_hwassist = TI_CSUM_FEATURES;
+	sc->ti_ifp->if_capabilities = IFCAP_TXCSUM | IFCAP_RXCSUM;
 	sc->ti_ifp->if_capenable = sc->ti_ifp->if_capabilities;
 
 	/*
@@ -2479,6 +2478,13 @@ ti_attach(device_t dev)
 	 */
 	ether_ifattach(ifp, eaddr);
 
+	/* VLAN capability setup. */
+	ifp->if_capabilities |= IFCAP_VLAN_MTU | IFCAP_VLAN_HWCSUM |
+	    IFCAP_VLAN_HWTAGGING;
+	ifp->if_capenable = ifp->if_capabilities;
+	/* Tell the upper layer we support VLAN over-sized frames. */
+	ifp->if_hdrlen = sizeof(struct ether_vlan_header);
+
 	/* Driver supports link state tracking. */
 	ifp->if_capabilities |= IFCAP_LINKSTATE;
 	ifp->if_capenable |= IFCAP_LINKSTATE;
@@ -2736,12 +2742,17 @@ ti_rxeof(struct ti_softc *sc)
 		ifp->if_ipackets++;
 		m->m_pkthdr.rcvif = ifp;
 
-		if (ifp->if_hwassist) {
-			m->m_pkthdr.csum_flags |= CSUM_IP_CHECKED |
-			    CSUM_DATA_VALID;
-			if ((cur_rx->ti_ip_cksum ^ 0xffff) == 0)
-				m->m_pkthdr.csum_flags |= CSUM_IP_VALID;
-			m->m_pkthdr.csum_data = cur_rx->ti_tcp_udp_cksum;
+		if (ifp->if_capenable & IFCAP_RXCSUM) {
+			if (cur_rx->ti_flags & TI_BDFLAG_IP_CKSUM) {
+				m->m_pkthdr.csum_flags |= CSUM_IP_CHECKED;
+				if ((cur_rx->ti_ip_cksum ^ 0xffff) == 0)
+					m->m_pkthdr.csum_flags |= CSUM_IP_VALID;
+			}
+			if (cur_rx->ti_flags & TI_BDFLAG_TCP_UDP_CKSUM) {
+				m->m_pkthdr.csum_data =
+				    cur_rx->ti_tcp_udp_cksum;
+				m->m_pkthdr.csum_flags |= CSUM_DATA_VALID;
+			}
 		}
 
 		/*
@@ -3406,15 +3417,32 @@ ti_ioctl(struct ifnet *ifp, u_long comma
 	case SIOCSIFCAP:
 		TI_LOCK(sc);
 		mask = ifr->ifr_reqcap ^ ifp->if_capenable;
-		if (mask & IFCAP_HWCSUM) {
-			if (IFCAP_HWCSUM & ifp->if_capenable)
-				ifp->if_capenable &= ~IFCAP_HWCSUM;
-			else
-				ifp->if_capenable |= IFCAP_HWCSUM;
-			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
+		if ((mask & IFCAP_TXCSUM) != 0 &&
+		    (ifp->if_capabilities & IFCAP_TXCSUM) != 0) {
+			ifp->if_capenable ^= IFCAP_TXCSUM;
+			if ((ifp->if_capenable & IFCAP_TXCSUM) != 0)
+				ifp->if_hwassist |= TI_CSUM_FEATURES;
+                        else
+				ifp->if_hwassist &= ~TI_CSUM_FEATURES;
+                }
+		if ((mask & IFCAP_RXCSUM) != 0 &&
+		    (ifp->if_capabilities & IFCAP_RXCSUM) != 0)
+			ifp->if_capenable ^= IFCAP_RXCSUM;
+		if ((mask & IFCAP_VLAN_HWTAGGING) != 0 &&
+		    (ifp->if_capabilities & IFCAP_VLAN_HWTAGGING) != 0)
+                        ifp->if_capenable ^= IFCAP_VLAN_HWTAGGING;
+		if ((mask & IFCAP_VLAN_HWCSUM) != 0 &&
+		    (ifp->if_capabilities & IFCAP_VLAN_HWCSUM) != 0)
+			ifp->if_capenable ^= IFCAP_VLAN_HWCSUM;
+		if ((mask & (IFCAP_TXCSUM | IFCAP_RXCSUM |
+		    IFCAP_VLAN_HWTAGGING)) != 0) {
+			if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
+				ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
 				ti_init_locked(sc);
+			}
 		}
 		TI_UNLOCK(sc);
+		VLAN_CAPABILITIES(ifp);
 		break;
 	default:
 		error = ether_ioctl(ifp, command, data);



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