From patchwork Thu Jan 24 12:47:40 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Bj=C3=B8rn_Mork?= X-Patchwork-Id: 215344 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id B472D2C0040 for ; Thu, 24 Jan 2013 23:48:52 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754321Ab3AXMsp (ORCPT ); Thu, 24 Jan 2013 07:48:45 -0500 Received: from canardo.mork.no ([148.122.252.1]:34193 "EHLO canardo.mork.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753865Ab3AXMsm convert rfc822-to-8bit (ORCPT ); Thu, 24 Jan 2013 07:48:42 -0500 Received: from nemi.mork.no (ip6-localhost [IPv6:::1]) by canardo.mork.no (8.14.3/8.14.3) with ESMTP id r0OClu6Q012590 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Thu, 24 Jan 2013 13:47:56 +0100 Received: from bjorn by nemi.mork.no with local (Exim 4.80) (envelope-from ) id 1TyMDU-0004V2-Cb; Thu, 24 Jan 2013 13:47:40 +0100 From: =?utf-8?Q?Bj=C3=B8rn_Mork?= To: Oliver Neukum Cc: linux-usb@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [RFC] net: usbnet: prevent buggy devices from killing us Organization: m References: <1359023152-32576-1-git-send-email-bjorn@mork.no> <2321910.EQLkSoxl50@linux-5eaq.site> <877gn2hlkh.fsf@nemi.mork.no> <1809412.hrq33TVdWp@linux-5eaq.site> Date: Thu, 24 Jan 2013 13:47:40 +0100 In-Reply-To: <1809412.hrq33TVdWp@linux-5eaq.site> (Oliver Neukum's message of "Thu, 24 Jan 2013 12:31:08 +0100") Message-ID: <87y5fig32r.fsf@nemi.mork.no> User-Agent: Gnus/5.11002 (No Gnus v0.20) Emacs/23.4 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-2.8 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, T_DATE_IN_FUTURE_96_Q autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on canardo.mork.no X-Virus-Scanned: clamav-milter 0.97.6 at canardo X-Virus-Status: Clean Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Oliver Neukum writes: > On Thursday 24 January 2013 12:22:54 Bjørn Mork wrote: > >> Sorry for being daft, but how do I code the "20 among the last 30" part >> there? > > Just by agreeing that you can live with false negatives but not false positives > > if (++counter > 30) { > counter = bogus = 0; > } else { > if (is_bogus(packet) > bogus++; > if (bogus > counter/2) > throttle(); > } So, add two new counters to struct usbnet for this? That seems a little overkill to me, but I don't see how else to implement anything like that. It is still not completely clear to me how the throttling/unthrottling should be done. It tested with static counters (to avoid having to rebuild everything for this test) and a new EVENT_RX_THROTTLE flag. Still on top of my previous patch just for safety while testing, as I am fed up of having to reboot all the time :-) Doing the flag test in rx_submit seems simpler than trying to track all the places this is called. Still checking the dev->done.qlen to be able to unthrottle. Was this along the lines you thought? --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 85c7ffd..e3a1d63 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -380,6 +382,13 @@ static int rx_submit (struct usbnet *dev, struct urb *urb, gfp_t flags) unsigned long lockflags; size_t size = dev->rx_urb_size; + + if (test_bit(EVENT_RX_THROTTLE, &dev->flags) && dev->done.qlen > 0) { + netif_dbg(dev, rx_err, dev->net, "%s: EVENT_RX_THROTTLE: (done.qlen=%u)\n", __func__, dev->done.qlen); + usb_free_urb(urb); + return -ENOLINK; + } + /* Do not let a device flood us to death! */ if (dev->done.qlen > 1024) { netif_dbg(dev, rx_err, dev->net, "done queue filling up (%u) - throttling\n", dev->done.qlen); @@ -482,6 +491,7 @@ static void rx_complete (struct urb *urb) struct usbnet *dev = entry->dev; int urb_status = urb->status; enum skb_state state; + static int counter, bogus; skb_put (skb, urb->actual_length); state = rx_done; @@ -547,6 +557,17 @@ block: break; } + /* keep track of bogus packet ratio */ + if (++counter > 30) { + counter = bogus = 0; + clear_bit(EVENT_RX_THROTTLE, &dev->flags); + } else { + if (state == rx_cleanup) + bogus++; + if (bogus > counter/2) + set_bit(EVENT_RX_THROTTLE, &dev->flags); + } + state = defer_bh(dev, skb, &dev->rxq, state); if (urb) {