From patchwork Mon Mar 7 20:15:36 2016 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: 593209 X-Patchwork-Delegate: davem@davemloft.net 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 390CE1402A0 for ; Tue, 8 Mar 2016 07:48:59 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; secure) header.d=mork.no header.i=@mork.no header.b=PtGz3i/u; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753697AbcCGUsx (ORCPT ); Mon, 7 Mar 2016 15:48:53 -0500 Received: from canardo.mork.no ([148.122.252.1]:36722 "EHLO canardo.mork.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752850AbcCGUsv convert rfc822-to-8bit (ORCPT ); Mon, 7 Mar 2016 15:48:51 -0500 Received: from nemi.mork.no (nemi.mork.no [IPv6:2001:4641:0:2:e8b:fdff:fe08:971]) (authenticated bits=0) by canardo.mork.no (8.14.4/8.14.4) with ESMTP id u27KmRqD016589 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Mon, 7 Mar 2016 21:48:27 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mork.no; s=a; t=1457383709; bh=ZgulxIGWcIlzeAbi3fwGToDAjb7y/PjbbETn16pwULY=; h=From:To:Cc:Date:References:Message-ID:From; b=PtGz3i/uipS8vMbmq3OEu3EUlWZ3h9N6msK7Vz6Wf9Y/MrCi9gFAXkgi9+X/Apn2E 6B6hx/jx9oOY+qV9E34ZWnBqor8Pgmvzb++Evtb9h57pmjqmJedA7dxtzboEwDgWmF R8IauVBpLIa0E0EsnPnRrpNpcYH1zBzCNNQgnEPfKxZ5uYCq41DiSZAtOqXdZoCyD0 GgdDrOKKphQD7C3gstP/lunyINZe2pMMx6MRZ6tXIs2ThLyODwqtHrLVp49S2qpg4X OXCfxzmE8UtMc8LLlqJOb5+xSvwk0WD4qaQTIShyqV9+IArNNjj4+W3xHbyqcavTMq k3av/XCaR+Fsg== Received: from bjorn by nemi.mork.no with local (Exim 4.84) (envelope-from ) id 1ad24p-00082A-DC; Mon, 07 Mar 2016 21:48:27 +0100 From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= To: David Miller Cc: andreyknvl@gmail.com, torvalds@linux-foundation.org, oneukum@suse.com, dvyukov@google.com, glider@google.com, kcc@google.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind Date: Mon, 7 Mar 2016 21:15:36 +0100 Organization: m References: <20160307.141100.1511700720120062677.davem@davemloft.net> <20160307.145406.188091362181073486.davem@davemloft.net> In-Reply-To: <20160307.145406.188091362181073486.davem@davemloft.net> (David Miller's message of "Mon, 07 Mar 2016 14:54:06 -0500 (EST)") Message-ID: <87k2le815w.fsf_-_@nemi.mork.no> User-Agent: Gnus/5.130015 (Ma Gnus v0.15) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.98.7 at canardo X-Virus-Status: Clean Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org usbnet_link_change will call schedule_work and should be avoided if bind is failing. Otherwise we will end up with scheduled work referring to a netdev which has gone away. Instead of making the call conditional, we can just defer it to usbnet_probe, using the driver_info flag made for this purpose. Fixes: 8a34b0ae8778 ("usbnet: cdc_ncm: apply usbnet_link_change") Reported-by: Andrey Konovalov Suggested-by: Linus Torvalds Signed-off-by: Bjørn Mork --- David Miller writes: > From: Andrey Konovalov > >> Could you also add: >> Reported-by: Andrey Konovalov >> ? > > Sorry it's already committed to my tree and I can't redo the commit message > once that happens since my tree has static history. Even with Oliver's generic fix we should still fix the inconsistency in cdc_ncm, as pointed out by Linus. This is a slightly different approach than the patch proposed by Linus. When I started looking at this I couldn't figure out why we were doing this differently in this driver from all the other usbnet drivers disabling the link at probe time. So let's make it consistent. Then at least we get consistent bugs :) Bjørn drivers/net/usb/cdc_ncm.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c index be927964375b..86ba30ba35e8 100644 --- a/drivers/net/usb/cdc_ncm.c +++ b/drivers/net/usb/cdc_ncm.c @@ -988,8 +988,6 @@ EXPORT_SYMBOL_GPL(cdc_ncm_select_altsetting); static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf) { - int ret; - /* MBIM backwards compatible function? */ if (cdc_ncm_select_altsetting(intf) != CDC_NCM_COMM_ALTSETTING_NCM) return -ENODEV; @@ -998,16 +996,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf) * Additionally, generic NCM devices are assumed to accept arbitrarily * placed NDP. */ - ret = cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0); - - /* - * We should get an event when network connection is "connected" or - * "disconnected". Set network connection in "disconnected" state - * (carrier is OFF) during attach, so the IP network stack does not - * start IPv6 negotiation and more. - */ - usbnet_link_change(dev, 0, 0); - return ret; + return cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0); } static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder, size_t max) @@ -1590,7 +1579,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb) static const struct driver_info cdc_ncm_info = { .description = "CDC NCM", - .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET, + .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET + | FLAG_LINK_INTR, .bind = cdc_ncm_bind, .unbind = cdc_ncm_unbind, .manage_power = usbnet_manage_power, @@ -1603,7 +1593,7 @@ static const struct driver_info cdc_ncm_info = { static const struct driver_info wwan_info = { .description = "Mobile Broadband Network Device", .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET - | FLAG_WWAN, + | FLAG_LINK_INTR | FLAG_WWAN, .bind = cdc_ncm_bind, .unbind = cdc_ncm_unbind, .manage_power = usbnet_manage_power, @@ -1616,7 +1606,7 @@ static const struct driver_info wwan_info = { static const struct driver_info wwan_noarp_info = { .description = "Mobile Broadband Network Device (NO ARP)", .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET - | FLAG_WWAN | FLAG_NOARP, + | FLAG_LINK_INTR | FLAG_WWAN | FLAG_NOARP, .bind = cdc_ncm_bind, .unbind = cdc_ncm_unbind, .manage_power = usbnet_manage_power,