From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756746AbcLPBMn (ORCPT ); Thu, 15 Dec 2016 20:12:43 -0500 Received: from mail-by2nam03on0130.outbound.protection.outlook.com ([104.47.42.130]:56224 "EHLO NAM03-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754230AbcLPBMf (ORCPT ); Thu, 15 Dec 2016 20:12:35 -0500 From: KY Srinivasan To: Stephen Hemminger , Greg KH CC: "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" , "Haiyang Zhang" Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers Thread-Topic: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers Thread-Index: AQHSUR217WryFwQHM0aSgrk15jlaWqD+NO0AgABUCDCAALFKAIAAtV0AgAAei4CAAAWGgIAAEUeAgAAD1wCAAAJrgIAAAyuAgAAIXYCAAB3RAIAAyPCAgAcBK4CAAAKCAIAABpgAgAGl5UA= Date: Fri, 16 Dec 2016 01:11:11 +0000 Message-ID: References: <20161209122935.2bd0779e@xeon-e3> <20161209134510.22087f81@xeon-e3> <20161209140509.1dd7cad2@xeon-e3> <20161209162148.44887938@xeon-e3> <20161210122059.GA21421@kroah.com> <20161214232758.GA24234@kroah.com> <20161214155134.2015831f@xeon-e3> In-Reply-To: <20161214155134.2015831f@xeon-e3> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=kys@microsoft.com; x-originating-ip: [131.107.147.197] x-o365eop-header: O365_EOP: Allow for Unauthenticated Relay x-o365ent-eop-header: Message processed by - O365_ENT: Allow from ranges (Engineering ONLY) x-ms-office365-filtering-correlation-id: 6b0c29fe-a837-4fc1-c7b3-08d425506c8b x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:CY1PR03MB1423; x-microsoft-exchange-diagnostics: 1;CY1PR03MB1423;7:iad8R2BqR8D0wbgazs3RWIkMjAe1JwWcfy424STE606GgrkU3QbAUW3RomsdNGKhFKYmjmy94ynb7YydXcqJWtWWwJuzVPfmlWcmUKhCxWqAHk5Sw769Vy3WCd3I7AjCtFIEEIQJs7FqlsDPOJgAu+56Oi7x+qKyCM6EsaZsXmyYuL/53/XJQruBy36PV1hOgylqp8ueHryU44/yvZgiEI4aqKRU0LO7fLXNsRvB3eyOXZDK6vsPOc24HMgq3FtGHSaD0nriR+BhD0uCdumU+LLtNDEKjNnMGu4gS2FikY4x8NtV4+4G9LMAQlAwn3VqV5MUVKdEfK9qRG1DxEHiaM5x8pC8YYcN3zNtO0UFXa+S9SRxK14SXulV8KnozuOoNItRMAKgkPdF+sTwuAOfcVpqIHhDNlH9ARnZr5g1P99t4O/qirz49aEbu0goY8sXB4zgVefrP+39Lvn3MqgS/jEm9ubY9vbw1VD9fPt5UGs= x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(9452136761055)(189930954265078)(219752817060721)(198206253151910); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(61425038)(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(61426038)(61427038)(6041248)(20161123555025)(20161123560025)(20161123562025)(20161123564025)(6047074)(6072148);SRVR:CY1PR03MB1423;BCL:0;PCL:0;RULEID:;SRVR:CY1PR03MB1423; x-forefront-prvs: 01583E185C x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(7916002)(39850400002)(39840400002)(39410400002)(39860400002)(39450400003)(377454003)(199003)(189002)(13464003)(24454002)(40224003)(2900100001)(189998001)(8676002)(6506006)(81166006)(81156014)(92566002)(25786008)(102836003)(106116001)(97736004)(6436002)(7696004)(68736007)(38730400001)(86362001)(106356001)(3846002)(305945005)(107886002)(6116002)(4001430100002)(575784001)(39060400001)(2906002)(99286002)(66066001)(86612001)(74316002)(77096006)(93886004)(4326007)(105586002)(8936002)(122556002)(7736002)(76576001)(5001770100001)(54356999)(229853002)(33656002)(50986999)(9686002)(3280700002)(5005710100001)(3660700001)(8990500004)(10290500002)(5660300001)(76176999)(2950100002)(101416001)(10090500001);DIR:OUT;SFP:1102;SCL:1;SRVR:CY1PR03MB1423;H:DM5PR03MB2490.namprd03.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Dec 2016 01:11:11.1771 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR03MB1423 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uBG1CvvA018658 > -----Original Message----- > From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On > Behalf Of Stephen Hemminger > Sent: Wednesday, December 14, 2016 3:52 PM > To: Greg KH > Cc: 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; Haiyang > Zhang > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial > numbers > > On Wed, 14 Dec 2016 15:27:58 -0800 > Greg KH wrote: > > > On Wed, Dec 14, 2016 at 11:18:59PM +0000, Haiyang Zhang wrote: > > > > > > > > > > -----Original Message----- > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org] > > > > Sent: Saturday, December 10, 2016 7:21 AM > > > > 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 > > > > > > > > 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 :( > > > > > > > > > > We register a notifier_block with: > > > register_netdevice_notifier(struct notifier_block *nb) > > > > > > The "struct notifier_block" basically contains a callback function: > > > struct notifier_block { > > > notifier_fn_t notifier_call; > > > struct notifier_block __rcu *next; > > > int priority; > > > }; > > > > > > It doesn't specify which device we want, so all net devices can trigger > > > this event. Seems we can't have this notifier return VF device only. > > > > Ok, I dug in the kernel and it looks like people check the netdev_ops > > structure to see if it matches up with their function pointers to "know" > > if this is their device or not. Why not do that here? Or compare the > > "string" of the driver name? Or any other such trick that the drivers > > that call register_netdevice_notifier do? > > > > All of which are more sane than walking the device tree... > > > > And why am I having to do network driver development, ick ick ick :) > > > > Come on, 'git grep' is your friend. Even better yet, use a good tool > > like 'vgrep' which makes git grep work really really well. > > Normally, that would work but in this case we have one driver (netvsc) > which is managing another driver which is unaware of Hyper-V or netvsc > drivers existence. The callback is happening in netvsc driver and it > needs to say "hey I know that SR-IOV device, it is associated with my > network device". This problem is how to know that N is associated with > V? The V device has to be a network device, that is easy. But then it > also has to be a PCI device, not to bad. But then the netvsc code > is matching based on hyper-V only PCI bus metadata (the serial #). > > The Microsoft developers made the rational decision not to go modifying > all the possible SR-IOV network devices from Intel and Mellanox to add > the functionality there. That would have been much worse. > > Maybe, rather than trying to do the management in the kernel it > could have been done better in user space. Unfortunately, this would > only move the problem. The PCI-hyperv host driver could expose serial > value through sysfs (with some pain). But the problem would be how > to make a new API to join the two V and N device. Doing a private > ioctl is worse than the notifier. All this has been discussed earlier in the thread. I think I have a solution to the problem: The only PCI (non-VF) NIC that may be present in the VM is the emulated NIC and we know exactly the device ID and vendor ID of this NIC. Furthermore, as a platform we are not going to be emulating additional NICs. So, if the PCI NIC is not the emulated NIC, it must be a VF and we can extract the serial number. Regards, K. Y > _______________________________________________ > devel mailing list > devel@linuxdriverproject.org > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdriverd > ev.linuxdriverproject.org%2Fmailman%2Flistinfo%2Fdriverdev- > devel&data=02%7C01%7Ckys%40microsoft.com%7C77c2c8a38fe2431945e408 > d4247c2c7d%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C63617356 > 3122444667&sdata=u5C0v7ixzRu%2Btw51tTzHNpbsNqCDQTpigzUtwahIPvE% > 3D&reserved=0