Making if_bwn less chatty. Can I?

[blackhaz]

Hi All,

The original if_bwn driver floods my console with error messages like "unsupported rate 0", "decryption attempted", rxeof, etc. I have made a few changes to the original code, commenting out various device_printf sections in the /usr/src/sys/dev/bwn/if_bwn.c (please see patch below). I wonder if anybody could check if this is an OK action against the console flood, and whether the driver supplied in the FreeBSD by default indeed needs to report all that stuff into the console. The patched driver seems to work fine, and my dmesg finally makes sense now.

--- if_bwn.c.orig	2012-12-30 21:37:49.000000000 +0100
+++ if_bwn.c	2013-01-05 14:29:30.000000000 +0100
@@ -9306,8 +9306,8 @@
 	int padding, rate, rssi = 0, noise = 0, type;
 	uint16_t phytype, phystat0, phystat3, chanstat;
 	unsigned char *mp = mtod(m, unsigned char *);
-	static int rx_mac_dec_rpt = 0;
-
+/*	static int rx_mac_dec_rpt = 0;
+*/
 	BWN_ASSERT_LOCKED(sc);
 
 	phystat0 = le16toh(rxhdr->phy_status0);
@@ -9325,25 +9325,25 @@
 
 	padding = (macstat & BWN_RX_MAC_PADDING) ? 2 : 0;
 	if (m->m_pkthdr.len < (sizeof(struct bwn_plcp6) + padding)) {
-		device_printf(sc->sc_dev, "frame too short (length=%d)\n",
-		    m->m_pkthdr.len);
+		/* device_printf(sc->sc_dev, "frame too short (length=%d)\n",
+		    m->m_pkthdr.len); */
 		goto drop;
 	}
 	plcp = (struct bwn_plcp6 *)(mp + padding);
 	m_adj(m, sizeof(struct bwn_plcp6) + padding);
 	if (m->m_pkthdr.len < IEEE80211_MIN_LEN) {
-		device_printf(sc->sc_dev, "frame too short (length=%d)\n",
-		    m->m_pkthdr.len);
+		/* device_printf(sc->sc_dev, "frame too short (length=%d)\n",
+		    m->m_pkthdr.len); */
 		goto drop;
 	}
 	wh = mtod(m, struct ieee80211_frame_min *);
 
-	if (macstat & BWN_RX_MAC_DEC && rx_mac_dec_rpt++ < 50)
+/*	if (macstat & BWN_RX_MAC_DEC && rx_mac_dec_rpt++ < 50)
 		device_printf(sc->sc_dev,
-		    "RX decryption attempted (old %d keyidx %#x)\n",
+		    "RX decryption attempted (old %d keyidx %#x)\n", 
 		    BWN_ISOLDFMT(mac),
 		    (macstat & BWN_RX_MAC_KEYIDX) >> BWN_RX_MAC_KEYIDX_SHIFT);
-
+*/
 	/* XXX calculating RSSI & noise & antenna */
 
 	if (phystat0 & BWN_RX_PHYST0_OFDM)
@@ -9379,7 +9379,10 @@
 	BWN_LOCK(sc);
 	return;
 drop:
-	device_printf(sc->sc_dev, "%s: dropped\n", __func__);
+;
+/*	device_printf(sc->sc_dev, "%s: dropped\n", __func__);
+*/
+
 }
 
 static void
@@ -9587,6 +9590,8 @@
 		return (BWN_CCK_RATE_5MB);
 	case 22:
 		return (BWN_CCK_RATE_11MB);
+	case 0:
+		return (BWN_CCK_RATE_1MB);
 	}
 
 	device_printf(sc->sc_dev, "unsupported rate %d\n", rate);

 

[fonz]

I wonder if anybody could check if this is an OK action against the console flood, and whether the driver supplied in the FreeBSD by default indeed needs to report all that stuff into the console.

I can imagine how a constant barrage of messages annoys you. And although the changes you made should not break anything, you might want to wonder what's actually causing the messages in the first place. Perhaps the driver is still (somewhat) immature, perhaps the developer who wrote it made the driver a bit too verbose, maybe the WLAN itself is crappy due to improper setup/maintenance/equipment, it could be any number of things.

In short: your patch looks safe to me but you might want to wonder why it's needed in the first place.

Fonz

 

[blackhaz]

Thanks, Fonz. Sometimes I have to be pushed - too lazy!

Although my skills are extremely limited, I found a little bit more. The original code defines the DPRINTF function that handles printing error messages and other stuff in case BWN_DEBUG is defined. That's right at the top of the code. The DPRINTF also allows to use different debug levels to control how chatty the driver is. In the case BWN_DEBUG is not defined, the DPRINTF does nothing. A great deal of error messages are handled using DPRINTF in the code. Then, for some reason, some errors are handled via device_printf that ignores any debug levels and prints everything. I've glimpsed over a few other drivers' code, namely zyd from OpenBSD, and some require a certain debug level to be defined for the driver to complain, for example, on packages being too short. I also browsed forums and see people advising to ignore errors about unsupported zero rates.

I wonder what is the reason to use device_printf in some cases, instead of using DPRINTF for everything.