From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id BC0B45939 for ; Thu, 8 Sep 2016 19:07:10 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 08 Sep 2016 10:07:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,301,1470726000"; d="scan'208";a="1047435864" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.92]) ([10.237.220.92]) by orsmga002.jf.intel.com with ESMTP; 08 Sep 2016 10:07:10 -0700 To: zhouyangchao References: <1473389167-2758-1-git-send-email-zhouyates@gmail.com> Cc: dev@dpdk.org From: Ferruh Yigit Message-ID: Date: Thu, 8 Sep 2016 18:07:08 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1473389167-2758-1-git-send-email-zhouyates@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] kni: unregister an unregisterd net_device could cause a kernel crash X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Sep 2016 17:07:11 -0000 On 9/9/2016 3:46 AM, zhouyangchao wrote: > Signed-off-by: zhouyangchao > --- > lib/librte_eal/linuxapp/kni/kni_misc.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c > index 67e9b7d..ad4e603 100644 > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c > @@ -361,7 +361,9 @@ kni_dev_remove(struct kni_dev *dev) > igb_kni_remove(dev->pci_dev); > > if (dev->net_dev) { > - unregister_netdev(dev->net_dev); > + if (dev->netdev->reg_state == NETREG_REGISTERED){ Although this will work, I believe we shouldn't know more about netdev internals unless we don't have to, and for this case we don't need to know it. More Linux internal knowledge means more ways to be broken in the future. Also I am not sure if reg_state intended to be used by network drivers. I am for first version of your patch, with missing free_netdev() added into rollback code. pseudo code: net_dev = alloc_netdev() ... ret = register_netdev() if (ret) kni_dev_remove() free_netdev() return kni->net_dev = net_dev OR if don't want to move where kni->net_dev assigned net_dev = alloc_netdev() kni->net_dev = net_dev ... ret = register_netdev() if (ret) kni->net_dev = NULL kni_dev_remove() free_netdev() return > + unregister_netdev(dev->net_dev); > + } > free_netdev(dev->net_dev); > } > >