RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-16 Thread KY Srinivasan


> -Original Message-
> From: Haiyang Zhang
> Sent: Friday, December 16, 2016 7:21 AM
> To: KY Srinivasan <k...@microsoft.com>; Stephen Hemminger
> <step...@networkplumber.org>; Greg KH <gre...@linuxfoundation.org>
> Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org; leann.ogasaw...@canonical.com
> Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> numbers
> 
> 
> 
> > -Original Message-
> > From: KY Srinivasan
> > Sent: Thursday, December 15, 2016 8:11 PM
> > To: Stephen Hemminger <step...@networkplumber.org>; Greg KH
> > <gre...@linuxfoundation.org>
> > Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; Haiyang Zhang
> <haiya...@microsoft.com>
> > Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> >
> >
> >
> > > -Original Message-
> > > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org]
> On
> > > Behalf Of Stephen Hemminger
> > > Sent: Wednesday, December 14, 2016 3:52 PM
> > > To: Greg KH <gre...@linuxfoundation.org>
> > > Cc: o...@aepfle.de; jasow...@redhat.com; linux-
> ker...@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > de...@linuxdriverproject.org; leann.ogasaw...@canonical.com; Haiyang
> > > Zhang <haiya...@microsoft.com>
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial
> > > numbers
> > >
> > > 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.
> 
> How about direct pass-through NIC devices. Do they have vPCI serial
> number?
> And, the numbers should be different from VF NIC?

This may not have a valid serial number; but is still a descendent of vmbus.

K. Y  
> 
> Thanks,
> - Haiyang



RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-16 Thread KY Srinivasan


> -Original Message-
> From: Haiyang Zhang
> Sent: Friday, December 16, 2016 7:21 AM
> To: KY Srinivasan ; Stephen Hemminger
> ; Greg KH 
> Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org; leann.ogasaw...@canonical.com
> Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> numbers
> 
> 
> 
> > -Original Message-
> > From: KY Srinivasan
> > Sent: Thursday, December 15, 2016 8:11 PM
> > To: Stephen Hemminger ; Greg KH
> > 
> > Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; Haiyang Zhang
> 
> > Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> >
> >
> >
> > > -Original Message-
> > > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org]
> On
> > > Behalf Of Stephen Hemminger
> > > Sent: Wednesday, December 14, 2016 3:52 PM
> > > To: Greg KH 
> > > Cc: o...@aepfle.de; jasow...@redhat.com; linux-
> ker...@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > de...@linuxdriverproject.org; leann.ogasaw...@canonical.com; Haiyang
> > > Zhang 
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial
> > > numbers
> > >
> > > 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.
> 
> How about direct pass-through NIC devices. Do they have vPCI serial
> number?
> And, the numbers should be different from VF NIC?

This may not have a valid serial number; but is still a descendent of vmbus.

K. Y  
> 
> Thanks,
> - Haiyang



Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-16 Thread Greg KH
On Wed, Dec 14, 2016 at 03:51:34PM -0800, Stephen Hemminger wrote:
> On Wed, 14 Dec 2016 15:27:58 -0800
> Greg KH <gre...@linuxfoundation.org> wrote:
> 
> > On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> > > 
> > >   
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Saturday, December 10, 2016 7:21 AM
> > > > To: Stephen Hemminger <step...@networkplumber.org>
> > > > Cc: Haiyang Zhang <haiya...@microsoft.com>; o...@aepfle.de;
> > > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com; 
> > > > de...@linuxdriverproject.org;
> > > > leann.ogasaw...@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 +
> > > > > Haiyang Zhang <haiya...@microsoft.com> 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 == _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.
> >

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-16 Thread Greg KH
On Wed, Dec 14, 2016 at 03:51:34PM -0800, Stephen Hemminger wrote:
> On Wed, 14 Dec 2016 15:27:58 -0800
> Greg KH  wrote:
> 
> > On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> > > 
> > >   
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Saturday, December 10, 2016 7:21 AM
> > > > To: Stephen Hemminger 
> > > > Cc: Haiyang Zhang ; o...@aepfle.de;
> > > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com; 
> > > > de...@linuxdriverproject.org;
> > > > leann.ogasaw...@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 +
> > > > > 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 == _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
> > > &

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-16 Thread Haiyang Zhang


> -Original Message-
> From: KY Srinivasan
> Sent: Thursday, December 15, 2016 8:11 PM
> To: Stephen Hemminger <step...@networkplumber.org>; Greg KH
> <gre...@linuxfoundation.org>
> Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; Haiyang Zhang <haiya...@microsoft.com>
> Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> 
> 
> > -Original Message-
> > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > Behalf Of Stephen Hemminger
> > Sent: Wednesday, December 14, 2016 3:52 PM
> > To: Greg KH <gre...@linuxfoundation.org>
> > Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com;
> > de...@linuxdriverproject.org; leann.ogasaw...@canonical.com; Haiyang
> > Zhang <haiya...@microsoft.com>
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial
> > numbers
> >
> > 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.

How about direct pass-through NIC devices. Do they have vPCI serial number?
And, the numbers should be different from VF NIC?

Thanks,
- Haiyang



RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-16 Thread Haiyang Zhang


> -Original Message-
> From: KY Srinivasan
> Sent: Thursday, December 15, 2016 8:11 PM
> To: Stephen Hemminger ; Greg KH
> 
> Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; Haiyang Zhang 
> Subject: RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> 
> 
> > -Original Message-
> > From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> > Behalf Of Stephen Hemminger
> > Sent: Wednesday, December 14, 2016 3:52 PM
> > To: Greg KH 
> > Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com;
> > de...@linuxdriverproject.org; leann.ogasaw...@canonical.com; Haiyang
> > Zhang 
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial
> > numbers
> >
> > 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.

How about direct pass-through NIC devices. Do they have vPCI serial number?
And, the numbers should be different from VF NIC?

Thanks,
- Haiyang



RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-15 Thread KY Srinivasan


> -Original Message-
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Stephen Hemminger
> Sent: Wednesday, December 14, 2016 3:52 PM
> To: Greg KH <gre...@linuxfoundation.org>
> Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org; leann.ogasaw...@canonical.com; Haiyang
> Zhang <haiya...@microsoft.com>
> 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 <gre...@linuxfoundation.org> wrote:
> 
> > On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Saturday, December 10, 2016 7:21 AM
> > > > To: Stephen Hemminger <step...@networkplumber.org>
> > > > Cc: Haiyang Zhang <haiya...@microsoft.com>; o...@aepfle.de;
> > > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > > leann.ogasaw...@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 +
> > > > > Haiyang Zhang <haiya...@microsoft.com> 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 == _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.
> > > >
&

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-15 Thread KY Srinivasan


> -Original Message-
> From: devel [mailto:driverdev-devel-boun...@linuxdriverproject.org] On
> Behalf Of Stephen Hemminger
> Sent: Wednesday, December 14, 2016 3:52 PM
> To: Greg KH 
> Cc: o...@aepfle.de; jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org; leann.ogasaw...@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 +, Haiyang Zhang wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Saturday, December 10, 2016 7:21 AM
> > > > To: Stephen Hemminger 
> > > > Cc: Haiyang Zhang ; o...@aepfle.de;
> > > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > > leann.ogasaw...@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 +
> > > > > 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 == _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
> >

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-14 Thread Haiyang Zhang


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, December 10, 2016 7:21 AM
> To: Stephen Hemminger <step...@networkplumber.org>
> Cc: Haiyang Zhang <haiya...@microsoft.com>; o...@aepfle.de;
> jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@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 +
> > Haiyang Zhang <haiya...@microsoft.com> 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 == _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.

Thanks,
- Haiyang



RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-14 Thread Haiyang Zhang


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Saturday, December 10, 2016 7:21 AM
> To: Stephen Hemminger 
> Cc: Haiyang Zhang ; o...@aepfle.de;
> jasow...@redhat.com; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@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 +
> > 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 == _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.

Thanks,
- Haiyang



Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-14 Thread Stephen Hemminger
On Wed, 14 Dec 2016 15:27:58 -0800
Greg KH <gre...@linuxfoundation.org> wrote:

> On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> > 
> >   
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Saturday, December 10, 2016 7:21 AM
> > > To: Stephen Hemminger <step...@networkplumber.org>
> > > Cc: Haiyang Zhang <haiya...@microsoft.com>; o...@aepfle.de;
> > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > > leann.ogasaw...@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 +
> > > > Haiyang Zhang <haiya...@microsoft.com> 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 == _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:
> &g

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-14 Thread Stephen Hemminger
On Wed, 14 Dec 2016 15:27:58 -0800
Greg KH  wrote:

> On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> > 
> >   
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Saturday, December 10, 2016 7:21 AM
> > > To: Stephen Hemminger 
> > > Cc: Haiyang Zhang ; o...@aepfle.de;
> > > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > > leann.ogasaw...@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 +
> > > > 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 == _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;
> > 

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-14 Thread Greg KH
On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Saturday, December 10, 2016 7:21 AM
> > To: Stephen Hemminger <step...@networkplumber.org>
> > Cc: Haiyang Zhang <haiya...@microsoft.com>; o...@aepfle.de;
> > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@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 +
> > > Haiyang Zhang <haiya...@microsoft.com> 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 == _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.

thanks,

greg k-h


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-14 Thread Greg KH
On Wed, Dec 14, 2016 at 11:18:59PM +, Haiyang Zhang wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Saturday, December 10, 2016 7:21 AM
> > To: Stephen Hemminger 
> > Cc: Haiyang Zhang ; o...@aepfle.de;
> > jasow...@redhat.com; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@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 +
> > > 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 == _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.

thanks,

greg k-h


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-10 Thread Greg KH
On Fri, Dec 09, 2016 at 04:21:48PM -0800, Stephen Hemminger wrote:
> On Fri, 9 Dec 2016 22:35:05 +
> 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 == _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


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-10 Thread Greg KH
On Fri, Dec 09, 2016 at 04:21:48PM -0800, Stephen Hemminger wrote:
> On Fri, 9 Dec 2016 22:35:05 +
> 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 == _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


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 22:35:05 +
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 == _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.

Also the loop to look for serial number in the devices on the
hv_pci bus could be made a separate function and have a short circuit
return (although it probably doesn't matter since there will only
be on e PCI VF device per bus there).


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 22:35:05 +
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 == _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.

Also the loop to look for serial number in the devices on the
hv_pci bus could be made a separate function and have a short circuit
return (although it probably doesn't matter since there will only
be on e PCI VF device per bus there).


RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 5:05 PM
> To: Haiyang Zhang <haiya...@microsoft.com>
> Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 21:53:49 +
> Haiyang Zhang <haiya...@microsoft.com> wrote:
> 
> > > -Original Message-
> > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > Sent: Friday, December 9, 2016 4:45 PM
> > > To: Haiyang Zhang <haiya...@microsoft.com>
> > > Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> > > <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > serial numbers
> > >
> > > On Fri, 9 Dec 2016 21:31:25 +
> > > Haiyang Zhang <haiya...@microsoft.com> wrote:
> > >
> > > > > -Original Message-
> > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > Sent: Friday, December 9, 2016 3:30 PM
> > > > > To: Haiyang Zhang <haiya...@microsoft.com>
> > > > > Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> > > > > <k...@microsoft.com>; o...@aepfle.de; linux-
> ker...@vger.kernel.org;
> > > > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > de...@linuxdriverproject.org;
> > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based
> on
> > > > > serial numbers
> > > > >
> > > > > On Fri, 9 Dec 2016 20:09:49 +
> > > > > Haiyang Zhang <haiya...@microsoft.com> wrote:
> > > > >
> > > > > > > -Original Message-
> > > > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > > > > To: Greg KH <gre...@linuxfoundation.org>
> > > > > > > Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de;
> Haiyang
> > > Zhang
> > > > > > > <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> > > > > > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > > > de...@linuxdriverproject.org;
> > > > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching
> based
> > > on
> > > > > > > serial numbers
> > > > > > >
> > > > > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > > > > Greg KH <gre...@linuxfoundation.org> wrote:
> > > > > > >
> > > > > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan
> wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -Original Message-
> > > > > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > > > > To: KY Srinivasan <k...@microsoft.com>
> > > > > > > > > > Cc: linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org;
> > > > > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang
> > > <haiya...@microsoft.com>
> > > > > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF
> matching
> > > > > based on
> > > > > > > serial
> > > > > > > > > > numbers
> > > > > > > > > >
> > > > > &g

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 5:05 PM
> To: Haiyang Zhang 
> Cc: Greg KH ; KY Srinivasan
> ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 21:53:49 +
> Haiyang Zhang  wrote:
> 
> > > -Original Message-
> > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > Sent: Friday, December 9, 2016 4:45 PM
> > > To: Haiyang Zhang 
> > > Cc: Greg KH ; KY Srinivasan
> > > ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > serial numbers
> > >
> > > On Fri, 9 Dec 2016 21:31:25 +
> > > Haiyang Zhang  wrote:
> > >
> > > > > -Original Message-
> > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > Sent: Friday, December 9, 2016 3:30 PM
> > > > > To: Haiyang Zhang 
> > > > > Cc: Greg KH ; KY Srinivasan
> > > > > ; o...@aepfle.de; linux-
> ker...@vger.kernel.org;
> > > > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > de...@linuxdriverproject.org;
> > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based
> on
> > > > > serial numbers
> > > > >
> > > > > On Fri, 9 Dec 2016 20:09:49 +
> > > > > Haiyang Zhang  wrote:
> > > > >
> > > > > > > -Original Message-
> > > > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > > > > To: Greg KH 
> > > > > > > Cc: KY Srinivasan ; o...@aepfle.de;
> Haiyang
> > > Zhang
> > > > > > > ; linux-kernel@vger.kernel.org;
> > > > > > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > > > de...@linuxdriverproject.org;
> > > > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching
> based
> > > on
> > > > > > > serial numbers
> > > > > > >
> > > > > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > > > > Greg KH  wrote:
> > > > > > >
> > > > > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan
> wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > -Original Message-
> > > > > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > > > > To: KY Srinivasan 
> > > > > > > > > > Cc: linux-kernel@vger.kernel.org;
> > > de...@linuxdriverproject.org;
> > > > > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang
> > > 
> > > > > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF
> matching
> > > > > based on
> > > > > > > serial
> > > > > > > > > > numbers
> > > > > > > > > >
> > > > > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> > > > > > > k...@exchange.microsoft.com
> > > > > > > > > > wrote:
> > > > > > > > > > > From: Haiyang Zhang 
> > > > > > > > > > >
> > > > > > > > > > > We currently use MAC address to match VF and
> synthetic
> > > NICs.
> > > > > > > Hyper-V
> > > > > &

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 1:21 PM
> To: Greg KH <gre...@linuxfoundation.org>
> Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de; Haiyang Zhang
> <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 08:31:22 +0100
> Greg KH <gre...@linuxfoundation.org> wrote:
> 
> > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > To: KY Srinivasan <k...@microsoft.com>
> > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial
> > > > numbers
> > > >
> > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> k...@exchange.microsoft.com
> > > > wrote:
> > > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > > >
> > > > > We currently use MAC address to match VF and synthetic NICs.
> Hyper-V
> > > > > provides a serial number to both devices for this purpose. This
> patch
> > > > > implements the matching based on VF serial numbers. This is the
> way
> > > > > specified by the protocol and more reliable.
> > > > >
> > > > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > > > ---
> > > > >  drivers/net/hyperv/netvsc_drv.c |   55
> > > > ---
> > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > > > b/drivers/net/hyperv/netvsc_drv.c
> > > > > index 9522763..c5778cf 100644
> > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> > > > net_device *netdev)
> > > > >   free_netdev(netdev);
> > > > >  }
> > > > >
> > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > >  {
> > > > >   struct net_device *dev;
> > > > > + struct net_device_context *ndev_ctx;
> > > > >
> > > > >   ASSERT_RTNL();
> > > > >
> > > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct
> net_device
> > > > *netdev)
> > > > >   if (dev->netdev_ops != _ops)
> > > > >   continue;   /* not a netvsc device */
> > > > >
> > > > > - if (ether_addr_equal(mac, dev->perm_addr))
> > > > > + ndev_ctx = netdev_priv(dev);
> > > > > + if (ndev_ctx->vf_serial == vfser)
> > > > >   return dev;
> > > > >   }
> > > > >
> > > > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct
> > > > net_device *netdev)
> > > > >   return NULL;
> > > > >  }
> > > > >
> > > > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > > > +{
> > > > > + struct device *dev;
> > > > > + struct hv_device *hdev;
> > > > > + struct hv_pcibus_device *hbus = NULL;
> > > > > + struct list_head *iter;
> > > > > + struct hv_pci_dev *hpdev;
> > > > > + unsigned long flags;
> > > > > + u32 vfser = 0;
> > > > > + u32 count = 0;
> > > > > +
> > > > > + for (dev = _netdev-&g

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 1:21 PM
> To: Greg KH 
> Cc: KY Srinivasan ; o...@aepfle.de; Haiyang Zhang
> ; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 08:31:22 +0100
> Greg KH  wrote:
> 
> > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > To: KY Srinivasan 
> > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > bjorn.helg...@gmail.com; Haiyang Zhang 
> > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial
> > > > numbers
> > > >
> > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> k...@exchange.microsoft.com
> > > > wrote:
> > > > > From: Haiyang Zhang 
> > > > >
> > > > > We currently use MAC address to match VF and synthetic NICs.
> Hyper-V
> > > > > provides a serial number to both devices for this purpose. This
> patch
> > > > > implements the matching based on VF serial numbers. This is the
> way
> > > > > specified by the protocol and more reliable.
> > > > >
> > > > > Signed-off-by: Haiyang Zhang 
> > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > ---
> > > > >  drivers/net/hyperv/netvsc_drv.c |   55
> > > > ---
> > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > > > b/drivers/net/hyperv/netvsc_drv.c
> > > > > index 9522763..c5778cf 100644
> > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> > > > net_device *netdev)
> > > > >   free_netdev(netdev);
> > > > >  }
> > > > >
> > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > >  {
> > > > >   struct net_device *dev;
> > > > > + struct net_device_context *ndev_ctx;
> > > > >
> > > > >   ASSERT_RTNL();
> > > > >
> > > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct
> net_device
> > > > *netdev)
> > > > >   if (dev->netdev_ops != _ops)
> > > > >   continue;   /* not a netvsc device */
> > > > >
> > > > > - if (ether_addr_equal(mac, dev->perm_addr))
> > > > > + ndev_ctx = netdev_priv(dev);
> > > > > + if (ndev_ctx->vf_serial == vfser)
> > > > >   return dev;
> > > > >   }
> > > > >
> > > > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct
> > > > net_device *netdev)
> > > > >   return NULL;
> > > > >  }
> > > > >
> > > > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > > > +{
> > > > > + struct device *dev;
> > > > > + struct hv_device *hdev;
> > > > > + struct hv_pcibus_device *hbus = NULL;
> > > > > + struct list_head *iter;
> > > > > + struct hv_pci_dev *hpdev;
> > > > > + unsigned long flags;
> > > > > + u32 vfser = 0;
> > > > > + u32 count = 0;
> > > > > +
> > > > > + for (dev = _netdev->dev; dev; dev = dev->parent) {
> > > >
> > > > You are going to walk the whole device tree backwards?  That's
> crazy.
> > > > And foolish.  And racy and broken (what happens if the tree
> changes
> > > > while you do

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 4:45 PM
> To: Haiyang Zhang <haiya...@microsoft.com>
> Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 21:31:25 +
> Haiyang Zhang <haiya...@microsoft.com> wrote:
> 
> > > -Original Message-
> > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > Sent: Friday, December 9, 2016 3:30 PM
> > > To: Haiyang Zhang <haiya...@microsoft.com>
> > > Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> > > <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > serial numbers
> > >
> > > On Fri, 9 Dec 2016 20:09:49 +
> > > Haiyang Zhang <haiya...@microsoft.com> wrote:
> > >
> > > > > -Original Message-
> > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > > To: Greg KH <gre...@linuxfoundation.org>
> > > > > Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de; Haiyang
> Zhang
> > > > > <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> > > > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > de...@linuxdriverproject.org;
> > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based
> on
> > > > > serial numbers
> > > > >
> > > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > > Greg KH <gre...@linuxfoundation.org> wrote:
> > > > >
> > > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -Original Message-----
> > > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > > To: KY Srinivasan <k...@microsoft.com>
> > > > > > > > Cc: linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org;
> > > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang
> <haiya...@microsoft.com>
> > > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching
> > > based on
> > > > > serial
> > > > > > > > numbers
> > > > > > > >
> > > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> > > > > k...@exchange.microsoft.com
> > > > > > > > wrote:
> > > > > > > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > > >
> > > > > > > > > We currently use MAC address to match VF and synthetic
> NICs.
> > > > > Hyper-V
> > > > > > > > > provides a serial number to both devices for this
> purpose.
> > > This
> > > > > patch
> > > > > > > > > implements the matching based on VF serial numbers. This
> is
> > > the
> > > > > way
> > > > > > > > > specified by the protocol and more reliable.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55
> > > > > > > > ---
> > > > > > >

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 4:45 PM
> To: Haiyang Zhang 
> Cc: Greg KH ; KY Srinivasan
> ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 21:31:25 +
> Haiyang Zhang  wrote:
> 
> > > -Original Message-
> > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > Sent: Friday, December 9, 2016 3:30 PM
> > > To: Haiyang Zhang 
> > > Cc: Greg KH ; KY Srinivasan
> > > ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > serial numbers
> > >
> > > On Fri, 9 Dec 2016 20:09:49 +
> > > Haiyang Zhang  wrote:
> > >
> > > > > -Original Message-
> > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > > To: Greg KH 
> > > > > Cc: KY Srinivasan ; o...@aepfle.de; Haiyang
> Zhang
> > > > > ; linux-kernel@vger.kernel.org;
> > > > > bjorn.helg...@gmail.com; a...@canonical.com;
> > > de...@linuxdriverproject.org;
> > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based
> on
> > > > > serial numbers
> > > > >
> > > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > > Greg KH  wrote:
> > > > >
> > > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > > > > > >
> > > > > > >
> > > > > > > > -Original Message-----
> > > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > > To: KY Srinivasan 
> > > > > > > > Cc: linux-kernel@vger.kernel.org;
> de...@linuxdriverproject.org;
> > > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang
> 
> > > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching
> > > based on
> > > > > serial
> > > > > > > > numbers
> > > > > > > >
> > > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> > > > > k...@exchange.microsoft.com
> > > > > > > > wrote:
> > > > > > > > > From: Haiyang Zhang 
> > > > > > > > >
> > > > > > > > > We currently use MAC address to match VF and synthetic
> NICs.
> > > > > Hyper-V
> > > > > > > > > provides a serial number to both devices for this
> purpose.
> > > This
> > > > > patch
> > > > > > > > > implements the matching based on VF serial numbers. This
> is
> > > the
> > > > > way
> > > > > > > > > specified by the protocol and more reliable.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Haiyang Zhang 
> > > > > > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > > > > > ---
> > > > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55
> > > > > > > > ---
> > > > > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > > index 9522763..c5778cf 100644
> > > > > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > > +++ b/drivers/net/hyperv/netvsc_d

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 21:53:49 +
Haiyang Zhang <haiya...@microsoft.com> wrote:

> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Friday, December 9, 2016 4:45 PM
> > To: Haiyang Zhang <haiya...@microsoft.com>
> > Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> > <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> > 
> > On Fri, 9 Dec 2016 21:31:25 +
> > Haiyang Zhang <haiya...@microsoft.com> wrote:
> >   
> > > > -Original Message-
> > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > Sent: Friday, December 9, 2016 3:30 PM
> > > > To: Haiyang Zhang <haiya...@microsoft.com>
> > > > Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> > > > <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com;  
> > de...@linuxdriverproject.org;  
> > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > > serial numbers
> > > >
> > > > On Fri, 9 Dec 2016 20:09:49 +
> > > > Haiyang Zhang <haiya...@microsoft.com> wrote:
> > > >  
> > > > > > -Original Message-
> > > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > > > To: Greg KH <gre...@linuxfoundation.org>
> > > > > > Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de; Haiyang  
> > Zhang  
> > > > > > <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> > > > > > bjorn.helg...@gmail.com; a...@canonical.com;  
> > > > de...@linuxdriverproject.org;  
> > > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based  
> > on  
> > > > > > serial numbers
> > > > > >
> > > > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > > > Greg KH <gre...@linuxfoundation.org> wrote:
> > > > > >  
> > > > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:  
> > > > > > > >
> > > > > > > >  
> > > > > > > > > -Original Message-
> > > > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > > > To: KY Srinivasan <k...@microsoft.com>
> > > > > > > > > Cc: linux-kernel@vger.kernel.org;  
> > de...@linuxdriverproject.org;  
> > > > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang  
> > <haiya...@microsoft.com>  
> > > > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching  
> > > > based on  
> > > > > > serial  
> > > > > > > > > numbers
> > > > > > > > >
> > > > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,  
> > > > > > k...@exchange.microsoft.com  
> > > > > > > > > wrote:  
> > > > > > > > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > > > >
> > > > > > > > > > We currently use MAC address to match VF and synthetic  
> > NICs.  
> > > > > > Hyper-V  
> > > > > > > > > > provides a serial number to both devices for this  
> > purpose.  
> > > > This  
> > > > > > patch  
> > > > > > > > > > implements the matching based on VF serial numbers. This  
> > is  
> > > > the  
> > > > > > way  
> > > > > > > > 

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 21:53:49 +
Haiyang Zhang  wrote:

> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Friday, December 9, 2016 4:45 PM
> > To: Haiyang Zhang 
> > Cc: Greg KH ; KY Srinivasan
> > ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> > 
> > On Fri, 9 Dec 2016 21:31:25 +
> > Haiyang Zhang  wrote:
> >   
> > > > -Original Message-
> > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > Sent: Friday, December 9, 2016 3:30 PM
> > > > To: Haiyang Zhang 
> > > > Cc: Greg KH ; KY Srinivasan
> > > > ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com;  
> > de...@linuxdriverproject.org;  
> > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > > serial numbers
> > > >
> > > > On Fri, 9 Dec 2016 20:09:49 +
> > > > Haiyang Zhang  wrote:
> > > >  
> > > > > > -Original Message-
> > > > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > > > To: Greg KH 
> > > > > > Cc: KY Srinivasan ; o...@aepfle.de; Haiyang  
> > Zhang  
> > > > > > ; linux-kernel@vger.kernel.org;
> > > > > > bjorn.helg...@gmail.com; a...@canonical.com;  
> > > > de...@linuxdriverproject.org;  
> > > > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based  
> > on  
> > > > > > serial numbers
> > > > > >
> > > > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > > > Greg KH  wrote:
> > > > > >  
> > > > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:  
> > > > > > > >
> > > > > > > >  
> > > > > > > > > -Original Message-
> > > > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > > > To: KY Srinivasan 
> > > > > > > > > Cc: linux-kernel@vger.kernel.org;  
> > de...@linuxdriverproject.org;  
> > > > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang  
> >   
> > > > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching  
> > > > based on  
> > > > > > serial  
> > > > > > > > > numbers
> > > > > > > > >
> > > > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,  
> > > > > > k...@exchange.microsoft.com  
> > > > > > > > > wrote:  
> > > > > > > > > > From: Haiyang Zhang 
> > > > > > > > > >
> > > > > > > > > > We currently use MAC address to match VF and synthetic  
> > NICs.  
> > > > > > Hyper-V  
> > > > > > > > > > provides a serial number to both devices for this  
> > purpose.  
> > > > This  
> > > > > > patch  
> > > > > > > > > > implements the matching based on VF serial numbers. This  
> > is  
> > > > the  
> > > > > > way  
> > > > > > > > > > specified by the protocol and more reliable.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Haiyang Zhang 
> > > > > > > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > > > > > > ---
> > > > > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > > > > > > > 

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 3:30 PM
> To: Haiyang Zhang <haiya...@microsoft.com>
> Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 20:09:49 +
> Haiyang Zhang <haiya...@microsoft.com> wrote:
> 
> > > -Original Message-
> > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > Sent: Friday, December 9, 2016 1:21 PM
> > > To: Greg KH <gre...@linuxfoundation.org>
> > > Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de; Haiyang Zhang
> > > <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > serial numbers
> > >
> > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > Greg KH <gre...@linuxfoundation.org> wrote:
> > >
> > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > > > >
> > > > >
> > > > > > -Original Message-
> > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > To: KY Srinivasan <k...@microsoft.com>
> > > > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching
> based on
> > > serial
> > > > > > numbers
> > > > > >
> > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> > > k...@exchange.microsoft.com
> > > > > > wrote:
> > > > > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > >
> > > > > > > We currently use MAC address to match VF and synthetic NICs.
> > > Hyper-V
> > > > > > > provides a serial number to both devices for this purpose.
> This
> > > patch
> > > > > > > implements the matching based on VF serial numbers. This is
> the
> > > way
> > > > > > > specified by the protocol and more reliable.
> > > > > > >
> > > > > > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > > > > > ---
> > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55
> > > > > > ---
> > > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > > > > > b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > index 9522763..c5778cf 100644
> > > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> > > > > > net_device *netdev)
> > > > > > >   free_netdev(netdev);
> > > > > > >  }
> > > > > > >
> > > > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > > > >  {
> > > > > > >   struct net_device *dev;
> > > > > > > + struct net_device_context *ndev_ctx;
> > > > > > >
> > > > > > >   ASSERT_RTNL();
> > > > > > >
> > > > > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct
> > > net_device
> > > > > > *netdev)
&

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Haiyang Zhang


> -Original Message-
> From: Stephen Hemminger [mailto:step...@networkplumber.org]
> Sent: Friday, December 9, 2016 3:30 PM
> To: Haiyang Zhang 
> Cc: Greg KH ; KY Srinivasan
> ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> leann.ogasaw...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> serial numbers
> 
> On Fri, 9 Dec 2016 20:09:49 +
> Haiyang Zhang  wrote:
> 
> > > -Original Message-
> > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > Sent: Friday, December 9, 2016 1:21 PM
> > > To: Greg KH 
> > > Cc: KY Srinivasan ; o...@aepfle.de; Haiyang Zhang
> > > ; linux-kernel@vger.kernel.org;
> > > bjorn.helg...@gmail.com; a...@canonical.com;
> de...@linuxdriverproject.org;
> > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > serial numbers
> > >
> > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > Greg KH  wrote:
> > >
> > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > > > >
> > > > >
> > > > > > -Original Message-
> > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > To: KY Srinivasan 
> > > > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > bjorn.helg...@gmail.com; Haiyang Zhang 
> > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching
> based on
> > > serial
> > > > > > numbers
> > > > > >
> > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,
> > > k...@exchange.microsoft.com
> > > > > > wrote:
> > > > > > > From: Haiyang Zhang 
> > > > > > >
> > > > > > > We currently use MAC address to match VF and synthetic NICs.
> > > Hyper-V
> > > > > > > provides a serial number to both devices for this purpose.
> This
> > > patch
> > > > > > > implements the matching based on VF serial numbers. This is
> the
> > > way
> > > > > > > specified by the protocol and more reliable.
> > > > > > >
> > > > > > > Signed-off-by: Haiyang Zhang 
> > > > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > > > ---
> > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55
> > > > > > ---
> > > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > > > > > b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > index 9522763..c5778cf 100644
> > > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> > > > > > net_device *netdev)
> > > > > > >   free_netdev(netdev);
> > > > > > >  }
> > > > > > >
> > > > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > > > >  {
> > > > > > >   struct net_device *dev;
> > > > > > > + struct net_device_context *ndev_ctx;
> > > > > > >
> > > > > > >   ASSERT_RTNL();
> > > > > > >
> > > > > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct
> > > net_device
> > > > > > *netdev)
> > > > > > >   if (dev->netdev_ops != _ops)
> > > > > > >   continue;   /* not a netvsc device */
> > > > > > >
> > > > > > > - if (ether_addr_equal(mac, dev->perm_addr))
> > > > > > > + ndev_ctx = netdev_priv(dev);
> > > &

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 21:31:25 +
Haiyang Zhang <haiya...@microsoft.com> wrote:

> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Friday, December 9, 2016 3:30 PM
> > To: Haiyang Zhang <haiya...@microsoft.com>
> > Cc: Greg KH <gre...@linuxfoundation.org>; KY Srinivasan
> > <k...@microsoft.com>; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> > 
> > On Fri, 9 Dec 2016 20:09:49 +
> > Haiyang Zhang <haiya...@microsoft.com> wrote:
> >   
> > > > -Original Message-
> > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > To: Greg KH <gre...@linuxfoundation.org>
> > > > Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de; Haiyang Zhang
> > > > <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com;  
> > de...@linuxdriverproject.org;  
> > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > > serial numbers
> > > >
> > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > Greg KH <gre...@linuxfoundation.org> wrote:
> > > >  
> > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:  
> > > > > >
> > > > > >  
> > > > > > > -Original Message-
> > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > To: KY Srinivasan <k...@microsoft.com>
> > > > > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching  
> > based on  
> > > > serial  
> > > > > > > numbers
> > > > > > >
> > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,  
> > > > k...@exchange.microsoft.com  
> > > > > > > wrote:  
> > > > > > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > >
> > > > > > > > We currently use MAC address to match VF and synthetic NICs.  
> > > > Hyper-V  
> > > > > > > > provides a serial number to both devices for this purpose.  
> > This  
> > > > patch  
> > > > > > > > implements the matching based on VF serial numbers. This is  
> > the  
> > > > way  
> > > > > > > > specified by the protocol and more reliable.
> > > > > > > >
> > > > > > > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > > > > > > ---
> > > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > > > > > ---  
> > > > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c  
> > > > > > > b/drivers/net/hyperv/netvsc_drv.c  
> > > > > > > > index 9522763..c5778cf 100644
> > > > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct  
> > > > > > > net_device *netdev)  
> > > > > > > > free_netdev(netdev);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > &

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 21:31:25 +
Haiyang Zhang  wrote:

> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Friday, December 9, 2016 3:30 PM
> > To: Haiyang Zhang 
> > Cc: Greg KH ; KY Srinivasan
> > ; o...@aepfle.de; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> > 
> > On Fri, 9 Dec 2016 20:09:49 +
> > Haiyang Zhang  wrote:
> >   
> > > > -Original Message-
> > > > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > > > Sent: Friday, December 9, 2016 1:21 PM
> > > > To: Greg KH 
> > > > Cc: KY Srinivasan ; o...@aepfle.de; Haiyang Zhang
> > > > ; linux-kernel@vger.kernel.org;
> > > > bjorn.helg...@gmail.com; a...@canonical.com;  
> > de...@linuxdriverproject.org;  
> > > > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > > > serial numbers
> > > >
> > > > On Fri, 9 Dec 2016 08:31:22 +0100
> > > > Greg KH  wrote:
> > > >  
> > > > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:  
> > > > > >
> > > > > >  
> > > > > > > -Original Message-
> > > > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > > > To: KY Srinivasan 
> > > > > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > > > bjorn.helg...@gmail.com; Haiyang Zhang 
> > > > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching  
> > based on  
> > > > serial  
> > > > > > > numbers
> > > > > > >
> > > > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,  
> > > > k...@exchange.microsoft.com  
> > > > > > > wrote:  
> > > > > > > > From: Haiyang Zhang 
> > > > > > > >
> > > > > > > > We currently use MAC address to match VF and synthetic NICs.  
> > > > Hyper-V  
> > > > > > > > provides a serial number to both devices for this purpose.  
> > This  
> > > > patch  
> > > > > > > > implements the matching based on VF serial numbers. This is  
> > the  
> > > > way  
> > > > > > > > specified by the protocol and more reliable.
> > > > > > > >
> > > > > > > > Signed-off-by: Haiyang Zhang 
> > > > > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > > > > ---
> > > > > > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > > > > > ---  
> > > > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c  
> > > > > > > b/drivers/net/hyperv/netvsc_drv.c  
> > > > > > > > index 9522763..c5778cf 100644
> > > > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct  
> > > > > > > net_device *netdev)  
> > > > > > > > free_netdev(netdev);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > > > > >  {
> > > > > > > > struct net_device *dev;
> > > > > > > > +   struct net_device_context *ndev_ctx;
> > > > > > > >
> > > > > > > > ASSERT_RTNL();
> > > > > > >

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 20:09:49 +
Haiyang Zhang <haiya...@microsoft.com> wrote:

> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Friday, December 9, 2016 1:21 PM
> > To: Greg KH <gre...@linuxfoundation.org>
> > Cc: KY Srinivasan <k...@microsoft.com>; o...@aepfle.de; Haiyang Zhang
> > <haiya...@microsoft.com>; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> > 
> > On Fri, 9 Dec 2016 08:31:22 +0100
> > Greg KH <gre...@linuxfoundation.org> wrote:
> >   
> > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:  
> > > >
> > > >  
> > > > > -Original Message-
> > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > To: KY Srinivasan <k...@microsoft.com>
> > > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on  
> > serial  
> > > > > numbers
> > > > >
> > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,  
> > k...@exchange.microsoft.com  
> > > > > wrote:  
> > > > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > > > >
> > > > > > We currently use MAC address to match VF and synthetic NICs.  
> > Hyper-V  
> > > > > > provides a serial number to both devices for this purpose. This  
> > patch  
> > > > > > implements the matching based on VF serial numbers. This is the  
> > way  
> > > > > > specified by the protocol and more reliable.
> > > > > >
> > > > > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > > > > ---
> > > > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > > > ---  
> > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c  
> > > > > b/drivers/net/hyperv/netvsc_drv.c  
> > > > > > index 9522763..c5778cf 100644
> > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct  
> > > > > net_device *netdev)  
> > > > > > free_netdev(netdev);
> > > > > >  }
> > > > > >
> > > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > > >  {
> > > > > > struct net_device *dev;
> > > > > > +   struct net_device_context *ndev_ctx;
> > > > > >
> > > > > > ASSERT_RTNL();
> > > > > >
> > > > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct  
> > net_device  
> > > > > *netdev)  
> > > > > > if (dev->netdev_ops != _ops)
> > > > > > continue;   /* not a netvsc device */
> > > > > >
> > > > > > -   if (ether_addr_equal(mac, dev->perm_addr))
> > > > > > +   ndev_ctx = netdev_priv(dev);
> > > > > > +   if (ndev_ctx->vf_serial == vfser)
> > > > > > return dev;
> > > > > > }
> > > > > >
> > > > > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct  
> > > > > net_device *netdev)  
> > > > > > return NULL;
> > > > > >  }
> > > > > >
> > > > > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
>

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 20:09:49 +
Haiyang Zhang  wrote:

> > -Original Message-
> > From: Stephen Hemminger [mailto:step...@networkplumber.org]
> > Sent: Friday, December 9, 2016 1:21 PM
> > To: Greg KH 
> > Cc: KY Srinivasan ; o...@aepfle.de; Haiyang Zhang
> > ; linux-kernel@vger.kernel.org;
> > bjorn.helg...@gmail.com; a...@canonical.com; de...@linuxdriverproject.org;
> > leann.ogasaw...@canonical.com; jasow...@redhat.com
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on
> > serial numbers
> > 
> > On Fri, 9 Dec 2016 08:31:22 +0100
> > Greg KH  wrote:
> >   
> > > On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:  
> > > >
> > > >  
> > > > > -Original Message-
> > > > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > > > Sent: Thursday, December 8, 2016 7:56 AM
> > > > > To: KY Srinivasan 
> > > > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > > > bjorn.helg...@gmail.com; Haiyang Zhang 
> > > > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on  
> > serial  
> > > > > numbers
> > > > >
> > > > > On Thu, Dec 08, 2016 at 12:33:43AM -0800,  
> > k...@exchange.microsoft.com  
> > > > > wrote:  
> > > > > > From: Haiyang Zhang 
> > > > > >
> > > > > > We currently use MAC address to match VF and synthetic NICs.  
> > Hyper-V  
> > > > > > provides a serial number to both devices for this purpose. This  
> > patch  
> > > > > > implements the matching based on VF serial numbers. This is the  
> > way  
> > > > > > specified by the protocol and more reliable.
> > > > > >
> > > > > > Signed-off-by: Haiyang Zhang 
> > > > > > Signed-off-by: K. Y. Srinivasan 
> > > > > > ---
> > > > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > > > ---  
> > > > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/hyperv/netvsc_drv.c  
> > > > > b/drivers/net/hyperv/netvsc_drv.c  
> > > > > > index 9522763..c5778cf 100644
> > > > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct  
> > > > > net_device *netdev)  
> > > > > > free_netdev(netdev);
> > > > > >  }
> > > > > >
> > > > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > > > >  {
> > > > > > struct net_device *dev;
> > > > > > +   struct net_device_context *ndev_ctx;
> > > > > >
> > > > > > ASSERT_RTNL();
> > > > > >
> > > > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct  
> > net_device  
> > > > > *netdev)  
> > > > > > if (dev->netdev_ops != _ops)
> > > > > > continue;   /* not a netvsc device */
> > > > > >
> > > > > > -   if (ether_addr_equal(mac, dev->perm_addr))
> > > > > > +   ndev_ctx = netdev_priv(dev);
> > > > > > +   if (ndev_ctx->vf_serial == vfser)
> > > > > > return dev;
> > > > > > }
> > > > > >
> > > > > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct  
> > > > > net_device *netdev)  
> > > > > > return NULL;
> > > > > >  }
> > > > > >
> > > > > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > > > > +{
> > > > > > +   struct device *dev;
> > > > > > +   struct hv_device *hdev;
> > > > > > +   struct hv_pcibus_device *hbus = NULL;
> > > > > > +   struct list_head *iter;
> > > > > &

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 08:31:22 +0100
Greg KH <gre...@linuxfoundation.org> wrote:

> On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > 
> >   
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Thursday, December 8, 2016 7:56 AM
> > > To: KY Srinivasan <k...@microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> > > numbers
> > > 
> > > On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com
> > > wrote:  
> > > > From: Haiyang Zhang <haiya...@microsoft.com>
> > > >
> > > > We currently use MAC address to match VF and synthetic NICs. Hyper-V
> > > > provides a serial number to both devices for this purpose. This patch
> > > > implements the matching based on VF serial numbers. This is the way
> > > > specified by the protocol and more reliable.
> > > >
> > > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > > ---
> > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > ---  
> > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/hyperv/netvsc_drv.c  
> > > b/drivers/net/hyperv/netvsc_drv.c  
> > > > index 9522763..c5778cf 100644
> > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct  
> > > net_device *netdev)  
> > > > free_netdev(netdev);
> > > >  }
> > > >
> > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > >  {
> > > > struct net_device *dev;
> > > > +   struct net_device_context *ndev_ctx;
> > > >
> > > > ASSERT_RTNL();
> > > >
> > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device  
> > > *netdev)  
> > > > if (dev->netdev_ops != _ops)
> > > > continue;   /* not a netvsc device */
> > > >
> > > > -   if (ether_addr_equal(mac, dev->perm_addr))
> > > > +   ndev_ctx = netdev_priv(dev);
> > > > +   if (ndev_ctx->vf_serial == vfser)
> > > > return dev;
> > > > }
> > > >
> > > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct  
> > > net_device *netdev)  
> > > > return NULL;
> > > >  }
> > > >
> > > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > > +{
> > > > +   struct device *dev;
> > > > +   struct hv_device *hdev;
> > > > +   struct hv_pcibus_device *hbus = NULL;
> > > > +   struct list_head *iter;
> > > > +   struct hv_pci_dev *hpdev;
> > > > +   unsigned long flags;
> > > > +   u32 vfser = 0;
> > > > +   u32 count = 0;
> > > > +
> > > > +   for (dev = _netdev->dev; dev; dev = dev->parent) {  
> > > 
> > > You are going to walk the whole device tree backwards?  That's crazy.
> > > And foolish.  And racy and broken (what happens if the tree changes
> > > while you do this?)  Where is the lock being grabbed while this happens?
> > > What about reference counts?  Do you see other drivers ever doing this
> > > (if you do, point them out and I'll go yell at them too...)  
> > 
> > Greg,
> > 
> > We are registering for netdev events. Coming into this function, the caller
> > guarantees that the list of netdevs does not change - we assert this on 
> > entry:
> > ASSERT_RTNL(). We are only walking up the device tree for the netdevs whose
> > state change is being notified to us - the device tree being walked here is 
> > limited to
> > netdevs under question.   
> 
> But 

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-09 Thread Stephen Hemminger
On Fri, 9 Dec 2016 08:31:22 +0100
Greg KH  wrote:

> On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> > 
> >   
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Thursday, December 8, 2016 7:56 AM
> > > To: KY Srinivasan 
> > > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > > bjorn.helg...@gmail.com; Haiyang Zhang 
> > > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> > > numbers
> > > 
> > > On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com
> > > wrote:  
> > > > From: Haiyang Zhang 
> > > >
> > > > We currently use MAC address to match VF and synthetic NICs. Hyper-V
> > > > provides a serial number to both devices for this purpose. This patch
> > > > implements the matching based on VF serial numbers. This is the way
> > > > specified by the protocol and more reliable.
> > > >
> > > > Signed-off-by: Haiyang Zhang 
> > > > Signed-off-by: K. Y. Srinivasan 
> > > > ---
> > > >  drivers/net/hyperv/netvsc_drv.c |   55  
> > > ---  
> > > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/hyperv/netvsc_drv.c  
> > > b/drivers/net/hyperv/netvsc_drv.c  
> > > > index 9522763..c5778cf 100644
> > > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct  
> > > net_device *netdev)  
> > > > free_netdev(netdev);
> > > >  }
> > > >
> > > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > > >  {
> > > > struct net_device *dev;
> > > > +   struct net_device_context *ndev_ctx;
> > > >
> > > > ASSERT_RTNL();
> > > >
> > > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device  
> > > *netdev)  
> > > > if (dev->netdev_ops != _ops)
> > > > continue;   /* not a netvsc device */
> > > >
> > > > -   if (ether_addr_equal(mac, dev->perm_addr))
> > > > +   ndev_ctx = netdev_priv(dev);
> > > > +   if (ndev_ctx->vf_serial == vfser)
> > > > return dev;
> > > > }
> > > >
> > > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct  
> > > net_device *netdev)  
> > > > return NULL;
> > > >  }
> > > >
> > > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > > +{
> > > > +   struct device *dev;
> > > > +   struct hv_device *hdev;
> > > > +   struct hv_pcibus_device *hbus = NULL;
> > > > +   struct list_head *iter;
> > > > +   struct hv_pci_dev *hpdev;
> > > > +   unsigned long flags;
> > > > +   u32 vfser = 0;
> > > > +   u32 count = 0;
> > > > +
> > > > +   for (dev = _netdev->dev; dev; dev = dev->parent) {  
> > > 
> > > You are going to walk the whole device tree backwards?  That's crazy.
> > > And foolish.  And racy and broken (what happens if the tree changes
> > > while you do this?)  Where is the lock being grabbed while this happens?
> > > What about reference counts?  Do you see other drivers ever doing this
> > > (if you do, point them out and I'll go yell at them too...)  
> > 
> > Greg,
> > 
> > We are registering for netdev events. Coming into this function, the caller
> > guarantees that the list of netdevs does not change - we assert this on 
> > entry:
> > ASSERT_RTNL(). We are only walking up the device tree for the netdevs whose
> > state change is being notified to us - the device tree being walked here is 
> > limited to
> > netdevs under question.   
> 
> But a netdev is a child of some type of "real" device, and you are now
> walking the tree of all devices up to the "root" parent device, which
>

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-08 Thread Greg KH
On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Thursday, December 8, 2016 7:56 AM
> > To: KY Srinivasan <k...@microsoft.com>
> > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> > numbers
> > 
> > On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com
> > wrote:
> > > From: Haiyang Zhang <haiya...@microsoft.com>
> > >
> > > We currently use MAC address to match VF and synthetic NICs. Hyper-V
> > > provides a serial number to both devices for this purpose. This patch
> > > implements the matching based on VF serial numbers. This is the way
> > > specified by the protocol and more reliable.
> > >
> > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > > ---
> > >  drivers/net/hyperv/netvsc_drv.c |   55
> > ---
> > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c
> > > index 9522763..c5778cf 100644
> > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> > net_device *netdev)
> > >   free_netdev(netdev);
> > >  }
> > >
> > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > >  {
> > >   struct net_device *dev;
> > > + struct net_device_context *ndev_ctx;
> > >
> > >   ASSERT_RTNL();
> > >
> > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device
> > *netdev)
> > >   if (dev->netdev_ops != _ops)
> > >   continue;   /* not a netvsc device */
> > >
> > > - if (ether_addr_equal(mac, dev->perm_addr))
> > > + ndev_ctx = netdev_priv(dev);
> > > + if (ndev_ctx->vf_serial == vfser)
> > >   return dev;
> > >   }
> > >
> > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct
> > net_device *netdev)
> > >   return NULL;
> > >  }
> > >
> > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > +{
> > > + struct device *dev;
> > > + struct hv_device *hdev;
> > > + struct hv_pcibus_device *hbus = NULL;
> > > + struct list_head *iter;
> > > + struct hv_pci_dev *hpdev;
> > > + unsigned long flags;
> > > + u32 vfser = 0;
> > > + u32 count = 0;
> > > +
> > > + for (dev = _netdev->dev; dev; dev = dev->parent) {
> > 
> > You are going to walk the whole device tree backwards?  That's crazy.
> > And foolish.  And racy and broken (what happens if the tree changes
> > while you do this?)  Where is the lock being grabbed while this happens?
> > What about reference counts?  Do you see other drivers ever doing this
> > (if you do, point them out and I'll go yell at them too...)
> 
> Greg,
> 
> We are registering for netdev events. Coming into this function, the caller
> guarantees that the list of netdevs does not change - we assert this on entry:
> ASSERT_RTNL(). We are only walking up the device tree for the netdevs whose
> state change is being notified to us - the device tree being walked here is 
> limited to
> netdevs under question. 

But a netdev is a child of some type of "real" device, and you are now
walking the tree of all devices up to the "root" parent device, which
means you will hit PCI bridges, USB controllers, and all sorts of fun
things if you are a child of those types of devices.

And can't you tell if the netdev for this event, really is "your"
netdev?  Or are you getting called this for "all" netdevs?  Sorry, I
don't know this api, any pointers to it would be appreciated.

> We have a reference to the device and we know the device is not going away. 
> Is it not
> safe to dereference the parent pointer - after all the child has taken a 
> reference on
> the parent as

Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-08 Thread Greg KH
On Fri, Dec 09, 2016 at 12:05:53AM +, KY Srinivasan wrote:
> 
> 
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Thursday, December 8, 2016 7:56 AM
> > To: KY Srinivasan 
> > Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> > jasow...@redhat.com; leann.ogasaw...@canonical.com;
> > bjorn.helg...@gmail.com; Haiyang Zhang 
> > Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> > numbers
> > 
> > On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com
> > wrote:
> > > From: Haiyang Zhang 
> > >
> > > We currently use MAC address to match VF and synthetic NICs. Hyper-V
> > > provides a serial number to both devices for this purpose. This patch
> > > implements the matching based on VF serial numbers. This is the way
> > > specified by the protocol and more reliable.
> > >
> > > Signed-off-by: Haiyang Zhang 
> > > Signed-off-by: K. Y. Srinivasan 
> > > ---
> > >  drivers/net/hyperv/netvsc_drv.c |   55
> > ---
> > >  1 files changed, 51 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c
> > > index 9522763..c5778cf 100644
> > > --- a/drivers/net/hyperv/netvsc_drv.c
> > > +++ b/drivers/net/hyperv/netvsc_drv.c
> > > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> > net_device *netdev)
> > >   free_netdev(netdev);
> > >  }
> > >
> > > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> > >  {
> > >   struct net_device *dev;
> > > + struct net_device_context *ndev_ctx;
> > >
> > >   ASSERT_RTNL();
> > >
> > > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device
> > *netdev)
> > >   if (dev->netdev_ops != _ops)
> > >   continue;   /* not a netvsc device */
> > >
> > > - if (ether_addr_equal(mac, dev->perm_addr))
> > > + ndev_ctx = netdev_priv(dev);
> > > + if (ndev_ctx->vf_serial == vfser)
> > >   return dev;
> > >   }
> > >
> > > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct
> > net_device *netdev)
> > >   return NULL;
> > >  }
> > >
> > > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > > +{
> > > + struct device *dev;
> > > + struct hv_device *hdev;
> > > + struct hv_pcibus_device *hbus = NULL;
> > > + struct list_head *iter;
> > > + struct hv_pci_dev *hpdev;
> > > + unsigned long flags;
> > > + u32 vfser = 0;
> > > + u32 count = 0;
> > > +
> > > + for (dev = _netdev->dev; dev; dev = dev->parent) {
> > 
> > You are going to walk the whole device tree backwards?  That's crazy.
> > And foolish.  And racy and broken (what happens if the tree changes
> > while you do this?)  Where is the lock being grabbed while this happens?
> > What about reference counts?  Do you see other drivers ever doing this
> > (if you do, point them out and I'll go yell at them too...)
> 
> Greg,
> 
> We are registering for netdev events. Coming into this function, the caller
> guarantees that the list of netdevs does not change - we assert this on entry:
> ASSERT_RTNL(). We are only walking up the device tree for the netdevs whose
> state change is being notified to us - the device tree being walked here is 
> limited to
> netdevs under question. 

But a netdev is a child of some type of "real" device, and you are now
walking the tree of all devices up to the "root" parent device, which
means you will hit PCI bridges, USB controllers, and all sorts of fun
things if you are a child of those types of devices.

And can't you tell if the netdev for this event, really is "your"
netdev?  Or are you getting called this for "all" netdevs?  Sorry, I
don't know this api, any pointers to it would be appreciated.

> We have a reference to the device and we know the device is not going away. 
> Is it not
> safe to dereference the parent pointer - after all the child has taken a 
> reference on
> the parent as part of  device_add() call.

It might be, and might not be.  There's a reason you don't see this
pattern anywhere in the kernel because of th

RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-08 Thread KY Srinivasan


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, December 8, 2016 7:56 AM
> To: KY Srinivasan <k...@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> jasow...@redhat.com; leann.ogasaw...@canonical.com;
> bjorn.helg...@gmail.com; Haiyang Zhang <haiya...@microsoft.com>
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> numbers
> 
> On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com
> wrote:
> > From: Haiyang Zhang <haiya...@microsoft.com>
> >
> > We currently use MAC address to match VF and synthetic NICs. Hyper-V
> > provides a serial number to both devices for this purpose. This patch
> > implements the matching based on VF serial numbers. This is the way
> > specified by the protocol and more reliable.
> >
> > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> > Signed-off-by: K. Y. Srinivasan <k...@microsoft.com>
> > ---
> >  drivers/net/hyperv/netvsc_drv.c |   55
> ---
> >  1 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index 9522763..c5778cf 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> net_device *netdev)
> > free_netdev(netdev);
> >  }
> >
> > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> >  {
> > struct net_device *dev;
> > +   struct net_device_context *ndev_ctx;
> >
> > ASSERT_RTNL();
> >
> > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device
> *netdev)
> > if (dev->netdev_ops != _ops)
> > continue;   /* not a netvsc device */
> >
> > -   if (ether_addr_equal(mac, dev->perm_addr))
> > +   ndev_ctx = netdev_priv(dev);
> > +   if (ndev_ctx->vf_serial == vfser)
> > return dev;
> > }
> >
> > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct
> net_device *netdev)
> > return NULL;
> >  }
> >
> > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > +{
> > +   struct device *dev;
> > +   struct hv_device *hdev;
> > +   struct hv_pcibus_device *hbus = NULL;
> > +   struct list_head *iter;
> > +   struct hv_pci_dev *hpdev;
> > +   unsigned long flags;
> > +   u32 vfser = 0;
> > +   u32 count = 0;
> > +
> > +   for (dev = _netdev->dev; dev; dev = dev->parent) {
> 
> You are going to walk the whole device tree backwards?  That's crazy.
> And foolish.  And racy and broken (what happens if the tree changes
> while you do this?)  Where is the lock being grabbed while this happens?
> What about reference counts?  Do you see other drivers ever doing this
> (if you do, point them out and I'll go yell at them too...)

Greg,

We are registering for netdev events. Coming into this function, the caller
guarantees that the list of netdevs does not change - we assert this on entry:
ASSERT_RTNL(). We are only walking up the device tree for the netdevs whose
state change is being notified to us - the device tree being walked here is 
limited to
netdevs under question. 

We have a reference to the device and we know the device is not going away. Is 
it not
safe to dereference the parent pointer - after all the child has taken a 
reference on
the parent as part of  device_add() call.
 
> 
> > +   if (!dev_is_vmbus(dev))
> > +   continue;
> 
> Ick.
> 
> Why isn't your parent pointer a vmbus device all the time?  How could
> you get burried down in the device hierarchy when you are the driver for
> a specific bus type in the first place?  How could this function ever be
> called for a device that is NOT of this type?

We get notified when state changes on any of the netdev devices in the system.
Not all netdevs in the system belong to vmbus. Consider for instance the 
emulated NIC that can be configured. This is an emulated PCI NIC. We are only
interested in netdevs that correspond to the VF instance that we are interested 
in.

Regards,

K. Y


RE: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-08 Thread KY Srinivasan


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, December 8, 2016 7:56 AM
> To: KY Srinivasan 
> Cc: linux-kernel@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> jasow...@redhat.com; leann.ogasaw...@canonical.com;
> bjorn.helg...@gmail.com; Haiyang Zhang 
> Subject: Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial
> numbers
> 
> On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com
> wrote:
> > From: Haiyang Zhang 
> >
> > We currently use MAC address to match VF and synthetic NICs. Hyper-V
> > provides a serial number to both devices for this purpose. This patch
> > implements the matching based on VF serial numbers. This is the way
> > specified by the protocol and more reliable.
> >
> > Signed-off-by: Haiyang Zhang 
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> >  drivers/net/hyperv/netvsc_drv.c |   55
> ---
> >  1 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> > index 9522763..c5778cf 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct
> net_device *netdev)
> > free_netdev(netdev);
> >  }
> >
> > -static struct net_device *get_netvsc_bymac(const u8 *mac)
> > +static struct net_device *get_netvsc_byvfser(u32 vfser)
> >  {
> > struct net_device *dev;
> > +   struct net_device_context *ndev_ctx;
> >
> > ASSERT_RTNL();
> >
> > @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device
> *netdev)
> > if (dev->netdev_ops != _ops)
> > continue;   /* not a netvsc device */
> >
> > -   if (ether_addr_equal(mac, dev->perm_addr))
> > +   ndev_ctx = netdev_priv(dev);
> > +   if (ndev_ctx->vf_serial == vfser)
> > return dev;
> > }
> >
> > @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct
> net_device *netdev)
> > return NULL;
> >  }
> >
> > +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> > +{
> > +   struct device *dev;
> > +   struct hv_device *hdev;
> > +   struct hv_pcibus_device *hbus = NULL;
> > +   struct list_head *iter;
> > +   struct hv_pci_dev *hpdev;
> > +   unsigned long flags;
> > +   u32 vfser = 0;
> > +   u32 count = 0;
> > +
> > +   for (dev = _netdev->dev; dev; dev = dev->parent) {
> 
> You are going to walk the whole device tree backwards?  That's crazy.
> And foolish.  And racy and broken (what happens if the tree changes
> while you do this?)  Where is the lock being grabbed while this happens?
> What about reference counts?  Do you see other drivers ever doing this
> (if you do, point them out and I'll go yell at them too...)

Greg,

We are registering for netdev events. Coming into this function, the caller
guarantees that the list of netdevs does not change - we assert this on entry:
ASSERT_RTNL(). We are only walking up the device tree for the netdevs whose
state change is being notified to us - the device tree being walked here is 
limited to
netdevs under question. 

We have a reference to the device and we know the device is not going away. Is 
it not
safe to dereference the parent pointer - after all the child has taken a 
reference on
the parent as part of  device_add() call.
 
> 
> > +   if (!dev_is_vmbus(dev))
> > +   continue;
> 
> Ick.
> 
> Why isn't your parent pointer a vmbus device all the time?  How could
> you get burried down in the device hierarchy when you are the driver for
> a specific bus type in the first place?  How could this function ever be
> called for a device that is NOT of this type?

We get notified when state changes on any of the netdev devices in the system.
Not all netdevs in the system belong to vmbus. Consider for instance the 
emulated NIC that can be configured. This is an emulated PCI NIC. We are only
interested in netdevs that correspond to the VF instance that we are interested 
in.

Regards,

K. Y


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-08 Thread Greg KH
On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com wrote:
> From: Haiyang Zhang 
> 
> We currently use MAC address to match VF and synthetic NICs. Hyper-V
> provides a serial number to both devices for this purpose. This patch
> implements the matching based on VF serial numbers. This is the way
> specified by the protocol and more reliable.
> 
> Signed-off-by: Haiyang Zhang 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  drivers/net/hyperv/netvsc_drv.c |   55 
> ---
>  1 files changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 9522763..c5778cf 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct net_device 
> *netdev)
>   free_netdev(netdev);
>  }
>  
> -static struct net_device *get_netvsc_bymac(const u8 *mac)
> +static struct net_device *get_netvsc_byvfser(u32 vfser)
>  {
>   struct net_device *dev;
> + struct net_device_context *ndev_ctx;
>  
>   ASSERT_RTNL();
>  
> @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device 
> *netdev)
>   if (dev->netdev_ops != _ops)
>   continue;   /* not a netvsc device */
>  
> - if (ether_addr_equal(mac, dev->perm_addr))
> + ndev_ctx = netdev_priv(dev);
> + if (ndev_ctx->vf_serial == vfser)
>   return dev;
>   }
>  
> @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct net_device 
> *netdev)
>   return NULL;
>  }
>  
> +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> +{
> + struct device *dev;
> + struct hv_device *hdev;
> + struct hv_pcibus_device *hbus = NULL;
> + struct list_head *iter;
> + struct hv_pci_dev *hpdev;
> + unsigned long flags;
> + u32 vfser = 0;
> + u32 count = 0;
> +
> + for (dev = _netdev->dev; dev; dev = dev->parent) {

You are going to walk the whole device tree backwards?  That's crazy.
And foolish.  And racy and broken (what happens if the tree changes
while you do this?)  Where is the lock being grabbed while this happens?
What about reference counts?  Do you see other drivers ever doing this
(if you do, point them out and I'll go yell at them too...)

> + if (!dev_is_vmbus(dev))
> + continue;

Ick.

Why isn't your parent pointer a vmbus device all the time?  How could
you get burried down in the device hierarchy when you are the driver for
a specific bus type in the first place?  How could this function ever be
called for a device that is NOT of this type?

thanks,

greg k-h


Re: [PATCH 3/3] hv_netvsc: Implement VF matching based on serial numbers

2016-12-08 Thread Greg KH
On Thu, Dec 08, 2016 at 12:33:43AM -0800, k...@exchange.microsoft.com wrote:
> From: Haiyang Zhang 
> 
> We currently use MAC address to match VF and synthetic NICs. Hyper-V
> provides a serial number to both devices for this purpose. This patch
> implements the matching based on VF serial numbers. This is the way
> specified by the protocol and more reliable.
> 
> Signed-off-by: Haiyang Zhang 
> Signed-off-by: K. Y. Srinivasan 
> ---
>  drivers/net/hyperv/netvsc_drv.c |   55 
> ---
>  1 files changed, 51 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 9522763..c5778cf 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -1165,9 +1165,10 @@ static void netvsc_free_netdev(struct net_device 
> *netdev)
>   free_netdev(netdev);
>  }
>  
> -static struct net_device *get_netvsc_bymac(const u8 *mac)
> +static struct net_device *get_netvsc_byvfser(u32 vfser)
>  {
>   struct net_device *dev;
> + struct net_device_context *ndev_ctx;
>  
>   ASSERT_RTNL();
>  
> @@ -1175,7 +1176,8 @@ static void netvsc_free_netdev(struct net_device 
> *netdev)
>   if (dev->netdev_ops != _ops)
>   continue;   /* not a netvsc device */
>  
> - if (ether_addr_equal(mac, dev->perm_addr))
> + ndev_ctx = netdev_priv(dev);
> + if (ndev_ctx->vf_serial == vfser)
>   return dev;
>   }
>  
> @@ -1205,21 +1207,66 @@ static void netvsc_free_netdev(struct net_device 
> *netdev)
>   return NULL;
>  }
>  
> +static u32 netvsc_get_vfser(struct net_device *vf_netdev)
> +{
> + struct device *dev;
> + struct hv_device *hdev;
> + struct hv_pcibus_device *hbus = NULL;
> + struct list_head *iter;
> + struct hv_pci_dev *hpdev;
> + unsigned long flags;
> + u32 vfser = 0;
> + u32 count = 0;
> +
> + for (dev = _netdev->dev; dev; dev = dev->parent) {

You are going to walk the whole device tree backwards?  That's crazy.
And foolish.  And racy and broken (what happens if the tree changes
while you do this?)  Where is the lock being grabbed while this happens?
What about reference counts?  Do you see other drivers ever doing this
(if you do, point them out and I'll go yell at them too...)

> + if (!dev_is_vmbus(dev))
> + continue;

Ick.

Why isn't your parent pointer a vmbus device all the time?  How could
you get burried down in the device hierarchy when you are the driver for
a specific bus type in the first place?  How could this function ever be
called for a device that is NOT of this type?

thanks,

greg k-h