Joe Perches <joe@xxxxxxxxxxx> writes:
> On Thu, 2013-01-24 at 20:16 +0100, Bjørn Mork wrote:
>> A device sending 0 length frames as fast as it can has been
>> observed killing the host system due to the resulting memory
>> pressure.
> []
>> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> []
>> @@ -539,6 +545,22 @@ block:
>> break;
>> }
>>
>> + /* stop rx if packet error rate is high */
>> + if (++dev->pkt_cnt > 30) {
>> + dev->pkt_cnt = 0;
>> + dev->pkt_err = 0;
>> + } else {
>> + if (state == rx_cleanup)
>> + dev->pkt_err++;
>> + if (dev->pkt_err > 20) {
>> + set_bit(EVENT_RX_KILL, &dev->flags);
>> + if (net_ratelimit())
>> + netif_dbg(dev, rx_err, dev->net,
>> + "rx kill: high error rate\n");
>> + dev->pkt_err = 0;
>> + }
>> + }
>
> Maybe use ratelimit() here?
>
>> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> []
>> @@ -33,6 +33,7 @@ struct usbnet {
>> wait_queue_head_t *wait;
>> struct mutex phy_mutex;
>> unsigned char suspend_count;
>> + unsigned char pkt_cnt, pkt_err;
>
> and instead:
>
> struct ratelimit_state errors;
Thanks. I took a look at this, but it seems to be more complex than I
really wanted for keeping the debug noise down here. The rest of usbnet
does not care much about rate limiting debug messages at all. I'll get
a message for every 0 length packet for example.
Maybe usbnet should get a private debug ratelimiter all over?
Is the problem that these instances will hide more important net
messages? Would it help to make the ratelimit call depend on whether
debugging is enabled like ath5k and brcm80211 seems to do?
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
