Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753000AbcLJMU5 (ORCPT ); Sat, 10 Dec 2016 07:20:57 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:48856 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752335AbcLJMU4 (ORCPT ); Sat, 10 Dec 2016 07:20:56 -0500 Date: Sat, 10 Dec 2016 13:20:59 +0100 From: Greg KH To: Stephen Hemminger Cc: Haiyang Zhang , "olaf@aepfle.de" , "jasowang@redhat.com" , "linux-kernel@vger.kernel.org" , "bjorn.helgaas@gmail.com" , "apw@canonical.com" , "devel@linuxdriverproject.org" , "leann.ogasawara@canonical.com" Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers Message-ID: <20161210122059.GA21421@kroah.com> References: <20161209073122.GB1513@kroah.com> <20161209102030.1f550c5a@xeon-e3> <20161209122935.2bd0779e@xeon-e3> <20161209134510.22087f81@xeon-e3> <20161209140509.1dd7cad2@xeon-e3> <20161209162148.44887938@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161209162148.44887938@xeon-e3> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2493 Lines: 61 On Fri, Dec 09, 2016 at 04:21:48PM -0800, Stephen Hemminger wrote: > On Fri, 9 Dec 2016 22:35:05 +0000 > Haiyang Zhang wrote: > > > > > > > > > > > Emulated NIC is already excluded in start of netvc notifier handler. > > > > > > > > > > static int netvsc_netdev_event(struct notifier_block *this, > > > > > unsigned long event, void *ptr) > > > > > { > > > > > struct net_device *event_dev = netdev_notifier_info_to_dev(ptr); > > > > > > > > > > /* Skip our own events */ > > > > > if (event_dev->netdev_ops == &device_ops) > > > > > return NOTIFY_DONE; > > > > > > > > > > > > > Emulated device is not based on netvsc. It's the native Linux > > > (dec100M?) > > > > Driver. So this line doesn't exclude it. And how about other NIC type > > > > may be added in the future? > > > > > > Sorry, forgot about that haven't used emulated device in years. > > > The emulated device should appear to be on a PCI bus, but the serial > > > would not match?? > > > > It's not a vmbus device, not a hv_pci device either. Hv_PCI is a subset > > of vmbus devices. So emulated NIC won't have hv_pci serial number. > > > > In my patch, the following code ensure, we only try to get serial number > > after confirming it's vmbus and hv_pci device: > > > > + if (!dev_is_vmbus(dev)) > > + continue; > > + > > + hdev = device_to_hv_device(dev); > > + if (hdev->device_id != HV_PCIE) > > + continue; > > Ok, the walk back up the device tree is logically ok, but I don't > know enough about PCI device tree to be assured that it is safe. > Also, you could short circuit away most of the unwanted devices > by making sure the vf_netdev->dev.parent is a PCI device. Ugh, this seems really really messy. Can't we just have the netdev_event interface pass back a pointer to something that we "know" what it is? This walking the device tree is a mess, and not good. I'd even argue that dev_is_pci() needs to be removed from the tree too, as it shouldn't be needed either. We did a lot of work on the driver model to prevent the need for having to declare the "type" of 'struct device' at all, and by doing this type of thing it goes against the basic design of the model. Yes, it makes things a bit "tougher" in places, but you don't do crazy things like walk device trees to try to find random devices and then think it's safe to actually use them :( thanks, greg k-h