d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

It seems all Broadcom controllers have a bug that can generate UDP

Browse Source 
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
This commit is contained in:
yongari 2010-08-22 01:39:09 +00:00
parent 50ff33cdb9
commit f862610a4b
2 changed files with 43 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
sys/dev/bge/if_bge.c
View File

@ -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 @@ bge_attach(device_t dev)
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 mbuf **m_head, uint32_t *txidx)
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 command, caddr_t data)
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;

2
sys/dev/bge/if_bgereg.h
View File

@ -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;