[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Siwei Liu]

On Thu, Apr 26, 2018 at 4:42 PM, Michael S. Tsirkin  wrote:
> On Thu, Apr 26, 2018 at 03:14:46PM -0700, Siwei Liu wrote:
>> On Wed, Apr 25, 2018 at 7:28 PM, Michael S. Tsirkin  wrote:
>> > On Wed, Apr 25, 2018 at 03:57:57PM -0700, Siwei Liu wrote:
>> >> On Wed, Apr 25, 2018 at 3:22 PM, Michael S. Tsirkin  
>> >> wrote:
>> >> > On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
>> >> >> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  
>> >> >> wrote:
>> >> >> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> >> >> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin 
>> >> >> >>  wrote:
>> >> >> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> >> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> >> >> >> "Michael S. Tsirkin"  wrote:
>> >> >> >> >>
>> >> >> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger 
>> >> >> >> >> > wrote:
>> >> >> >> >> > > > >
>> >> >> >> >> > > > >I will NAK patches to change to common code for netvsc 
>> >> >> >> >> > > > >especially the
>> >> >> >> >> > > > >three device model.  MS worked hard with distro vendors 
>> >> >> >> >> > > > >to support transparent
>> >> >> >> >> > > > >mode, ans we really can't have a new model; or do 
>> >> >> >> >> > > > >backport.
>> >> >> >> >> > > > >
>> >> >> >> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> >> >> >> > > >
>> >> >> >> >> > > > Sorry, but nobody here cares about dpdk or other similar 
>> >> >> >> >> > > > oddities.
>> >> >> >> >> > >
>> >> >> >> >> > > The network device model is a userspace API, and DPDK is a 
>> >> >> >> >> > > userspace application.
>> >> >> >> >> >
>> >> >> >> >> > It is userspace but are you sure dpdk is actually poking at 
>> >> >> >> >> > netdevs?
>> >> >> >> >> > AFAIK it's normally banging device registers directly.
>> >> >> >> >> >
>> >> >> >> >> > > You can't go breaking userspace even if you don't like the 
>> >> >> >> >> > > application.
>> >> >> >> >> >
>> >> >> >> >> > Could you please explain how is the proposed patchset breaking
>> >> >> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the 
>> >> >> >> >> > userspace
>> >> >> >> >> > API at all.
>> >> >> >> >> >
>> >> >> >> >>
>> >> >> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux 
>> >> >> >> >> network devices
>> >> >> >> >> to look for Linux netvsc device and the paired VF device and 
>> >> >> >> >> setup the
>> >> >> >> >> DPDK environment.  This setup creates a DPDK failsafe 
>> >> >> >> >> (bondingish) instance
>> >> >> >> >> and sets up TAP support over the Linux netvsc device as well as 
>> >> >> >> >> the Mellanox
>> >> >> >> >> VF device.
>> >> >> >> >>
>> >> >> >> >> So it depends on existing 2 device model. You can't go to a 3 
>> >> >> >> >> device model
>> >> >> >> >> or start hiding devices from userspace.
>> >> >> >> >
>> >> >> >> > Okay so how does the existing patch break that? IIUC does not go 
>> >> >> >> > to
>> >> >> >> > a 3 device model since netvsc calls failover_register directly.
>> >> >> >> >
>> >> >> >> >> Also, I am working on associating netvsc and VF device based on 
>> >> >> >> >> serial number
>> >> >> >> >> rather than MAC address. The serial number is how Windows works 
>> >> >> >> >> now, and it makes
>> >> >> >> >> sense for Linux and Windows to use the same mechanism if 
>> >> >> >> >> possible.
>> >> >> >> >
>> >> >> >> > Maybe we should support same for virtio ...
>> >> >> >> > Which serial do you mean? From vpd?
>> >> >> >> >
>> >> >> >> > I guess you will want to keep supporting MAC for old hypervisors?
>> >> >> >> >
>> >> >> >> > It all seems like a reasonable thing to support in the generic 
>> >> >> >> > core.
>> >> >> >>
>> >> >> >> That's the reason why I chose explicit identifier rather than rely 
>> >> >> >> on
>> >> >> >> MAC address to bind/pair a device. MAC address can change. Even if 
>> >> >> >> it
>> >> >> >> can't, malicious guest user can fake MAC address to skip binding.
>> >> >> >>
>> >> >> >> -Siwei
>> >> >> >
>> >> >> > Address should be sampled at device creation to prevent this
>> >> >> > kind of hack. Not that it buys the malicious user much:
>> >> >> > if you can poke at MAC addresses you probably already can
>> >> >> > break networking.
>> >> >>
>> >> >> I don't understand why poking at MAC address may potentially break
>> >> >> networking.
>> >> >
>> >> > Set a MAC address to match another device on the same LAN,
>> >> > packets will stop reaching that MAC.
>> >>
>> >> What I meant was guest users may create a virtual link, say veth that
>> >> has exactly the same MAC address as that for the VF, which can easily
>> >> get around of the binding procedure.
>> >
>> > This patchset limits binding to PCI devices so it won't be affected
>> > by any hacks around virtual devices.
>>
>> Wait, I vaguely recall you seemed to like to generalize this feature
>> 

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Thu, Apr 26, 2018 at 03:14:46PM -0700, Siwei Liu wrote:
> On Wed, Apr 25, 2018 at 7:28 PM, Michael S. Tsirkin  wrote:
> > On Wed, Apr 25, 2018 at 03:57:57PM -0700, Siwei Liu wrote:
> >> On Wed, Apr 25, 2018 at 3:22 PM, Michael S. Tsirkin  
> >> wrote:
> >> > On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
> >> >> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  
> >> >> wrote:
> >> >> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
> >> >> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin 
> >> >> >>  wrote:
> >> >> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
> >> >> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
> >> >> >> >> "Michael S. Tsirkin"  wrote:
> >> >> >> >>
> >> >> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger 
> >> >> >> >> > wrote:
> >> >> >> >> > > > >
> >> >> >> >> > > > >I will NAK patches to change to common code for netvsc 
> >> >> >> >> > > > >especially the
> >> >> >> >> > > > >three device model.  MS worked hard with distro vendors to 
> >> >> >> >> > > > >support transparent
> >> >> >> >> > > > >mode, ans we really can't have a new model; or do backport.
> >> >> >> >> > > > >
> >> >> >> >> > > > >Plus, DPDK is now dependent on existing model.
> >> >> >> >> > > >
> >> >> >> >> > > > Sorry, but nobody here cares about dpdk or other similar 
> >> >> >> >> > > > oddities.
> >> >> >> >> > >
> >> >> >> >> > > The network device model is a userspace API, and DPDK is a 
> >> >> >> >> > > userspace application.
> >> >> >> >> >
> >> >> >> >> > It is userspace but are you sure dpdk is actually poking at 
> >> >> >> >> > netdevs?
> >> >> >> >> > AFAIK it's normally banging device registers directly.
> >> >> >> >> >
> >> >> >> >> > > You can't go breaking userspace even if you don't like the 
> >> >> >> >> > > application.
> >> >> >> >> >
> >> >> >> >> > Could you please explain how is the proposed patchset breaking
> >> >> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the 
> >> >> >> >> > userspace
> >> >> >> >> > API at all.
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux 
> >> >> >> >> network devices
> >> >> >> >> to look for Linux netvsc device and the paired VF device and 
> >> >> >> >> setup the
> >> >> >> >> DPDK environment.  This setup creates a DPDK failsafe 
> >> >> >> >> (bondingish) instance
> >> >> >> >> and sets up TAP support over the Linux netvsc device as well as 
> >> >> >> >> the Mellanox
> >> >> >> >> VF device.
> >> >> >> >>
> >> >> >> >> So it depends on existing 2 device model. You can't go to a 3 
> >> >> >> >> device model
> >> >> >> >> or start hiding devices from userspace.
> >> >> >> >
> >> >> >> > Okay so how does the existing patch break that? IIUC does not go to
> >> >> >> > a 3 device model since netvsc calls failover_register directly.
> >> >> >> >
> >> >> >> >> Also, I am working on associating netvsc and VF device based on 
> >> >> >> >> serial number
> >> >> >> >> rather than MAC address. The serial number is how Windows works 
> >> >> >> >> now, and it makes
> >> >> >> >> sense for Linux and Windows to use the same mechanism if possible.
> >> >> >> >
> >> >> >> > Maybe we should support same for virtio ...
> >> >> >> > Which serial do you mean? From vpd?
> >> >> >> >
> >> >> >> > I guess you will want to keep supporting MAC for old hypervisors?
> >> >> >> >
> >> >> >> > It all seems like a reasonable thing to support in the generic 
> >> >> >> > core.
> >> >> >>
> >> >> >> That's the reason why I chose explicit identifier rather than rely on
> >> >> >> MAC address to bind/pair a device. MAC address can change. Even if it
> >> >> >> can't, malicious guest user can fake MAC address to skip binding.
> >> >> >>
> >> >> >> -Siwei
> >> >> >
> >> >> > Address should be sampled at device creation to prevent this
> >> >> > kind of hack. Not that it buys the malicious user much:
> >> >> > if you can poke at MAC addresses you probably already can
> >> >> > break networking.
> >> >>
> >> >> I don't understand why poking at MAC address may potentially break
> >> >> networking.
> >> >
> >> > Set a MAC address to match another device on the same LAN,
> >> > packets will stop reaching that MAC.
> >>
> >> What I meant was guest users may create a virtual link, say veth that
> >> has exactly the same MAC address as that for the VF, which can easily
> >> get around of the binding procedure.
> >
> > This patchset limits binding to PCI devices so it won't be affected
> > by any hacks around virtual devices.
> 
> Wait, I vaguely recall you seemed to like to generalize this feature
> to non-PCI device. 

It's purely a layering thing.  It is cleaner not to have PCI specific
data in the device-specific transport-independent section of the virtio
spec.


> But now you're saying it should stick to PCI. It's
> not that I'm reluctant with 

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Siwei Liu]

On Wed, Apr 25, 2018 at 7:28 PM, Michael S. Tsirkin  wrote:
> On Wed, Apr 25, 2018 at 03:57:57PM -0700, Siwei Liu wrote:
>> On Wed, Apr 25, 2018 at 3:22 PM, Michael S. Tsirkin  wrote:
>> > On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
>> >> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  
>> >> wrote:
>> >> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> >> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  
>> >> >> wrote:
>> >> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> >> >> "Michael S. Tsirkin"  wrote:
>> >> >> >>
>> >> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> >> >> >> > > > >
>> >> >> >> > > > >I will NAK patches to change to common code for netvsc 
>> >> >> >> > > > >especially the
>> >> >> >> > > > >three device model.  MS worked hard with distro vendors to 
>> >> >> >> > > > >support transparent
>> >> >> >> > > > >mode, ans we really can't have a new model; or do backport.
>> >> >> >> > > > >
>> >> >> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> >> >> > > >
>> >> >> >> > > > Sorry, but nobody here cares about dpdk or other similar 
>> >> >> >> > > > oddities.
>> >> >> >> > >
>> >> >> >> > > The network device model is a userspace API, and DPDK is a 
>> >> >> >> > > userspace application.
>> >> >> >> >
>> >> >> >> > It is userspace but are you sure dpdk is actually poking at 
>> >> >> >> > netdevs?
>> >> >> >> > AFAIK it's normally banging device registers directly.
>> >> >> >> >
>> >> >> >> > > You can't go breaking userspace even if you don't like the 
>> >> >> >> > > application.
>> >> >> >> >
>> >> >> >> > Could you please explain how is the proposed patchset breaking
>> >> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the 
>> >> >> >> > userspace
>> >> >> >> > API at all.
>> >> >> >> >
>> >> >> >>
>> >> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux 
>> >> >> >> network devices
>> >> >> >> to look for Linux netvsc device and the paired VF device and setup 
>> >> >> >> the
>> >> >> >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) 
>> >> >> >> instance
>> >> >> >> and sets up TAP support over the Linux netvsc device as well as the 
>> >> >> >> Mellanox
>> >> >> >> VF device.
>> >> >> >>
>> >> >> >> So it depends on existing 2 device model. You can't go to a 3 
>> >> >> >> device model
>> >> >> >> or start hiding devices from userspace.
>> >> >> >
>> >> >> > Okay so how does the existing patch break that? IIUC does not go to
>> >> >> > a 3 device model since netvsc calls failover_register directly.
>> >> >> >
>> >> >> >> Also, I am working on associating netvsc and VF device based on 
>> >> >> >> serial number
>> >> >> >> rather than MAC address. The serial number is how Windows works 
>> >> >> >> now, and it makes
>> >> >> >> sense for Linux and Windows to use the same mechanism if possible.
>> >> >> >
>> >> >> > Maybe we should support same for virtio ...
>> >> >> > Which serial do you mean? From vpd?
>> >> >> >
>> >> >> > I guess you will want to keep supporting MAC for old hypervisors?
>> >> >> >
>> >> >> > It all seems like a reasonable thing to support in the generic core.
>> >> >>
>> >> >> That's the reason why I chose explicit identifier rather than rely on
>> >> >> MAC address to bind/pair a device. MAC address can change. Even if it
>> >> >> can't, malicious guest user can fake MAC address to skip binding.
>> >> >>
>> >> >> -Siwei
>> >> >
>> >> > Address should be sampled at device creation to prevent this
>> >> > kind of hack. Not that it buys the malicious user much:
>> >> > if you can poke at MAC addresses you probably already can
>> >> > break networking.
>> >>
>> >> I don't understand why poking at MAC address may potentially break
>> >> networking.
>> >
>> > Set a MAC address to match another device on the same LAN,
>> > packets will stop reaching that MAC.
>>
>> What I meant was guest users may create a virtual link, say veth that
>> has exactly the same MAC address as that for the VF, which can easily
>> get around of the binding procedure.
>
> This patchset limits binding to PCI devices so it won't be affected
> by any hacks around virtual devices.

Wait, I vaguely recall you seemed to like to generalize this feature
to non-PCI device. But now you're saying it should stick to PCI. It's
not that I'm reluctant with sticking to PCI. The fact is that I don't
think we can go with implementation until the semantics of the
so-called _F_STANDBY feature can be clearly defined into the spec.
Previously the boundary of using MAC address as the identifier for
bonding was quite confusing to me. And now PCI adds to the matrix.
However it still does not gurantee uniqueness I think. It's almost
incorrect of choosing MAC address as the ID in the beginning since
that has the 

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Wed, Apr 25, 2018 at 05:18:31PM -0700, Stephen Hemminger wrote:
> On Wed, 25 Apr 2018 15:57:57 -0700
> Siwei Liu  wrote:
> 
> > >
> > > I think ideally the infrastructure should suppport flexible matching of
> > > NICs - netvsc is already reported to be moving to some kind of serial
> > > address.
> > >  
> > As Stephen said, Hyper-V supports the serial UUID thing from day-one.
> > It's just the Linux netvsc guest driver itself does not leverage that
> > ID from the very beginging.
> > 
> > Regards,
> > -Siwei
> 
> I am working on that.  The problem is that it requires some messy work
> to go from VF netdevice back to PCI device and from there to the PCI hyperv
> host infrastructure to find the serial number.
> 
> I was hoping that the serial number would also match the concept of PCI 
> Express
> device serial number. But that is a completely different ID :-( 
> The PCI-E serial number is a hardware serial number more like MAC address.
> The Hyper-V serial number is more like PCI slot value.

Asuming you mean the Device Serial Number Capability,
I did consider this, and
we could use that, changing the UUID to one matching the
PV device, but I'm not sure no drivers will get confused
suddenly seeing the UUID of another company there.

If we are going the UUID route, a better idea might be to
specify the UUID of the PCI port into which the PT device is
being hotplugged.  This is PCI-Express specific, but presumably
legacy PCI/PCI-X devices aren't common enough to bother about
this for now.


-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Wed, Apr 25, 2018 at 03:57:57PM -0700, Siwei Liu wrote:
> On Wed, Apr 25, 2018 at 3:22 PM, Michael S. Tsirkin  wrote:
> > On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
> >> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  
> >> wrote:
> >> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
> >> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  
> >> >> wrote:
> >> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
> >> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
> >> >> >> "Michael S. Tsirkin"  wrote:
> >> >> >>
> >> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
> >> >> >> > > > >
> >> >> >> > > > >I will NAK patches to change to common code for netvsc 
> >> >> >> > > > >especially the
> >> >> >> > > > >three device model.  MS worked hard with distro vendors to 
> >> >> >> > > > >support transparent
> >> >> >> > > > >mode, ans we really can't have a new model; or do backport.
> >> >> >> > > > >
> >> >> >> > > > >Plus, DPDK is now dependent on existing model.
> >> >> >> > > >
> >> >> >> > > > Sorry, but nobody here cares about dpdk or other similar 
> >> >> >> > > > oddities.
> >> >> >> > >
> >> >> >> > > The network device model is a userspace API, and DPDK is a 
> >> >> >> > > userspace application.
> >> >> >> >
> >> >> >> > It is userspace but are you sure dpdk is actually poking at 
> >> >> >> > netdevs?
> >> >> >> > AFAIK it's normally banging device registers directly.
> >> >> >> >
> >> >> >> > > You can't go breaking userspace even if you don't like the 
> >> >> >> > > application.
> >> >> >> >
> >> >> >> > Could you please explain how is the proposed patchset breaking
> >> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the 
> >> >> >> > userspace
> >> >> >> > API at all.
> >> >> >> >
> >> >> >>
> >> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux 
> >> >> >> network devices
> >> >> >> to look for Linux netvsc device and the paired VF device and setup 
> >> >> >> the
> >> >> >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) 
> >> >> >> instance
> >> >> >> and sets up TAP support over the Linux netvsc device as well as the 
> >> >> >> Mellanox
> >> >> >> VF device.
> >> >> >>
> >> >> >> So it depends on existing 2 device model. You can't go to a 3 device 
> >> >> >> model
> >> >> >> or start hiding devices from userspace.
> >> >> >
> >> >> > Okay so how does the existing patch break that? IIUC does not go to
> >> >> > a 3 device model since netvsc calls failover_register directly.
> >> >> >
> >> >> >> Also, I am working on associating netvsc and VF device based on 
> >> >> >> serial number
> >> >> >> rather than MAC address. The serial number is how Windows works now, 
> >> >> >> and it makes
> >> >> >> sense for Linux and Windows to use the same mechanism if possible.
> >> >> >
> >> >> > Maybe we should support same for virtio ...
> >> >> > Which serial do you mean? From vpd?
> >> >> >
> >> >> > I guess you will want to keep supporting MAC for old hypervisors?
> >> >> >
> >> >> > It all seems like a reasonable thing to support in the generic core.
> >> >>
> >> >> That's the reason why I chose explicit identifier rather than rely on
> >> >> MAC address to bind/pair a device. MAC address can change. Even if it
> >> >> can't, malicious guest user can fake MAC address to skip binding.
> >> >>
> >> >> -Siwei
> >> >
> >> > Address should be sampled at device creation to prevent this
> >> > kind of hack. Not that it buys the malicious user much:
> >> > if you can poke at MAC addresses you probably already can
> >> > break networking.
> >>
> >> I don't understand why poking at MAC address may potentially break
> >> networking.
> >
> > Set a MAC address to match another device on the same LAN,
> > packets will stop reaching that MAC.
> 
> What I meant was guest users may create a virtual link, say veth that
> has exactly the same MAC address as that for the VF, which can easily
> get around of the binding procedure.

This patchset limits binding to PCI devices so it won't be affected
by any hacks around virtual devices.

> There's no explicit flag to
> identify a VF or pass-through device AFAIK. And sometimes this happens
> maybe due to user misconfiguring the link. This process should be
> hardened to avoid from any potential configuration errors.

They are still PCI devices though.

> >
> >> Unlike VF, passthrough PCI endpoint device has its freedom
> >> to change the MAC address. Even on a VF setup it's not neccessarily
> >> always safe to assume the VF's MAC address cannot or shouldn't be
> >> changed. That depends on the specific need whether the host admin
> >> wants to restrict guest from changing the MAC address, although in
> >> most cases it's true.
> >>
> >> I understand we can use the perm_addr to distinguish. But as said,
> >> this will pose limitation of flexible configuration where 

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Siwei Liu]

On Wed, Apr 25, 2018 at 3:22 PM, Michael S. Tsirkin  wrote:
> On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
>> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  wrote:
>> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  
>> >> wrote:
>> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> >> "Michael S. Tsirkin"  wrote:
>> >> >>
>> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> >> >> > > > >
>> >> >> > > > >I will NAK patches to change to common code for netvsc 
>> >> >> > > > >especially the
>> >> >> > > > >three device model.  MS worked hard with distro vendors to 
>> >> >> > > > >support transparent
>> >> >> > > > >mode, ans we really can't have a new model; or do backport.
>> >> >> > > > >
>> >> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> >> > > >
>> >> >> > > > Sorry, but nobody here cares about dpdk or other similar 
>> >> >> > > > oddities.
>> >> >> > >
>> >> >> > > The network device model is a userspace API, and DPDK is a 
>> >> >> > > userspace application.
>> >> >> >
>> >> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
>> >> >> > AFAIK it's normally banging device registers directly.
>> >> >> >
>> >> >> > > You can't go breaking userspace even if you don't like the 
>> >> >> > > application.
>> >> >> >
>> >> >> > Could you please explain how is the proposed patchset breaking
>> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the 
>> >> >> > userspace
>> >> >> > API at all.
>> >> >> >
>> >> >>
>> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux network 
>> >> >> devices
>> >> >> to look for Linux netvsc device and the paired VF device and setup the
>> >> >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) 
>> >> >> instance
>> >> >> and sets up TAP support over the Linux netvsc device as well as the 
>> >> >> Mellanox
>> >> >> VF device.
>> >> >>
>> >> >> So it depends on existing 2 device model. You can't go to a 3 device 
>> >> >> model
>> >> >> or start hiding devices from userspace.
>> >> >
>> >> > Okay so how does the existing patch break that? IIUC does not go to
>> >> > a 3 device model since netvsc calls failover_register directly.
>> >> >
>> >> >> Also, I am working on associating netvsc and VF device based on serial 
>> >> >> number
>> >> >> rather than MAC address. The serial number is how Windows works now, 
>> >> >> and it makes
>> >> >> sense for Linux and Windows to use the same mechanism if possible.
>> >> >
>> >> > Maybe we should support same for virtio ...
>> >> > Which serial do you mean? From vpd?
>> >> >
>> >> > I guess you will want to keep supporting MAC for old hypervisors?
>> >> >
>> >> > It all seems like a reasonable thing to support in the generic core.
>> >>
>> >> That's the reason why I chose explicit identifier rather than rely on
>> >> MAC address to bind/pair a device. MAC address can change. Even if it
>> >> can't, malicious guest user can fake MAC address to skip binding.
>> >>
>> >> -Siwei
>> >
>> > Address should be sampled at device creation to prevent this
>> > kind of hack. Not that it buys the malicious user much:
>> > if you can poke at MAC addresses you probably already can
>> > break networking.
>>
>> I don't understand why poking at MAC address may potentially break
>> networking.
>
> Set a MAC address to match another device on the same LAN,
> packets will stop reaching that MAC.

What I meant was guest users may create a virtual link, say veth that
has exactly the same MAC address as that for the VF, which can easily
get around of the binding procedure. There's no explicit flag to
identify a VF or pass-through device AFAIK. And sometimes this happens
maybe due to user misconfiguring the link. This process should be
hardened to avoid from any potential configuration errors.

>
>> Unlike VF, passthrough PCI endpoint device has its freedom
>> to change the MAC address. Even on a VF setup it's not neccessarily
>> always safe to assume the VF's MAC address cannot or shouldn't be
>> changed. That depends on the specific need whether the host admin
>> wants to restrict guest from changing the MAC address, although in
>> most cases it's true.
>>
>> I understand we can use the perm_addr to distinguish. But as said,
>> this will pose limitation of flexible configuration where one can
>> assign VFs with identical MAC address at all while each VF belongs to
>> different PF and/or different subnet for e.g. load balancing.
>> And
>> furthermore, the QEMU device model never uses MAC address to be
>> interpreted as an identifier, which requires to be unique per VM
>> instance. Why we're introducing this inconsistency?
>>
>> -Siwei
>
> Because it addresses most of the issues and is simple.  That's already
> much 

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  wrote:
> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  
> >> wrote:
> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
> >> >> "Michael S. Tsirkin"  wrote:
> >> >>
> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
> >> >> > > > >
> >> >> > > > >I will NAK patches to change to common code for netvsc 
> >> >> > > > >especially the
> >> >> > > > >three device model.  MS worked hard with distro vendors to 
> >> >> > > > >support transparent
> >> >> > > > >mode, ans we really can't have a new model; or do backport.
> >> >> > > > >
> >> >> > > > >Plus, DPDK is now dependent on existing model.
> >> >> > > >
> >> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
> >> >> > >
> >> >> > > The network device model is a userspace API, and DPDK is a 
> >> >> > > userspace application.
> >> >> >
> >> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
> >> >> > AFAIK it's normally banging device registers directly.
> >> >> >
> >> >> > > You can't go breaking userspace even if you don't like the 
> >> >> > > application.
> >> >> >
> >> >> > Could you please explain how is the proposed patchset breaking
> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the 
> >> >> > userspace
> >> >> > API at all.
> >> >> >
> >> >>
> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux network 
> >> >> devices
> >> >> to look for Linux netvsc device and the paired VF device and setup the
> >> >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) 
> >> >> instance
> >> >> and sets up TAP support over the Linux netvsc device as well as the 
> >> >> Mellanox
> >> >> VF device.
> >> >>
> >> >> So it depends on existing 2 device model. You can't go to a 3 device 
> >> >> model
> >> >> or start hiding devices from userspace.
> >> >
> >> > Okay so how does the existing patch break that? IIUC does not go to
> >> > a 3 device model since netvsc calls failover_register directly.
> >> >
> >> >> Also, I am working on associating netvsc and VF device based on serial 
> >> >> number
> >> >> rather than MAC address. The serial number is how Windows works now, 
> >> >> and it makes
> >> >> sense for Linux and Windows to use the same mechanism if possible.
> >> >
> >> > Maybe we should support same for virtio ...
> >> > Which serial do you mean? From vpd?
> >> >
> >> > I guess you will want to keep supporting MAC for old hypervisors?
> >> >
> >> > It all seems like a reasonable thing to support in the generic core.
> >>
> >> That's the reason why I chose explicit identifier rather than rely on
> >> MAC address to bind/pair a device. MAC address can change. Even if it
> >> can't, malicious guest user can fake MAC address to skip binding.
> >>
> >> -Siwei
> >
> > Address should be sampled at device creation to prevent this
> > kind of hack. Not that it buys the malicious user much:
> > if you can poke at MAC addresses you probably already can
> > break networking.
> 
> I don't understand why poking at MAC address may potentially break
> networking.

Set a MAC address to match another device on the same LAN,
packets will stop reaching that MAC.

> Unlike VF, passthrough PCI endpoint device has its freedom
> to change the MAC address. Even on a VF setup it's not neccessarily
> always safe to assume the VF's MAC address cannot or shouldn't be
> changed. That depends on the specific need whether the host admin
> wants to restrict guest from changing the MAC address, although in
> most cases it's true.
> 
> I understand we can use the perm_addr to distinguish. But as said,
> this will pose limitation of flexible configuration where one can
> assign VFs with identical MAC address at all while each VF belongs to
> different PF and/or different subnet for e.g. load balancing.
> And
> furthermore, the QEMU device model never uses MAC address to be
> interpreted as an identifier, which requires to be unique per VM
> instance. Why we're introducing this inconsistency?
> 
> -Siwei

Because it addresses most of the issues and is simple.  That's already
much better than what we have now which is nothing unless guest
configures things manually.

I think ideally the infrastructure should suppport flexible matching of
NICs - netvsc is already reported to be moving to some kind of serial
address.


> >
> >
> >
> >
> >>
> >> >
> >> > --
> >> > MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Siwei Liu]

On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin  wrote:
> On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  wrote:
>> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> "Michael S. Tsirkin"  wrote:
>> >>
>> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> >> > > > >
>> >> > > > >I will NAK patches to change to common code for netvsc especially 
>> >> > > > >the
>> >> > > > >three device model.  MS worked hard with distro vendors to support 
>> >> > > > >transparent
>> >> > > > >mode, ans we really can't have a new model; or do backport.
>> >> > > > >
>> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> > > >
>> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
>> >> > >
>> >> > > The network device model is a userspace API, and DPDK is a userspace 
>> >> > > application.
>> >> >
>> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
>> >> > AFAIK it's normally banging device registers directly.
>> >> >
>> >> > > You can't go breaking userspace even if you don't like the 
>> >> > > application.
>> >> >
>> >> > Could you please explain how is the proposed patchset breaking
>> >> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
>> >> > API at all.
>> >> >
>> >>
>> >> The DPDK has a device driver vdev_netvsc which scans the Linux network 
>> >> devices
>> >> to look for Linux netvsc device and the paired VF device and setup the
>> >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) 
>> >> instance
>> >> and sets up TAP support over the Linux netvsc device as well as the 
>> >> Mellanox
>> >> VF device.
>> >>
>> >> So it depends on existing 2 device model. You can't go to a 3 device model
>> >> or start hiding devices from userspace.
>> >
>> > Okay so how does the existing patch break that? IIUC does not go to
>> > a 3 device model since netvsc calls failover_register directly.
>> >
>> >> Also, I am working on associating netvsc and VF device based on serial 
>> >> number
>> >> rather than MAC address. The serial number is how Windows works now, and 
>> >> it makes
>> >> sense for Linux and Windows to use the same mechanism if possible.
>> >
>> > Maybe we should support same for virtio ...
>> > Which serial do you mean? From vpd?
>> >
>> > I guess you will want to keep supporting MAC for old hypervisors?
>> >
>> > It all seems like a reasonable thing to support in the generic core.
>>
>> That's the reason why I chose explicit identifier rather than rely on
>> MAC address to bind/pair a device. MAC address can change. Even if it
>> can't, malicious guest user can fake MAC address to skip binding.
>>
>> -Siwei
>
> Address should be sampled at device creation to prevent this
> kind of hack. Not that it buys the malicious user much:
> if you can poke at MAC addresses you probably already can
> break networking.

I don't understand why poking at MAC address may potentially break
networking. Unlike VF, passthrough PCI endpoint device has its freedom
to change the MAC address. Even on a VF setup it's not neccessarily
always safe to assume the VF's MAC address cannot or shouldn't be
changed. That depends on the specific need whether the host admin
wants to restrict guest from changing the MAC address, although in
most cases it's true.

I understand we can use the perm_addr to distinguish. But as said,
this will pose limitation of flexible configuration where one can
assign VFs with identical MAC address at all while each VF belongs to
different PF and/or different subnet for e.g. load balancing. And
furthermore, the QEMU device model never uses MAC address to be
interpreted as an identifier, which requires to be unique per VM
instance. Why we're introducing this inconsistency?

-Siwei

>
>
>
>
>>
>> >
>> > --
>> > MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 06:25:03PM -0700, Stephen Hemminger wrote:
> On Mon, 23 Apr 2018 12:44:39 -0700
> Siwei Liu  wrote:
> 
> > On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  
> > wrote:
> > > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:  
> > >> On Mon, 23 Apr 2018 20:24:56 +0300
> > >> "Michael S. Tsirkin"  wrote:
> > >>  
> > >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:  
> > >> > > > >
> > >> > > > >I will NAK patches to change to common code for netvsc especially 
> > >> > > > >the
> > >> > > > >three device model.  MS worked hard with distro vendors to 
> > >> > > > >support transparent
> > >> > > > >mode, ans we really can't have a new model; or do backport.
> > >> > > > >
> > >> > > > >Plus, DPDK is now dependent on existing model.  
> > >> > > >
> > >> > > > Sorry, but nobody here cares about dpdk or other similar oddities. 
> > >> > > >  
> > >> > >
> > >> > > The network device model is a userspace API, and DPDK is a userspace 
> > >> > > application.  
> > >> >
> > >> > It is userspace but are you sure dpdk is actually poking at netdevs?
> > >> > AFAIK it's normally banging device registers directly.
> > >> >  
> > >> > > You can't go breaking userspace even if you don't like the 
> > >> > > application.  
> > >> >
> > >> > Could you please explain how is the proposed patchset breaking
> > >> > userspace? Ignoring DPDK for now, I don't think it changes the 
> > >> > userspace
> > >> > API at all.
> > >> >  
> > >>
> > >> The DPDK has a device driver vdev_netvsc which scans the Linux network 
> > >> devices
> > >> to look for Linux netvsc device and the paired VF device and setup the
> > >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) 
> > >> instance
> > >> and sets up TAP support over the Linux netvsc device as well as the 
> > >> Mellanox
> > >> VF device.
> > >>
> > >> So it depends on existing 2 device model. You can't go to a 3 device 
> > >> model
> > >> or start hiding devices from userspace.  
> > >
> > > Okay so how does the existing patch break that? IIUC does not go to
> > > a 3 device model since netvsc calls failover_register directly.
> > >  
> > >> Also, I am working on associating netvsc and VF device based on serial 
> > >> number
> > >> rather than MAC address. The serial number is how Windows works now, and 
> > >> it makes
> > >> sense for Linux and Windows to use the same mechanism if possible.  
> > >
> > > Maybe we should support same for virtio ...
> > > Which serial do you mean? From vpd?
> > >
> > > I guess you will want to keep supporting MAC for old hypervisors?
> 
> The serial number has always been in the hypervisor since original support of 
> SR-IOV
> in WS2008.  So no backward compatibility special cases would be needed.

Is that a serial from real hardware or a hypervisor thing?


-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  wrote:
> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
> >> On Mon, 23 Apr 2018 20:24:56 +0300
> >> "Michael S. Tsirkin"  wrote:
> >>
> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
> >> > > > >
> >> > > > >I will NAK patches to change to common code for netvsc especially 
> >> > > > >the
> >> > > > >three device model.  MS worked hard with distro vendors to support 
> >> > > > >transparent
> >> > > > >mode, ans we really can't have a new model; or do backport.
> >> > > > >
> >> > > > >Plus, DPDK is now dependent on existing model.
> >> > > >
> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
> >> > >
> >> > > The network device model is a userspace API, and DPDK is a userspace 
> >> > > application.
> >> >
> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
> >> > AFAIK it's normally banging device registers directly.
> >> >
> >> > > You can't go breaking userspace even if you don't like the application.
> >> >
> >> > Could you please explain how is the proposed patchset breaking
> >> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
> >> > API at all.
> >> >
> >>
> >> The DPDK has a device driver vdev_netvsc which scans the Linux network 
> >> devices
> >> to look for Linux netvsc device and the paired VF device and setup the
> >> DPDK environment.  This setup creates a DPDK failsafe (bondingish) instance
> >> and sets up TAP support over the Linux netvsc device as well as the 
> >> Mellanox
> >> VF device.
> >>
> >> So it depends on existing 2 device model. You can't go to a 3 device model
> >> or start hiding devices from userspace.
> >
> > Okay so how does the existing patch break that? IIUC does not go to
> > a 3 device model since netvsc calls failover_register directly.
> >
> >> Also, I am working on associating netvsc and VF device based on serial 
> >> number
> >> rather than MAC address. The serial number is how Windows works now, and 
> >> it makes
> >> sense for Linux and Windows to use the same mechanism if possible.
> >
> > Maybe we should support same for virtio ...
> > Which serial do you mean? From vpd?
> >
> > I guess you will want to keep supporting MAC for old hypervisors?
> >
> > It all seems like a reasonable thing to support in the generic core.
> 
> That's the reason why I chose explicit identifier rather than rely on
> MAC address to bind/pair a device. MAC address can change. Even if it
> can't, malicious guest user can fake MAC address to skip binding.
> 
> -Siwei

Address should be sampled at device creation to prevent this
kind of hack. Not that it buys the malicious user much:
if you can poke at MAC addresses you probably already can
break networking.




> 
> >
> > --
> > MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Siwei Liu]

On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin  wrote:
> On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> On Mon, 23 Apr 2018 20:24:56 +0300
>> "Michael S. Tsirkin"  wrote:
>>
>> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> > > > >
>> > > > >I will NAK patches to change to common code for netvsc especially the
>> > > > >three device model.  MS worked hard with distro vendors to support 
>> > > > >transparent
>> > > > >mode, ans we really can't have a new model; or do backport.
>> > > > >
>> > > > >Plus, DPDK is now dependent on existing model.
>> > > >
>> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
>> > >
>> > > The network device model is a userspace API, and DPDK is a userspace 
>> > > application.
>> >
>> > It is userspace but are you sure dpdk is actually poking at netdevs?
>> > AFAIK it's normally banging device registers directly.
>> >
>> > > You can't go breaking userspace even if you don't like the application.
>> >
>> > Could you please explain how is the proposed patchset breaking
>> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
>> > API at all.
>> >
>>
>> The DPDK has a device driver vdev_netvsc which scans the Linux network 
>> devices
>> to look for Linux netvsc device and the paired VF device and setup the
>> DPDK environment.  This setup creates a DPDK failsafe (bondingish) instance
>> and sets up TAP support over the Linux netvsc device as well as the Mellanox
>> VF device.
>>
>> So it depends on existing 2 device model. You can't go to a 3 device model
>> or start hiding devices from userspace.
>
> Okay so how does the existing patch break that? IIUC does not go to
> a 3 device model since netvsc calls failover_register directly.
>
>> Also, I am working on associating netvsc and VF device based on serial number
>> rather than MAC address. The serial number is how Windows works now, and it 
>> makes
>> sense for Linux and Windows to use the same mechanism if possible.
>
> Maybe we should support same for virtio ...
> Which serial do you mean? From vpd?
>
> I guess you will want to keep supporting MAC for old hypervisors?
>
> It all seems like a reasonable thing to support in the generic core.

That's the reason why I chose explicit identifier rather than rely on
MAC address to bind/pair a device. MAC address can change. Even if it
can't, malicious guest user can fake MAC address to skip binding.

-Siwei


>
> --
> MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
> On Mon, 23 Apr 2018 20:24:56 +0300
> "Michael S. Tsirkin"  wrote:
> 
> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
> > > > >
> > > > >I will NAK patches to change to common code for netvsc especially the
> > > > >three device model.  MS worked hard with distro vendors to support 
> > > > >transparent
> > > > >mode, ans we really can't have a new model; or do backport.
> > > > >
> > > > >Plus, DPDK is now dependent on existing model.
> > > > 
> > > > Sorry, but nobody here cares about dpdk or other similar oddities.  
> > > 
> > > The network device model is a userspace API, and DPDK is a userspace 
> > > application.  
> > 
> > It is userspace but are you sure dpdk is actually poking at netdevs?
> > AFAIK it's normally banging device registers directly.
> > 
> > > You can't go breaking userspace even if you don't like the application.  
> > 
> > Could you please explain how is the proposed patchset breaking
> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
> > API at all.
> > 
> 
> The DPDK has a device driver vdev_netvsc which scans the Linux network devices
> to look for Linux netvsc device and the paired VF device and setup the
> DPDK environment.  This setup creates a DPDK failsafe (bondingish) instance
> and sets up TAP support over the Linux netvsc device as well as the Mellanox
> VF device.
> 
> So it depends on existing 2 device model. You can't go to a 3 device model
> or start hiding devices from userspace.

Okay so how does the existing patch break that? IIUC does not go to
a 3 device model since netvsc calls failover_register directly.

> Also, I am working on associating netvsc and VF device based on serial number
> rather than MAC address. The serial number is how Windows works now, and it 
> makes
> sense for Linux and Windows to use the same mechanism if possible.

Maybe we should support same for virtio ...
Which serial do you mean? From vpd?

I guess you will want to keep supporting MAC for old hypervisors?

It all seems like a reasonable thing to support in the generic core.

-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
> > >
> > >I will NAK patches to change to common code for netvsc especially the
> > >three device model.  MS worked hard with distro vendors to support 
> > >transparent
> > >mode, ans we really can't have a new model; or do backport.
> > >
> > >Plus, DPDK is now dependent on existing model.  
> > 
> > Sorry, but nobody here cares about dpdk or other similar oddities.
> 
> The network device model is a userspace API, and DPDK is a userspace 
> application.

It is userspace but are you sure dpdk is actually poking at netdevs?
AFAIK it's normally banging device registers directly.

> You can't go breaking userspace even if you don't like the application.

Could you please explain how is the proposed patchset breaking
userspace? Ignoring DPDK for now, I don't think it changes the userspace
API at all.

-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Samudrala, Sridhar]

On 4/20/2018 8:28 AM, Stephen Hemminger wrote:

On Thu, 19 Apr 2018 18:42:04 -0700
Sridhar Samudrala  wrote:


Use the registration/notification framework supported by the generic
failover infrastructure.

Signed-off-by: Sridhar Samudrala 

Do what you want to other devices but leave netvsc alone.
Adding these failover ops does not reduce the code size, and really is
no benefit.  The netvsc device driver needs to be backported to several
other distributions and doing this makes that harder.

I will NAK patches to change to common code for netvsc especially the
three device model.  MS worked hard with distro vendors to support transparent
mode, ans we really can't have a new model; or do backport.


failover_ops are specifically added to support both 2-netdev and 3-netdev models
This patch doesn't change netvsc model. It still keeps its 2-netdev model. From
netvsc, point of view it is just moving some code from netvsc to the failover 
module
and also i think the eventhandling and getbymac routines are more optimal.



Plus, DPDK is now dependent on existing model.



-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org

[virtio-dev] Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework

[Michael S. Tsirkin]

On Fri, Apr 20, 2018 at 08:28:02AM -0700, Stephen Hemminger wrote:
> On Thu, 19 Apr 2018 18:42:04 -0700
> Sridhar Samudrala  wrote:
> 
> > Use the registration/notification framework supported by the generic
> > failover infrastructure.
> > 
> > Signed-off-by: Sridhar Samudrala 
> 
> Do what you want to other devices but leave netvsc alone.
> Adding these failover ops does not reduce the code size,

drivers/net/hyperv/Kconfig  |   1 +
drivers/net/hyperv/hyperv_net.h |   2 +
drivers/net/hyperv/netvsc_drv.c | 208 ++--
3 files changed, 55 insertions(+), 156 deletions(-)

100 lines gone.


> and really is
> no benefit.  The netvsc device driver needs to be backported to several
> other distributions and doing this makes that harder.
> 
> I will NAK patches to change to common code for netvsc

Wow.

> especially the
> three device model.

AFAIK these patches do not change netvsc to a three device model.

> MS worked hard with distro vendors to support transparent
> mode, ans we really can't have a new model;

That's why Sridhar worked hard to preserve a 2 device model for netvsc.

> or do backport.
>
> Plus, DPDK is now dependent on existing model.

DPDK does the kernel bypass thing, doesn't it? Why does the kernel care?

-- 
MST

-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org