Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 22 Aug 2010 01:39:09 +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: r211596 - head/sys/dev/bge
Message-ID:  <201008220139.o7M1d9r8086345@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: yongari
Date: Sun Aug 22 01:39:09 2010
New Revision: 211596
URL: http://svn.freebsd.org/changeset/base/211596

Log:
  It seems all Broadcom controllers have a bug that can generate UDP
  datagrams with checksum value 0 when TX UDP checksum offloading is
  enabled.  Generating UDP checksum value 0 is RFC 768 violation.
  Even though the probability of generating such UDP datagrams is
  low, I don't want to see FreeBSD boxes to inject such datagrams
  into network so disable UDP checksum offloading by default.  Users
  still override this behavior by setting a sysctl variable or loader
  tunable, dev.bge.%d.forced_udpcsum.
  
  I have no idea why this issue was not reported so far given that
  bge(4) is one of the most commonly used controller on high-end
  server class systems. Thanks to andre@ who passed the PR to me.
  
  PR:	kern/104826

Modified:
  head/sys/dev/bge/if_bge.c
  head/sys/dev/bge/if_bgereg.h

Modified: head/sys/dev/bge/if_bge.c
==============================================================================
--- head/sys/dev/bge/if_bge.c	Sun Aug 22 00:04:24 2010	(r211595)
+++ head/sys/dev/bge/if_bge.c	Sun Aug 22 01:39:09 2010	(r211596)
@@ -120,7 +120,7 @@ __FBSDID("$FreeBSD$");
 
 #include <dev/bge/if_bgereg.h>
 
-#define	BGE_CSUM_FEATURES	(CSUM_IP | CSUM_TCP | CSUM_UDP)
+#define	BGE_CSUM_FEATURES	(CSUM_IP | CSUM_TCP)
 #define	ETHER_MIN_NOPAD		(ETHER_MIN_LEN - ETHER_CRC_LEN) /* i.e., 60 */
 
 MODULE_DEPEND(bge, pci, 1, 1, 1);
@@ -2795,6 +2795,8 @@ bge_attach(device_t dev)
 		goto fail;
 	}
 
+	bge_add_sysctls(sc);
+
 	/* Set default tuneable values. */
 	sc->bge_stat_ticks = BGE_TICKS_PER_SEC;
 	sc->bge_rx_coal_ticks = 150;
@@ -2802,6 +2804,11 @@ bge_attach(device_t dev)
 	sc->bge_rx_max_coal_bds = 10;
 	sc->bge_tx_max_coal_bds = 10;
 
+	/* Initialize checksum features to use. */
+	sc->bge_csum_features = BGE_CSUM_FEATURES;
+	if (sc->bge_forced_udpcsum != 0)
+		sc->bge_csum_features |= CSUM_UDP;
+
 	/* Set up ifnet structure */
 	ifp = sc->bge_ifp = if_alloc(IFT_ETHER);
 	if (ifp == NULL) {
@@ -2818,7 +2825,7 @@ bge_attach(device_t dev)
 	ifp->if_snd.ifq_drv_maxlen = BGE_TX_RING_CNT - 1;
 	IFQ_SET_MAXLEN(&ifp->if_snd, ifp->if_snd.ifq_drv_maxlen);
 	IFQ_SET_READY(&ifp->if_snd);
-	ifp->if_hwassist = BGE_CSUM_FEATURES;
+	ifp->if_hwassist = sc->bge_csum_features;
 	ifp->if_capabilities = IFCAP_HWCSUM | IFCAP_VLAN_HWTAGGING |
 	    IFCAP_VLAN_MTU;
 	if ((sc->bge_flags & BGE_FLAG_TSO) != 0) {
@@ -2975,8 +2982,6 @@ again:
 		device_printf(sc->bge_dev, "couldn't set up irq\n");
 	}
 
-	bge_add_sysctls(sc);
-
 	return (0);
 
 fail:
@@ -3960,7 +3965,7 @@ bge_encap(struct bge_softc *sc, struct m
 			return (ENOBUFS);
 		csum_flags |= BGE_TXBDFLAG_CPU_PRE_DMA |
 		    BGE_TXBDFLAG_CPU_POST_DMA;
-	} else if ((m->m_pkthdr.csum_flags & BGE_CSUM_FEATURES) != 0) {
+	} else if ((m->m_pkthdr.csum_flags & sc->bge_csum_features) != 0) {
 		if (m->m_pkthdr.csum_flags & CSUM_IP)
 			csum_flags |= BGE_TXBDFLAG_IP_CSUM;
 		if (m->m_pkthdr.csum_flags & (CSUM_TCP | CSUM_UDP)) {
@@ -4237,6 +4242,17 @@ bge_init_locked(struct bge_softc *sc)
 	/* Program VLAN tag stripping. */
 	bge_setvlan(sc);
 
+	/* Override UDP checksum offloading. */
+	if (sc->bge_forced_udpcsum == 0)
+		sc->bge_csum_features &= ~CSUM_UDP;
+	else
+		sc->bge_csum_features |= CSUM_UDP;
+	if (ifp->if_capabilities & IFCAP_TXCSUM &&
+	    ifp->if_capenable & IFCAP_TXCSUM) {
+		ifp->if_hwassist &= ~(BGE_CSUM_FEATURES | CSUM_UDP);
+		ifp->if_hwassist |= sc->bge_csum_features;
+	}
+
 	/* Init RX ring. */
 	if (bge_init_rx_ring_std(sc) != 0) {
 		device_printf(sc->bge_dev, "no memory for std Rx buffers.\n");
@@ -4562,9 +4578,9 @@ bge_ioctl(struct ifnet *ifp, u_long comm
 			ifp->if_capenable ^= IFCAP_HWCSUM;
 			if (IFCAP_HWCSUM & ifp->if_capenable &&
 			    IFCAP_HWCSUM & ifp->if_capabilities)
-				ifp->if_hwassist |= BGE_CSUM_FEATURES;
+				ifp->if_hwassist |= sc->bge_csum_features;
 			else
-				ifp->if_hwassist &= ~BGE_CSUM_FEATURES;
+				ifp->if_hwassist &= ~sc->bge_csum_features;
 		}
 
 		if ((mask & IFCAP_TSO4) != 0 &&
@@ -4940,6 +4956,24 @@ bge_add_sysctls(struct bge_softc *sc)
 	    "Number of fragmented TX buffers of a frame allowed before "
 	    "forced collapsing");
 
+	/*
+	 * It seems all Broadcom controllers have a bug that can generate UDP
+	 * datagrams with checksum value 0 when TX UDP checksum offloading is
+	 * enabled.  Generating UDP checksum value 0 is RFC 768 violation.
+	 * Even though the probability of generating such UDP datagrams is
+	 * low, I don't want to see FreeBSD boxes to inject such datagrams
+	 * into network so disable UDP checksum offloading by default.  Users
+	 * still override this behavior by setting a sysctl variable,
+	 * dev.bge.0.forced_udpcsum.
+	 */
+	sc->bge_forced_udpcsum = 0;
+	snprintf(tn, sizeof(tn), "dev.bge.%d.bge_forced_udpcsum", unit);
+	TUNABLE_INT_FETCH(tn, &sc->bge_forced_udpcsum);
+	SYSCTL_ADD_INT(ctx, children, OID_AUTO, "forced_udpcsum",
+	    CTLFLAG_RW, &sc->bge_forced_udpcsum, 0,
+	    "Enable UDP checksum offloading even if controller can "
+	    "generate UDP checksum value 0");
+
 	if (BGE_IS_5705_PLUS(sc))
 		return;
 

Modified: head/sys/dev/bge/if_bgereg.h
==============================================================================
--- head/sys/dev/bge/if_bgereg.h	Sun Aug 22 00:04:24 2010	(r211595)
+++ head/sys/dev/bge/if_bgereg.h	Sun Aug 22 01:39:09 2010	(r211596)
@@ -2644,6 +2644,8 @@ struct bge_softc {
 	int			bge_link_evt;	/* pending link event */
 	int			bge_timer;
 	int			bge_forced_collapse;
+	int			bge_forced_udpcsum;
+	int			bge_csum_features;
 	struct callout		bge_stat_ch;
 	uint32_t		bge_rx_discards;
 	uint32_t		bge_tx_discards;



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