Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932624Ab2HPTPe (ORCPT ); Thu, 16 Aug 2012 15:15:34 -0400 Received: from smtp.riverbed.com ([208.70.196.45]:20545 "EHLO smtp1.riverbed.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932163Ab2HPTPc convert rfc822-to-8bit (ORCPT ); Thu, 16 Aug 2012 15:15:32 -0400 From: Jiang Wang To: Michael Chan CC: "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Chaitanya Lala , "Francis St. Amant" Subject: RE: [PATCH] bnx2: turn off the network statck during initialization Thread-Topic: [PATCH] bnx2: turn off the network statck during initialization Thread-Index: AQHNe9wN/GY2GzvEwk20Q1fuXq+2eJddP4KA//+NcgA= Date: Thu, 16 Aug 2012 19:15:30 +0000 Message-ID: References: <1345141295-7589-1-git-send-email-Jiang.Wang@riverbed.com> <1345143431.6916.36.camel@LTIRV-MCHAN1.corp.ad.broadcom.com> In-Reply-To: <1345143431.6916.36.camel@LTIRV-MCHAN1.corp.ad.broadcom.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.16.205.250] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3353 Lines: 92 Hi Michael, I just checked the code for netif_carrier_off(), and it will return if the dev is not initialized (registered) as follows: void netif_carrier_off(struct net_device *dev) { if (!test_and_set_bit(__LINK_STATE_NOCARRIER, &dev->state)) { if (dev->reg_state == NETREG_UNINITIALIZED) return; linkwatch_fire_event(dev); } } So linkwatch_fire_event should not fire in this case, right? Thanks for the quick response. Regards, Jiang ------------------------------------- Jiang Wang Member of Technical Staff Riverbed Technology Tel: (408) 522-5109 Email: Jiang.Wang@riverbed.com www.riverbed.com -----Original Message----- From: Michael Chan [mailto:mchan@broadcom.com] Sent: Thursday, August 16, 2012 11:57 AM To: Jiang Wang Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Chaitanya Lala; Francis St. Amant; Jiang Wang Subject: Re: [PATCH] bnx2: turn off the network statck during initialization On Thu, 2012-08-16 at 11:21 -0700, Jiang Wang wrote: > The initialization state of bnx2 driver is wrong. It does not turn of > the Linux network stack using netif_carrier_off. This may lead to > inconsistent report from ethtool as the link is up but speed is > unknown when the cable is not plugged in. > > E.g. > Speed: Unknown! (0)<-------------------------------------- > Duplex: Half <-------------------------------------- > MDI: Unknown! (0) > Port: Twisted Pair > PHYAD: 1 > Transceiver: internal > Auto-negotiation: on > Supports Wake-on: g > Wake-on: d > Link detected: yes <--------------------------------------- > > This patches fixed the problem by turning off the network stack during > initialization. > > Signed-off-by: Jiang Wang > --- > drivers/net/ethernet/broadcom/bnx2.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/bnx2.c > b/drivers/net/ethernet/broadcom/bnx2.c > index ac7b744..ce4548d 100644 > --- a/drivers/net/ethernet/broadcom/bnx2.c > +++ b/drivers/net/ethernet/broadcom/bnx2.c > @@ -8463,6 +8463,10 @@ bnx2_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > dev->features |= dev->hw_features; > dev->priv_flags |= IFF_UNICAST_FLT; > > + /* tell the stack to leave us alone until bnx2_open() is called */ > + netif_carrier_off(dev); We have tried this before and this didn't work. netif_carrier_off() calls linkwatch_fire_event() to schedule the event. If register_netdev() fails for whatever reason, the netdev will be freed but the link_watch event may still be scheduled. Calling netif_carrier_off() after register_netdev() returns successfully also may not work as it will race with bnx2_open() which enables IRQ. An linkup IRQ can happen at time and we may end up with the wrong carrier state because of the race condition. > + netif_stop_queue(dev); > + > if ((rc = register_netdev(dev))) { > dev_err(&pdev->dev, "Cannot register net device\n"); > goto error; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/