svn commit: r211596 - head/sys/dev/bge
Pyun YongHyeon
yongari at FreeBSD.org
Sun Aug 22 01:39:09 UTC 2010
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;
More information about the svn-src-all
mailing list