* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Stephen Hemminger @ 2019-02-28 0:03 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Samudrala, Sridhar, virtualization, Siwei Liu, liran.alon, Netdev,
si-wei liu, David Miller
In-Reply-To: <20190227184601-mutt-send-email-mst@kernel.org>
On Wed, 27 Feb 2019 18:50:44 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Feb 27, 2019 at 03:34:56PM -0800, si-wei liu wrote:
> >
> >
> > On 2/27/2019 2:38 PM, Michael S. Tsirkin wrote:
> > > On Tue, Feb 26, 2019 at 04:17:21PM -0800, si-wei liu wrote:
> > > >
> > > > On 2/25/2019 6:08 PM, Michael S. Tsirkin wrote:
> > > > > On Mon, Feb 25, 2019 at 04:58:07PM -0800, si-wei liu wrote:
> > > > > > On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
> > > > > > > On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:
> > > > > > > > On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
> > > > > > > > > On 2/21/2019 7:33 PM, si-wei liu wrote:
> > > > > > > > > > On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
> > > > > > > > > > > On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
> > > > > > > > > > > > Sorry for replying to this ancient thread. There was some remaining
> > > > > > > > > > > > issue that I don't think the initial net_failover patch got addressed
> > > > > > > > > > > > cleanly, see:
> > > > > > > > > > > >
> > > > > > > > > > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
> > > > > > > > > > > >
> > > > > > > > > > > > The renaming of 'eth0' to 'ens4' fails because the udev userspace was
> > > > > > > > > > > > not specifically writtten for such kernel automatic enslavement.
> > > > > > > > > > > > Specifically, if it is a bond or team, the slave would typically get
> > > > > > > > > > > > renamed *before* virtual device gets created, that's what udev can
> > > > > > > > > > > > control (without getting netdev opened early by the other part of
> > > > > > > > > > > > kernel) and other userspace components for e.g. initramfs,
> > > > > > > > > > > > init-scripts can coordinate well in between. The in-kernel
> > > > > > > > > > > > auto-enslavement of net_failover breaks this userspace convention,
> > > > > > > > > > > > which don't provides a solution if user care about consistent naming
> > > > > > > > > > > > on the slave netdevs specifically.
> > > > > > > > > > > >
> > > > > > > > > > > > Previously this issue had been specifically called out when IFF_HIDDEN
> > > > > > > > > > > > and the 1-netdev was proposed, but no one gives out a solution to this
> > > > > > > > > > > > problem ever since. Please share your mind how to proceed and solve
> > > > > > > > > > > > this userspace issue if netdev does not welcome a 1-netdev model.
> > > > > > > > > > > Above says:
> > > > > > > > > > >
> > > > > > > > > > > there's no motivation in the systemd/udevd community at
> > > > > > > > > > > this point to refactor the rename logic and make it work well with
> > > > > > > > > > > 3-netdev.
> > > > > > > > > > >
> > > > > > > > > > > What would the fix be? Skip slave devices?
> > > > > > > > > > >
> > > > > > > > > > There's nothing user can get if just skipping slave devices - the
> > > > > > > > > > name is still unchanged and unpredictable e.g. eth0, or eth1 the
> > > > > > > > > > next reboot, while the rest may conform to the naming scheme (ens3
> > > > > > > > > > and such). There's no way one can fix this in userspace alone - when
> > > > > > > > > > the failover is created the enslaved netdev was opened by the kernel
> > > > > > > > > > earlier than the userspace is made aware of, and there's no
> > > > > > > > > > negotiation protocol for kernel to know when userspace has done
> > > > > > > > > > initial renaming of the interface. I would expect netdev list should
> > > > > > > > > > at least provide the direction in general for how this can be
> > > > > > > > > > solved...
> > > > > > > I was just wondering what did you mean when you said
> > > > > > > "refactor the rename logic and make it work well with 3-netdev" -
> > > > > > > was there a proposal udev rejected?
> > > > > > No. I never believed this particular issue can be fixed in userspace alone.
> > > > > > Previously someone had said it could be, but I never see any work or
> > > > > > relevant discussion ever happened in various userspace communities (for e.g.
> > > > > > dracut, initramfs-tools, systemd, udev, and NetworkManager). IMHO the root
> > > > > > of the issue derives from the kernel, it makes more sense to start from
> > > > > > netdev, work out and decide on a solution: see what can be done in the
> > > > > > kernel in order to fix it, then after that engage userspace community for
> > > > > > the feasibility...
> > > > > >
> > > > > > > Anyway, can we write a time diagram for what happens in which order that
> > > > > > > leads to failure? That would help look for triggers that we can tie
> > > > > > > into, or add new ones.
> > > > > > >
> > > > > > See attached diagram.
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > > Is there an issue if slave device names are not predictable? The user/admin scripts are expected
> > > > > > > > > to only work with the master failover device.
> > > > > > > > Where does this expectation come from?
> > > > > > > >
> > > > > > > > Admin users may have ethtool or tc configurations that need to deal with
> > > > > > > > predictable interface name. Third-party app which was built upon specifying
> > > > > > > > certain interface name can't be modified to chase dynamic names.
> > > > > > > >
> > > > > > > > Specifically, we have pre-canned image that uses ethtool to fine tune VF
> > > > > > > > offload settings post boot for specific workload. Those images won't work
> > > > > > > > well if the name is constantly changing just after couple rounds of live
> > > > > > > > migration.
> > > > > > > It should be possible to specify the ethtool configuration on the
> > > > > > > master and have it automatically propagated to the slave.
> > > > > > >
> > > > > > > BTW this is something we should look at IMHO.
> > > > > > I was elaborating a few examples that the expectation and assumption that
> > > > > > user/admin scripts only deal with master failover device is incorrect. It
> > > > > > had never been taken good care of, although I did try to emphasize it from
> > > > > > the very beginning.
> > > > > >
> > > > > > Basically what you said about propagating the ethtool configuration down to
> > > > > > the slave is the key pursuance of 1-netdev model. However, what I am seeking
> > > > > > now is any alternative that can also fix the specific udev rename problem,
> > > > > > before concluding that 1-netdev is the only solution. Generally a 1-netdev
> > > > > > scheme would take time to implement, while I'm trying to find a way out to
> > > > > > fix this particular naming problem under 3-netdev.
> > > > > >
> > > > > > > > > Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> > > > > > > > > about moving them to a hidden network namespace so that they are not visible from the default namespace.
> > > > > > > > > I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> > > > > > > > > kernel. If so, we could use this mechanism to simulate a 1-netdev model.
> > > > > > > > Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> > > > > > > > model as much transparent to a real NIC as possible, while a hidden netns is
> > > > > > > > just the vehicle). However, I recall there was resistance around this
> > > > > > > > discussion that even the concept of hiding itself is a taboo for Linux
> > > > > > > > netdev. I would like to summon potential alternatives before concluding
> > > > > > > > 1-netdev is the only solution too soon.
> > > > > > > >
> > > > > > > > Thanks,
> > > > > > > > -Siwei
> > > > > > > Your scripts would not work at all then, right?
> > > > > > At this point we don't claim images with such usage as SR-IOV live
> > > > > > migrate-able. We would flag it as live migrate-able until this ethtool
> > > > > > config issue is fully addressed and a transparent live migration solution
> > > > > > emerges in upstream eventually.
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > > -Siwei
> > > > > > > > > > -Siwei
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > ---------------------------------------------------------------------
> > > > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > > > > >
> > > > > > net_failover(kernel) | network.service (user) | systemd-udevd (user)
> > > > > > --------------------------------------------------+------------------------------+--------------------------------------------
> > > > > > (standby virtio-net and net_failover | |
> > > > > > devices created and initialized, | |
> > > > > > i.e. virtnet_probe()-> | |
> > > > > > net_failover_create() | |
> > > > > > was done.) | |
> > > > > > | |
> > > > > > | runs `ifup ens3' -> |
> > > > > > | ip link set dev ens3 up |
> > > > > > net_failover_open() | |
> > > > > > dev_open(virtnet_dev) | |
> > > > > > virtnet_open(virtnet_dev) | |
> > > > > > netif_carrier_on(failover_dev) | |
> > > > > > ... | |
> > > > > > | |
> > > > > > (VF hot plugged in) | |
> > > > > > ixgbevf_probe() | |
> > > > > > register_netdev(ixgbevf_netdev) | |
> > > > > > netdev_register_kobject(ixgbevf_netdev) | |
> > > > > > kobject_add(ixgbevf_dev) | |
> > > > > > device_add(ixgbevf_dev) | |
> > > > > > kobject_uevent(&ixgbevf_dev->kobj, KOBJ_ADD) | |
> > > > > > netlink_broadcast() | |
> > > > > > ... | |
> > > > > > call_netdevice_notifiers(NETDEV_REGISTER) | |
> > > > > > failover_event(..., NETDEV_REGISTER, ...) | |
> > > > > > failover_slave_register(ixgbevf_netdev) | |
> > > > > > net_failover_slave_register(ixgbevf_netdev) | |
> > > > > > dev_open(ixgbevf_netdev) | |
> > > > > > | |
> > > > > > | |
> > > > > > | | received ADD uevent from netlink fd
> > > > > > | | ...
> > > > > > | | udev-builtin-net_id.c:dev_pci_slot()
> > > > > > | | (decided to renamed 'eth0' )
> > > > > > | | ip link set dev eth0 name ens4
> > > > > > (dev_change_name() returns -EBUSY as | |
> > > > > > ixgbevf_netdev->flags has IFF_UP) | |
> > > > > > | |
> > > > > >
> > > > > Given renaming slaves does not work anyway:
> > > > I was actually thinking what if we relieve the rename restriction just for
> > > > the failover slave? What the impact would be? I think users don't care about
> > > > slave being renamed when it's in use, especially the initial rename.
> > > > Thoughts?
> > > >
> > > > > would it work if we just
> > > > > hard-coded slave names instead?
> > > > >
> > > > > E.g.
> > > > > 1. fail slave renames
> > > > > 2. rename of failover to XX automatically renames standby to XXnsby
> > > > > and primary to XXnpry
> > > > That wouldn't help. The time when the failover master gets renamed, the VF
> > > > may not be present.
> > > In this scheme if VF is not there it will be renamed immediately after registration.
> > Who will be responsible to rename the slave, the kernel?
>
> That's the idea.
>
> > Note the master's
> > name may or may not come from the userspace. If it comes from the userspace,
> > should the userspace daemon change their expectation not to name/rename
> > _any_ slaves (today there's no distinction)?
>
> Yes the idea would be to fail renaming slaves.
>
> > How do users know which name to
> > trust, depending on which wins the race more often? Say if kernel wants a
> > ens3npry name while userspace wants it named as ens4.
> >
> > -Siwei
>
> With this approach kernel will deny attempts by userspace to rename
> slaves. Slaves will always be named XXXnsby and XXnpry. Master renames
> will rename both slaves.
>
> It seems pretty solid to me, the only issue is that in theory userspace
> can use a name like XXXnsby for something else. But this seems unlikely.
Similar schemes (with kernel providing naming) were also previously rejected
upstream. It has been a consistent theme that the kernel should not be in
the renaming business. It will certainly break userspace.
^ permalink raw reply
* Regression with "arm64: KVM: Skip MMIO insn after emulation" on 4.4 stable
From: Daniel Verkamp @ 2019-02-28 0:36 UTC (permalink / raw)
To: virtualization, stable; +Cc: Mark Rutland, Marc Zyngier
Hello,
In my testing of crosvm[1] with Linux 4.4.175, I am observing failures
on a 'kevin' Chromebook (RK3399) device - the guest kernel does not
even get to the point of printing its first messages, and the host
seems to be spinning at 100% CPU in KVM_RUN.
I narrowed this down to the 4.4 stable backport of "arm64: KVM: Skip
MMIO insn after emulation" - with this patch reverted, I can boot the
guest kernel as normal again.
Unfortunately, I am unable to easily test with a newer upstream kernel
(this board is using the Chrome OS kernel with many additional patches
applied on top of 4.4), so I'm not sure if this issue was introduced
in the mainline commit or only in the stable branch. Is it possible
that this patch has other dependencies that were missed in the
backport? It looks like it was part of a larger series, but only this
patch got pulled for 4.4 stable.
Thanks,
-- Daniel
[1]: https://chromium.googlesource.com/chromiumos/platform/crosvm/
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 0:38 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Samudrala, Sridhar, virtualization, Siwei Liu, liran.alon, Netdev,
si-wei liu, David Miller
In-Reply-To: <20190227160342.788dc2b4@shemminger-XPS-13-9360>
On Wed, Feb 27, 2019 at 04:03:42PM -0800, Stephen Hemminger wrote:
> > With this approach kernel will deny attempts by userspace to rename
> > slaves. Slaves will always be named XXXnsby and XXnpry. Master renames
> > will rename both slaves.
> >
> > It seems pretty solid to me, the only issue is that in theory userspace
> > can use a name like XXXnsby for something else. But this seems unlikely.
>
> Similar schemes (with kernel providing naming) were also previously rejected
> upstream.
Links?
I'm inclined to try and see what happens.
> It has been a consistent theme that the kernel should not be in
> the renaming business.
In this case it's not in renaming business per se. The only reason
we even have the original name is due to the ways internal APIs
work. You can look at it as simply having slaves names being
part of master.
> It will certainly break userspace.
That's a strong claim. What is it based on? It so happens that
userspace renaming slaves is already broken on virtio. So we can fix it
any way we like :)
And yes it won't help netvsc because netvsc wants compatibility with old
scripts but then netvsc uses a 2 device model anyway.
--
MST
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 0:41 UTC (permalink / raw)
To: si-wei liu
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Samudrala, Sridhar, virtualization, Siwei Liu, liran.alon, Netdev,
David Miller
In-Reply-To: <a617ce13-4114-469d-ef33-a1c91150eeca@oracle.com>
On Wed, Feb 27, 2019 at 04:38:00PM -0800, si-wei liu wrote:
>
>
> On 2/27/2019 3:50 PM, Michael S. Tsirkin wrote:
> > On Wed, Feb 27, 2019 at 03:34:56PM -0800, si-wei liu wrote:
> > >
> > > On 2/27/2019 2:38 PM, Michael S. Tsirkin wrote:
> > > > On Tue, Feb 26, 2019 at 04:17:21PM -0800, si-wei liu wrote:
> > > > > On 2/25/2019 6:08 PM, Michael S. Tsirkin wrote:
> > > > > > On Mon, Feb 25, 2019 at 04:58:07PM -0800, si-wei liu wrote:
> > > > > > > On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
> > > > > > > > On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:
> > > > > > > > > On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
> > > > > > > > > > On 2/21/2019 7:33 PM, si-wei liu wrote:
> > > > > > > > > > > On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
> > > > > > > > > > > > On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
> > > > > > > > > > > > > Sorry for replying to this ancient thread. There was some remaining
> > > > > > > > > > > > > issue that I don't think the initial net_failover patch got addressed
> > > > > > > > > > > > > cleanly, see:
> > > > > > > > > > > > >
> > > > > > > > > > > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
> > > > > > > > > > > > >
> > > > > > > > > > > > > The renaming of 'eth0' to 'ens4' fails because the udev userspace was
> > > > > > > > > > > > > not specifically writtten for such kernel automatic enslavement.
> > > > > > > > > > > > > Specifically, if it is a bond or team, the slave would typically get
> > > > > > > > > > > > > renamed *before* virtual device gets created, that's what udev can
> > > > > > > > > > > > > control (without getting netdev opened early by the other part of
> > > > > > > > > > > > > kernel) and other userspace components for e.g. initramfs,
> > > > > > > > > > > > > init-scripts can coordinate well in between. The in-kernel
> > > > > > > > > > > > > auto-enslavement of net_failover breaks this userspace convention,
> > > > > > > > > > > > > which don't provides a solution if user care about consistent naming
> > > > > > > > > > > > > on the slave netdevs specifically.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Previously this issue had been specifically called out when IFF_HIDDEN
> > > > > > > > > > > > > and the 1-netdev was proposed, but no one gives out a solution to this
> > > > > > > > > > > > > problem ever since. Please share your mind how to proceed and solve
> > > > > > > > > > > > > this userspace issue if netdev does not welcome a 1-netdev model.
> > > > > > > > > > > > Above says:
> > > > > > > > > > > >
> > > > > > > > > > > > there's no motivation in the systemd/udevd community at
> > > > > > > > > > > > this point to refactor the rename logic and make it work well with
> > > > > > > > > > > > 3-netdev.
> > > > > > > > > > > >
> > > > > > > > > > > > What would the fix be? Skip slave devices?
> > > > > > > > > > > >
> > > > > > > > > > > There's nothing user can get if just skipping slave devices - the
> > > > > > > > > > > name is still unchanged and unpredictable e.g. eth0, or eth1 the
> > > > > > > > > > > next reboot, while the rest may conform to the naming scheme (ens3
> > > > > > > > > > > and such). There's no way one can fix this in userspace alone - when
> > > > > > > > > > > the failover is created the enslaved netdev was opened by the kernel
> > > > > > > > > > > earlier than the userspace is made aware of, and there's no
> > > > > > > > > > > negotiation protocol for kernel to know when userspace has done
> > > > > > > > > > > initial renaming of the interface. I would expect netdev list should
> > > > > > > > > > > at least provide the direction in general for how this can be
> > > > > > > > > > > solved...
> > > > > > > > I was just wondering what did you mean when you said
> > > > > > > > "refactor the rename logic and make it work well with 3-netdev" -
> > > > > > > > was there a proposal udev rejected?
> > > > > > > No. I never believed this particular issue can be fixed in userspace alone.
> > > > > > > Previously someone had said it could be, but I never see any work or
> > > > > > > relevant discussion ever happened in various userspace communities (for e.g.
> > > > > > > dracut, initramfs-tools, systemd, udev, and NetworkManager). IMHO the root
> > > > > > > of the issue derives from the kernel, it makes more sense to start from
> > > > > > > netdev, work out and decide on a solution: see what can be done in the
> > > > > > > kernel in order to fix it, then after that engage userspace community for
> > > > > > > the feasibility...
> > > > > > >
> > > > > > > > Anyway, can we write a time diagram for what happens in which order that
> > > > > > > > leads to failure? That would help look for triggers that we can tie
> > > > > > > > into, or add new ones.
> > > > > > > >
> > > > > > > See attached diagram.
> > > > > > >
> > > > > > > >
> > > > > > > > > > Is there an issue if slave device names are not predictable? The user/admin scripts are expected
> > > > > > > > > > to only work with the master failover device.
> > > > > > > > > Where does this expectation come from?
> > > > > > > > >
> > > > > > > > > Admin users may have ethtool or tc configurations that need to deal with
> > > > > > > > > predictable interface name. Third-party app which was built upon specifying
> > > > > > > > > certain interface name can't be modified to chase dynamic names.
> > > > > > > > >
> > > > > > > > > Specifically, we have pre-canned image that uses ethtool to fine tune VF
> > > > > > > > > offload settings post boot for specific workload. Those images won't work
> > > > > > > > > well if the name is constantly changing just after couple rounds of live
> > > > > > > > > migration.
> > > > > > > > It should be possible to specify the ethtool configuration on the
> > > > > > > > master and have it automatically propagated to the slave.
> > > > > > > >
> > > > > > > > BTW this is something we should look at IMHO.
> > > > > > > I was elaborating a few examples that the expectation and assumption that
> > > > > > > user/admin scripts only deal with master failover device is incorrect. It
> > > > > > > had never been taken good care of, although I did try to emphasize it from
> > > > > > > the very beginning.
> > > > > > >
> > > > > > > Basically what you said about propagating the ethtool configuration down to
> > > > > > > the slave is the key pursuance of 1-netdev model. However, what I am seeking
> > > > > > > now is any alternative that can also fix the specific udev rename problem,
> > > > > > > before concluding that 1-netdev is the only solution. Generally a 1-netdev
> > > > > > > scheme would take time to implement, while I'm trying to find a way out to
> > > > > > > fix this particular naming problem under 3-netdev.
> > > > > > >
> > > > > > > > > > Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> > > > > > > > > > about moving them to a hidden network namespace so that they are not visible from the default namespace.
> > > > > > > > > > I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> > > > > > > > > > kernel. If so, we could use this mechanism to simulate a 1-netdev model.
> > > > > > > > > Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> > > > > > > > > model as much transparent to a real NIC as possible, while a hidden netns is
> > > > > > > > > just the vehicle). However, I recall there was resistance around this
> > > > > > > > > discussion that even the concept of hiding itself is a taboo for Linux
> > > > > > > > > netdev. I would like to summon potential alternatives before concluding
> > > > > > > > > 1-netdev is the only solution too soon.
> > > > > > > > >
> > > > > > > > > Thanks,
> > > > > > > > > -Siwei
> > > > > > > > Your scripts would not work at all then, right?
> > > > > > > At this point we don't claim images with such usage as SR-IOV live
> > > > > > > migrate-able. We would flag it as live migrate-able until this ethtool
> > > > > > > config issue is fully addressed and a transparent live migration solution
> > > > > > > emerges in upstream eventually.
> > > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > > -Siwei
> > > > > > > > > > > -Siwei
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > ---------------------------------------------------------------------
> > > > > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > > > > > >
> > > > > > > net_failover(kernel) | network.service (user) | systemd-udevd (user)
> > > > > > > --------------------------------------------------+------------------------------+--------------------------------------------
> > > > > > > (standby virtio-net and net_failover | |
> > > > > > > devices created and initialized, | |
> > > > > > > i.e. virtnet_probe()-> | |
> > > > > > > net_failover_create() | |
> > > > > > > was done.) | |
> > > > > > > | |
> > > > > > > | runs `ifup ens3' -> |
> > > > > > > | ip link set dev ens3 up |
> > > > > > > net_failover_open() | |
> > > > > > > dev_open(virtnet_dev) | |
> > > > > > > virtnet_open(virtnet_dev) | |
> > > > > > > netif_carrier_on(failover_dev) | |
> > > > > > > ... | |
> > > > > > > | |
> > > > > > > (VF hot plugged in) | |
> > > > > > > ixgbevf_probe() | |
> > > > > > > register_netdev(ixgbevf_netdev) | |
> > > > > > > netdev_register_kobject(ixgbevf_netdev) | |
> > > > > > > kobject_add(ixgbevf_dev) | |
> > > > > > > device_add(ixgbevf_dev) | |
> > > > > > > kobject_uevent(&ixgbevf_dev->kobj, KOBJ_ADD) | |
> > > > > > > netlink_broadcast() | |
> > > > > > > ... | |
> > > > > > > call_netdevice_notifiers(NETDEV_REGISTER) | |
> > > > > > > failover_event(..., NETDEV_REGISTER, ...) | |
> > > > > > > failover_slave_register(ixgbevf_netdev) | |
> > > > > > > net_failover_slave_register(ixgbevf_netdev) | |
> > > > > > > dev_open(ixgbevf_netdev) | |
> > > > > > > | |
> > > > > > > | |
> > > > > > > | | received ADD uevent from netlink fd
> > > > > > > | | ...
> > > > > > > | | udev-builtin-net_id.c:dev_pci_slot()
> > > > > > > | | (decided to renamed 'eth0' )
> > > > > > > | | ip link set dev eth0 name ens4
> > > > > > > (dev_change_name() returns -EBUSY as | |
> > > > > > > ixgbevf_netdev->flags has IFF_UP) | |
> > > > > > > | |
> > > > > > >
> > > > > > Given renaming slaves does not work anyway:
> > > > > I was actually thinking what if we relieve the rename restriction just for
> > > > > the failover slave? What the impact would be? I think users don't care about
> > > > > slave being renamed when it's in use, especially the initial rename.
> > > > > Thoughts?
> > > > >
> > > > > > would it work if we just
> > > > > > hard-coded slave names instead?
> > > > > >
> > > > > > E.g.
> > > > > > 1. fail slave renames
> > > > > > 2. rename of failover to XX automatically renames standby to XXnsby
> > > > > > and primary to XXnpry
> > > > > That wouldn't help. The time when the failover master gets renamed, the VF
> > > > > may not be present.
> > > > In this scheme if VF is not there it will be renamed immediately after registration.
> > > Who will be responsible to rename the slave, the kernel?
> > That's the idea.
> >
> > > Note the master's
> > > name may or may not come from the userspace. If it comes from the userspace,
> > > should the userspace daemon change their expectation not to name/rename
> > > _any_ slaves (today there's no distinction)?
> > Yes the idea would be to fail renaming slaves.
> No I was asking about the userspace expectation: whether it should track and
> detect the lifecycle events of failover slaves and decide what to do. How
> does it get back to the user specified name if VF is not enslaved (say
> someone unloads the virtio-net module)?
When virtio net is removed VF will shortly be removed too.
> As this scheme adds much complexity to the kernel naming convention
> (currently it's just ethX names) that no userspace can understand.
Anything that pokes at slaves needs to be specially designed anyway.
Naming seems like a minor issue.
> Will the
> change break userspace further?
>
> -Siwei
Didn't you show userspace is already broken. You can't "further
break it", rename already fails.
> >
> > > How do users know which name to
> > > trust, depending on which wins the race more often? Say if kernel wants a
> > > ens3npry name while userspace wants it named as ens4.
> > >
> > > -Siwei
> > With this approach kernel will deny attempts by userspace to rename
> > slaves. Slaves will always be named XXXnsby and XXnpry. Master renames
> > will rename both slaves.
> >
> > It seems pretty solid to me, the only issue is that in theory userspace
> > can use a name like XXXnsby for something else. But this seems unlikely.
> >
> >
> > > > > I don't like the idea to delay exposing failover master
> > > > > until VF is hot plugged in (probably subject to various failures) later.
> > > > >
> > > > > Thanks,
> > > > > -Siwei
> > > > I agree, this was not what I meant.
> > > >
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Jakub Kicinski @ 2019-02-28 0:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Samudrala, Sridhar,
virtualization, Siwei Liu, liran.alon, Netdev, si-wei liu,
David Miller
In-Reply-To: <20190227193923-mutt-send-email-mst@kernel.org>
On Wed, 27 Feb 2019 19:41:32 -0500, Michael S. Tsirkin wrote:
> > As this scheme adds much complexity to the kernel naming convention
> > (currently it's just ethX names) that no userspace can understand.
>
> Anything that pokes at slaves needs to be specially designed anyway.
> Naming seems like a minor issue.
Can the users who care about the naming put net_failover into
"user space will do the bond enslavement" mode, and do the bond
creation/management themselves from user space (in systemd/
Network Manager) based on the failover flag?
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 1:26 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Samudrala, Sridhar,
virtualization, Siwei Liu, liran.alon, Netdev, si-wei liu,
David Miller
In-Reply-To: <20190227165205.307ed83c@cakuba.netronome.com>
On Wed, Feb 27, 2019 at 04:52:05PM -0800, Jakub Kicinski wrote:
> On Wed, 27 Feb 2019 19:41:32 -0500, Michael S. Tsirkin wrote:
> > > As this scheme adds much complexity to the kernel naming convention
> > > (currently it's just ethX names) that no userspace can understand.
> >
> > Anything that pokes at slaves needs to be specially designed anyway.
> > Naming seems like a minor issue.
>
> Can the users who care about the naming put net_failover into
> "user space will do the bond enslavement" mode, and do the bond
> creation/management themselves from user space (in systemd/
> Network Manager) based on the failover flag?
Putting issues of compatibility aside (userspace tends to be confused if
you give it two devices with same MAC), how would you have it work in
practice? Timer based hacks like netvsc where if userspace didn't
respond within X seconds we assume it won't and do everything ourselves?
--
MST
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Jakub Kicinski @ 2019-02-28 1:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Samudrala, Sridhar,
virtualization, Siwei Liu, liran.alon, Netdev, si-wei liu,
David Miller
In-Reply-To: <20190227201857-mutt-send-email-mst@kernel.org>
On Wed, 27 Feb 2019 20:26:02 -0500, Michael S. Tsirkin wrote:
> On Wed, Feb 27, 2019 at 04:52:05PM -0800, Jakub Kicinski wrote:
> > On Wed, 27 Feb 2019 19:41:32 -0500, Michael S. Tsirkin wrote:
> > > > As this scheme adds much complexity to the kernel naming convention
> > > > (currently it's just ethX names) that no userspace can understand.
> > >
> > > Anything that pokes at slaves needs to be specially designed anyway.
> > > Naming seems like a minor issue.
> >
> > Can the users who care about the naming put net_failover into
> > "user space will do the bond enslavement" mode, and do the bond
> > creation/management themselves from user space (in systemd/
> > Network Manager) based on the failover flag?
>
> Putting issues of compatibility aside (userspace tends to be confused if
> you give it two devices with same MAC), how would you have it work in
> practice? Timer based hacks like netvsc where if userspace didn't
> respond within X seconds we assume it won't and do everything ourselves?
Well, what I'm saying is basically if user space knows how to deal with
the auto-bonding, we can put aside net_failover for the most part. It
can either be blacklisted or it can have some knob which will
effectively disable the auto-enslavement.
Auto-bonding capable user space can do the renames, spawn the bond,
etc. all by itself. I'm basically going back to my initial proposal
here :) There is a RedHat bugzilla for the NetworkManager team to do
this, but we merged net_failover before those folks got around to
implementing it.
IOW if NM/systemd is capable of doing the auto-bonding itself it can
disable the kernel mechanism and take care of it all. If kernel is
booted with an old user space which doesn't have capable NM/systemd -
net_failover will kick in and do its best.
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 4:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Samudrala, Sridhar,
virtualization, Siwei Liu, liran.alon, Netdev, si-wei liu,
David Miller
In-Reply-To: <20190227175218.736e13b6@cakuba.netronome.com>
On Wed, Feb 27, 2019 at 05:52:18PM -0800, Jakub Kicinski wrote:
> On Wed, 27 Feb 2019 20:26:02 -0500, Michael S. Tsirkin wrote:
> > On Wed, Feb 27, 2019 at 04:52:05PM -0800, Jakub Kicinski wrote:
> > > On Wed, 27 Feb 2019 19:41:32 -0500, Michael S. Tsirkin wrote:
> > > > > As this scheme adds much complexity to the kernel naming convention
> > > > > (currently it's just ethX names) that no userspace can understand.
> > > >
> > > > Anything that pokes at slaves needs to be specially designed anyway.
> > > > Naming seems like a minor issue.
> > >
> > > Can the users who care about the naming put net_failover into
> > > "user space will do the bond enslavement" mode, and do the bond
> > > creation/management themselves from user space (in systemd/
> > > Network Manager) based on the failover flag?
> >
> > Putting issues of compatibility aside (userspace tends to be confused if
> > you give it two devices with same MAC), how would you have it work in
> > practice? Timer based hacks like netvsc where if userspace didn't
> > respond within X seconds we assume it won't and do everything ourselves?
>
> Well, what I'm saying is basically if user space knows how to deal with
> the auto-bonding, we can put aside net_failover for the most part. It
> can either be blacklisted or it can have some knob which will
> effectively disable the auto-enslavement.
OK I guess we could add a module parameter to skip this.
Is this what you mean?
> Auto-bonding capable user space can do the renames, spawn the bond,
> etc. all by itself. I'm basically going back to my initial proposal
> here :) There is a RedHat bugzilla for the NetworkManager team to do
> this, but we merged net_failover before those folks got around to
> implementing it.
In particular because there's no policy involved whatsoever
here so it's just mechanism being pushed up to userspace.
> IOW if NM/systemd is capable of doing the auto-bonding itself it can
> disable the kernel mechanism and take care of it all. If kernel is
> booted with an old user space which doesn't have capable NM/systemd -
> net_failover will kick in and do its best.
Sure - it's just 2 lines of code, see below.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
But I don't intend to bother until there's actual interest from
userspace developers to bother. In particular it is not just NM/systemd
even on Fedora - e.g. you will need to teach dracut to somehow detect
and handle this - right now it gets confused if there are two devices
with same MAC addresses.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 955b3e76eb8d..dd2b2c370003 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -43,6 +43,7 @@ static bool csum = true, gso = true, napi_tx;
module_param(csum, bool, 0444);
module_param(gso, bool, 0444);
module_param(napi_tx, bool, 0644);
+module_param(disable_failover, bool, 0644);
/* FIXME: MTU in config. */
#define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
@@ -3163,6 +3164,7 @@ static int virtnet_probe(struct virtio_device *vdev)
virtnet_init_settings(dev);
- if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY) &&
+ !disable_failover) {
vi->failover = net_failover_create(vi->dev);
if (IS_ERR(vi->failover)) {
err = PTR_ERR(vi->failover);
^ permalink raw reply related
* Re: Regression with "arm64: KVM: Skip MMIO insn after emulation" on 4.4 stable
From: Greg KH @ 2019-02-28 8:16 UTC (permalink / raw)
To: Daniel Verkamp; +Cc: Mark Rutland, Marc Zyngier, stable, virtualization
In-Reply-To: <CABVzXAnm78j6mwbNF2MWTwkBcXVmCdWYdZfXJBhiThOEK3135w@mail.gmail.com>
On Wed, Feb 27, 2019 at 04:36:39PM -0800, Daniel Verkamp wrote:
> Hello,
>
> In my testing of crosvm[1] with Linux 4.4.175, I am observing failures
> on a 'kevin' Chromebook (RK3399) device - the guest kernel does not
> even get to the point of printing its first messages, and the host
> seems to be spinning at 100% CPU in KVM_RUN.
>
> I narrowed this down to the 4.4 stable backport of "arm64: KVM: Skip
> MMIO insn after emulation" - with this patch reverted, I can boot the
> guest kernel as normal again.
>
> Unfortunately, I am unable to easily test with a newer upstream kernel
> (this board is using the Chrome OS kernel with many additional patches
> applied on top of 4.4), so I'm not sure if this issue was introduced
> in the mainline commit or only in the stable branch. Is it possible
> that this patch has other dependencies that were missed in the
> backport? It looks like it was part of a larger series, but only this
> patch got pulled for 4.4 stable.
It looks like this got pulled in by Sasha's bot which is why it was
applied.
I have no objection to reverting this if it is causing problems, unless
someone here really thinks it needs to remain in the tree?
thanks,
greg k-h
^ permalink raw reply
* Re: Regression with "arm64: KVM: Skip MMIO insn after emulation" on 4.4 stable
From: Marc Zyngier @ 2019-02-28 8:49 UTC (permalink / raw)
To: Greg KH; +Cc: Mark Rutland, stable, virtualization
In-Reply-To: <20190228081605.GB865@kroah.com>
On Thu, 28 Feb 2019 08:16:05 +0000,
Greg KH <gregkh@linuxfoundation.org> wrote:
Hi both,
>
> On Wed, Feb 27, 2019 at 04:36:39PM -0800, Daniel Verkamp wrote:
> > Hello,
> >
> > In my testing of crosvm[1] with Linux 4.4.175, I am observing failures
> > on a 'kevin' Chromebook (RK3399) device - the guest kernel does not
> > even get to the point of printing its first messages, and the host
> > seems to be spinning at 100% CPU in KVM_RUN.
> >
> > I narrowed this down to the 4.4 stable backport of "arm64: KVM: Skip
> > MMIO insn after emulation" - with this patch reverted, I can boot the
> > guest kernel as normal again.
> >
> > Unfortunately, I am unable to easily test with a newer upstream kernel
> > (this board is using the Chrome OS kernel with many additional patches
> > applied on top of 4.4), so I'm not sure if this issue was introduced
> > in the mainline commit or only in the stable branch. Is it possible
> > that this patch has other dependencies that were missed in the
> > backport? It looks like it was part of a larger series, but only this
> > patch got pulled for 4.4 stable.
Thanks for reporting this. I'll have a look ASAP.
> It looks like this got pulled in by Sasha's bot which is why it was
> applied.
>
> I have no objection to reverting this if it is causing problems, unless
> someone here really thinks it needs to remain in the tree?
Can we hold on a tiny bit so that I can verify we haven't regressed
crosvm all the way into mainline?
Daniel, is there any chance you could test with kvmtool as well on
your setup? You can easily compile it as a static binary and run it
from ChromeOS with:
lkvm-static run -c 1 -p "earlycon=uart,mmio,0x3f8" -l Image
which should start a UP guest and drop you to a shell. In the
meantime, I'm starting to build crosvm on my kevin running mainline.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* Re: [PATCH 2/2] drm/bochs: Drop best_encoder
From: Gerd Hoffmann @ 2019-02-28 8:55 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Daniel Vetter, DRI Development, virtualization
In-Reply-To: <20190221155857.19773-2-daniel.vetter@ffwll.ch>
On Thu, Feb 21, 2019 at 04:58:57PM +0100, Daniel Vetter wrote:
> This is the default for atomic drivers.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> ---
> drivers/gpu/drm/bochs/bochs_kms.c | 11 -----------
> 1 file changed, 11 deletions(-)
Pushed to drm-misc-next.
thanks,
Gerd
^ permalink raw reply
* Re: [PATCH 2/2] drm/bochs: Drop best_encoder
From: Daniel Vetter @ 2019-02-28 9:32 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Daniel Vetter, virtualization, DRI Development, Daniel Vetter
In-Reply-To: <20190228085522.mkf6tcueygihyjht@sirius.home.kraxel.org>
On Thu, Feb 28, 2019 at 09:55:22AM +0100, Gerd Hoffmann wrote:
> On Thu, Feb 21, 2019 at 04:58:57PM +0100, Daniel Vetter wrote:
> > This is the default for atomic drivers.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: virtualization@lists.linux-foundation.org
> > ---
> > drivers/gpu/drm/bochs/bochs_kms.c | 11 -----------
> > 1 file changed, 11 deletions(-)
>
> Pushed to drm-misc-next.
Thanks. r-b on the first patch too so I can pull that in?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: Regression with "arm64: KVM: Skip MMIO insn after emulation" on 4.4 stable
From: Marc Zyngier @ 2019-02-28 11:13 UTC (permalink / raw)
To: Greg KH, Daniel Verkamp; +Cc: Mark Rutland, stable, virtualization
In-Reply-To: <86r2bswb5e.wl-marc.zyngier@arm.com>
On 28/02/2019 08:49, Marc Zyngier wrote:
> On Thu, 28 Feb 2019 08:16:05 +0000,
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
> Hi both,
>
>>
>> On Wed, Feb 27, 2019 at 04:36:39PM -0800, Daniel Verkamp wrote:
>>> Hello,
>>>
>>> In my testing of crosvm[1] with Linux 4.4.175, I am observing failures
>>> on a 'kevin' Chromebook (RK3399) device - the guest kernel does not
>>> even get to the point of printing its first messages, and the host
>>> seems to be spinning at 100% CPU in KVM_RUN.
>>>
>>> I narrowed this down to the 4.4 stable backport of "arm64: KVM: Skip
>>> MMIO insn after emulation" - with this patch reverted, I can boot the
>>> guest kernel as normal again.
>>>
>>> Unfortunately, I am unable to easily test with a newer upstream kernel
>>> (this board is using the Chrome OS kernel with many additional patches
>>> applied on top of 4.4), so I'm not sure if this issue was introduced
>>> in the mainline commit or only in the stable branch. Is it possible
>>> that this patch has other dependencies that were missed in the
>>> backport? It looks like it was part of a larger series, but only this
>>> patch got pulled for 4.4 stable.
>
> Thanks for reporting this. I'll have a look ASAP.
>
>> It looks like this got pulled in by Sasha's bot which is why it was
>> applied.
>>
>> I have no objection to reverting this if it is causing problems, unless
>> someone here really thinks it needs to remain in the tree?
>
> Can we hold on a tiny bit so that I can verify we haven't regressed
> crosvm all the way into mainline?
>
> Daniel, is there any chance you could test with kvmtool as well on
> your setup? You can easily compile it as a static binary and run it
> from ChromeOS with:
>
> lkvm-static run -c 1 -p "earlycon=uart,mmio,0x3f8" -l Image
>
> which should start a UP guest and drop you to a shell. In the
> meantime, I'm starting to build crosvm on my kevin running mainline.
OK, that wasn't a fun experience. Some build instructions for the mere
mortals (aka non-ChromeOS people) would be nice (and a mention that
crosvm cannot yet deal with GICv3-only systems...)
Anyway, I've got enough of it working that I can boot a trivial Debian
guest on kevin running 5.0-rc7, so I'm pretty confident that mainline is
OK. I'll move back to 4.4, and try to understand what happens there.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: Regression with "arm64: KVM: Skip MMIO insn after emulation" on 4.4 stable
From: Marc Zyngier @ 2019-02-28 12:19 UTC (permalink / raw)
To: Greg KH, Daniel Verkamp; +Cc: Mark Rutland, stable, virtualization
In-Reply-To: <6dd1026e-5310-0b41-2e90-716120180956@arm.com>
On 28/02/2019 11:13, Marc Zyngier wrote:
> On 28/02/2019 08:49, Marc Zyngier wrote:
>> On Thu, 28 Feb 2019 08:16:05 +0000,
>> Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> Hi both,
>>
>>>
>>> On Wed, Feb 27, 2019 at 04:36:39PM -0800, Daniel Verkamp wrote:
>>>> Hello,
>>>>
>>>> In my testing of crosvm[1] with Linux 4.4.175, I am observing failures
>>>> on a 'kevin' Chromebook (RK3399) device - the guest kernel does not
>>>> even get to the point of printing its first messages, and the host
>>>> seems to be spinning at 100% CPU in KVM_RUN.
>>>>
>>>> I narrowed this down to the 4.4 stable backport of "arm64: KVM: Skip
>>>> MMIO insn after emulation" - with this patch reverted, I can boot the
>>>> guest kernel as normal again.
>>>>
>>>> Unfortunately, I am unable to easily test with a newer upstream kernel
>>>> (this board is using the Chrome OS kernel with many additional patches
>>>> applied on top of 4.4), so I'm not sure if this issue was introduced
>>>> in the mainline commit or only in the stable branch. Is it possible
>>>> that this patch has other dependencies that were missed in the
>>>> backport? It looks like it was part of a larger series, but only this
>>>> patch got pulled for 4.4 stable.
>>
>> Thanks for reporting this. I'll have a look ASAP.
>>
>>> It looks like this got pulled in by Sasha's bot which is why it was
>>> applied.
>>>
>>> I have no objection to reverting this if it is causing problems, unless
>>> someone here really thinks it needs to remain in the tree?
>>
>> Can we hold on a tiny bit so that I can verify we haven't regressed
>> crosvm all the way into mainline?
>>
>> Daniel, is there any chance you could test with kvmtool as well on
>> your setup? You can easily compile it as a static binary and run it
>> from ChromeOS with:
>>
>> lkvm-static run -c 1 -p "earlycon=uart,mmio,0x3f8" -l Image
>>
>> which should start a UP guest and drop you to a shell. In the
>> meantime, I'm starting to build crosvm on my kevin running mainline.
>
> OK, that wasn't a fun experience. Some build instructions for the mere
> mortals (aka non-ChromeOS people) would be nice (and a mention that
> crosvm cannot yet deal with GICv3-only systems...)
>
> Anyway, I've got enough of it working that I can boot a trivial Debian
> guest on kevin running 5.0-rc7, so I'm pretty confident that mainline is
> OK. I'll move back to 4.4, and try to understand what happens there.
I got 4.4.175 running on an AMD Seattle, and this patch broke more than
just crosvm. It actually breaks the whole KVM in-kernel device emulation
which can't make forward progress (it is forever stuck on the same
instruction, writing to the emulated GIC). Both QEMU and kvmtool are
also affected. Irk. :-(
It turns out 4.4.175 is missing two patches on which $SUBJECT relies:
83091db981e1 KVM: arm/arm64: Fix MMIO emulation data handling
1d6a821277aa arm/arm64: KVM: Feed initialized memory to MMIO accesses
which are implicit dependencies for it. With these two patches
cherry-picked, I get both kvmtool and crosvm working.
Greg: I'll send these two patches backported to 4.4-stable ASAP, so that
we don't have to revert anything.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 14:26 UTC (permalink / raw)
To: si-wei liu
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Samudrala, Sridhar, virtualization, Siwei Liu, liran.alon, Netdev,
David Miller
In-Reply-To: <36901346-e3d5-4e51-6a8d-678eb5b9e352@oracle.com>
On Thu, Feb 28, 2019 at 01:32:12AM -0800, si-wei liu wrote:
> > > Will the
> > > change break userspace further?
> > >
> > > -Siwei
> > Didn't you show userspace is already broken. You can't "further
> > break it", rename already fails.
> It's a race, userspace tends to give slave a user(space) desired name but
> sometimes may fail due to this race. Today if failover master is not up,
> rename would succeed anyway. While what you proposed prohibits user from
> providing a name in all circumstances if I understand you correctly. That's
> what I meant of breaking userspace further. On the other hand, you seem to
> tighten the kernel default naming to udev predictable names, which is
> derived from only recent systemd-udevd, while there exists many possible
> userspace naming schemes out of that. Users today who deliberately chooses
> to disable predictable naming (net.ifnames=0 biosdevname=0) and fall back to
> kernel provided names would expect the ethX pattern, with this change
> admin/user scripts which matches the ethX pattern could potentially break.
Whatever crashes with a name not matching ethX will crash on the
standby interface *anyway*.
So I think what you are saying is that someone might have already
written scripts and gotten them to work on v4.17 when STANDBY was
included and these scripts rely on ethX. Now these scripts
will break.
Maybe it is still early enough (just half a year passed) that the
number of these users would be small. So how about a kernel config
option and maybe a module parameter to rename the primary? People can
then opt in to the old broken behaviour.
--
MST
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Jakub Kicinski @ 2019-02-28 18:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Samudrala, Sridhar,
virtualization, Siwei Liu, liran.alon, Netdev, si-wei liu,
David Miller
In-Reply-To: <20190227233812-mutt-send-email-mst@kernel.org>
On Wed, 27 Feb 2019 23:47:33 -0500, Michael S. Tsirkin wrote:
> On Wed, Feb 27, 2019 at 05:52:18PM -0800, Jakub Kicinski wrote:
> > > > Can the users who care about the naming put net_failover into
> > > > "user space will do the bond enslavement" mode, and do the bond
> > > > creation/management themselves from user space (in systemd/
> > > > Network Manager) based on the failover flag?
> > >
> > > Putting issues of compatibility aside (userspace tends to be confused if
> > > you give it two devices with same MAC), how would you have it work in
> > > practice? Timer based hacks like netvsc where if userspace didn't
> > > respond within X seconds we assume it won't and do everything ourselves?
> >
> > Well, what I'm saying is basically if user space knows how to deal with
> > the auto-bonding, we can put aside net_failover for the most part. It
> > can either be blacklisted or it can have some knob which will
> > effectively disable the auto-enslavement.
>
> OK I guess we could add a module parameter to skip this.
> Is this what you mean?
Yup.
> > Auto-bonding capable user space can do the renames, spawn the bond,
> > etc. all by itself. I'm basically going back to my initial proposal
> > here :) There is a RedHat bugzilla for the NetworkManager team to do
> > this, but we merged net_failover before those folks got around to
> > implementing it.
>
> In particular because there's no policy involved whatsoever
> here so it's just mechanism being pushed up to userspace.
>
> > IOW if NM/systemd is capable of doing the auto-bonding itself it can
> > disable the kernel mechanism and take care of it all. If kernel is
> > booted with an old user space which doesn't have capable NM/systemd -
> > net_failover will kick in and do its best.
>
> Sure - it's just 2 lines of code, see below.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> But I don't intend to bother until there's actual interest from
> userspace developers to bother. In particular it is not just NM/systemd
> even on Fedora - e.g. you will need to teach dracut to somehow detect
> and handle this - right now it gets confused if there are two devices
> with same MAC addresses.
It is a bit of a the chicken or the egg situation ;) But users can
just blacklist, too. Anyway, I think this is far better than module
parameters for twiddling kernel-based interface naming policy.. :S
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 955b3e76eb8d..dd2b2c370003 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -43,6 +43,7 @@ static bool csum = true, gso = true, napi_tx;
> module_param(csum, bool, 0444);
> module_param(gso, bool, 0444);
> module_param(napi_tx, bool, 0644);
> +module_param(disable_failover, bool, 0644);
>
> /* FIXME: MTU in config. */
> #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> @@ -3163,6 +3164,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> virtnet_init_settings(dev);
>
> - if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY) &&
> + !disable_failover) {
> vi->failover = net_failover_create(vi->dev);
> if (IS_ERR(vi->failover)) {
> err = PTR_ERR(vi->failover);
>
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 19:36 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Samudrala, Sridhar,
virtualization, Siwei Liu, liran.alon, Netdev, si-wei liu,
David Miller
In-Reply-To: <20190228101356.39ac70aa@cakuba.netronome.com>
On Thu, Feb 28, 2019 at 10:13:56AM -0800, Jakub Kicinski wrote:
> On Wed, 27 Feb 2019 23:47:33 -0500, Michael S. Tsirkin wrote:
> > On Wed, Feb 27, 2019 at 05:52:18PM -0800, Jakub Kicinski wrote:
> > > > > Can the users who care about the naming put net_failover into
> > > > > "user space will do the bond enslavement" mode, and do the bond
> > > > > creation/management themselves from user space (in systemd/
> > > > > Network Manager) based on the failover flag?
> > > >
> > > > Putting issues of compatibility aside (userspace tends to be confused if
> > > > you give it two devices with same MAC), how would you have it work in
> > > > practice? Timer based hacks like netvsc where if userspace didn't
> > > > respond within X seconds we assume it won't and do everything ourselves?
> > >
> > > Well, what I'm saying is basically if user space knows how to deal with
> > > the auto-bonding, we can put aside net_failover for the most part. It
> > > can either be blacklisted or it can have some knob which will
> > > effectively disable the auto-enslavement.
> >
> > OK I guess we could add a module parameter to skip this.
> > Is this what you mean?
>
> Yup.
>
> > > Auto-bonding capable user space can do the renames, spawn the bond,
> > > etc. all by itself. I'm basically going back to my initial proposal
> > > here :) There is a RedHat bugzilla for the NetworkManager team to do
> > > this, but we merged net_failover before those folks got around to
> > > implementing it.
> >
> > In particular because there's no policy involved whatsoever
> > here so it's just mechanism being pushed up to userspace.
> >
> > > IOW if NM/systemd is capable of doing the auto-bonding itself it can
> > > disable the kernel mechanism and take care of it all. If kernel is
> > > booted with an old user space which doesn't have capable NM/systemd -
> > > net_failover will kick in and do its best.
> >
> > Sure - it's just 2 lines of code, see below.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > But I don't intend to bother until there's actual interest from
> > userspace developers to bother. In particular it is not just NM/systemd
> > even on Fedora - e.g. you will need to teach dracut to somehow detect
> > and handle this - right now it gets confused if there are two devices
> > with same MAC addresses.
>
> It is a bit of a the chicken or the egg situation ;) But users can
> just blacklist, too. Anyway, I think this is far better than module
> parameters
Sorry I'm a bit confused. What is better than what?
> for twiddling kernel-based interface naming policy.. :S
I see your point. But my point is slave names don't really matter, only
master name matters. So I am not sure there's any policy worth talking
about here.
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 955b3e76eb8d..dd2b2c370003 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -43,6 +43,7 @@ static bool csum = true, gso = true, napi_tx;
> > module_param(csum, bool, 0444);
> > module_param(gso, bool, 0444);
> > module_param(napi_tx, bool, 0644);
> > +module_param(disable_failover, bool, 0644);
> >
> > /* FIXME: MTU in config. */
> > #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> > @@ -3163,6 +3164,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > virtnet_init_settings(dev);
> >
> > - if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY) &&
> > + !disable_failover) {
> > vi->failover = net_failover_create(vi->dev);
> > if (IS_ERR(vi->failover)) {
> > err = PTR_ERR(vi->failover);
> >
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Jakub Kicinski @ 2019-02-28 19:56 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, Jiri Pirko, Samudrala, Sridhar, virtualization,
Siwei Liu, liran.alon, Netdev, si-wei liu, David Miller
In-Reply-To: <20190228143511-mutt-send-email-mst@kernel.org>
On Thu, 28 Feb 2019 14:36:56 -0500, Michael S. Tsirkin wrote:
> > It is a bit of a the chicken or the egg situation ;) But users can
> > just blacklist, too. Anyway, I think this is far better than module
> > parameters
>
> Sorry I'm a bit confused. What is better than what?
I mean that blacklist net_failover or module param to disable
net_failover and handle in user space are better than trying to solve
the renaming at kernel level (either by adding module params that make
the kernel rename devices or letting user space change names of running
devices if they are slaves).
> > for twiddling kernel-based interface naming policy.. :S
>
> I see your point. But my point is slave names don't really matter, only
> master name matters. So I am not sure there's any policy worth talking
> about here.
Oh yes, I don't disagree with you, but others seems to want to rename
the auto-bonded lower devices. Which can be done trivially if it was
a daemon in user space instantiating the auto-bond. We are just
providing a basic version of auto-bonding in the kernel. If there are
extra requirements on policy, or naming - the whole thing is better
solved in user space.
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 20:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexander Duyck, Jiri Pirko, Samudrala, Sridhar, virtualization,
Siwei Liu, liran.alon, Netdev, si-wei liu, David Miller
In-Reply-To: <20190228115641.7afe6f09@cakuba.netronome.com>
On Thu, Feb 28, 2019 at 11:56:41AM -0800, Jakub Kicinski wrote:
> On Thu, 28 Feb 2019 14:36:56 -0500, Michael S. Tsirkin wrote:
> > > It is a bit of a the chicken or the egg situation ;) But users can
> > > just blacklist, too. Anyway, I think this is far better than module
> > > parameters
> >
> > Sorry I'm a bit confused. What is better than what?
>
> I mean that blacklist net_failover or module param to disable
> net_failover and handle in user space are better than trying to solve
> the renaming at kernel level (either by adding module params that make
> the kernel rename devices or letting user space change names of running
> devices if they are slaves).
>
> > > for twiddling kernel-based interface naming policy.. :S
> >
> > I see your point. But my point is slave names don't really matter, only
> > master name matters. So I am not sure there's any policy worth talking
> > about here.
>
> Oh yes, I don't disagree with you, but others seems to want to rename
> the auto-bonded lower devices. Which can be done trivially if it was
> a daemon in user space instantiating the auto-bond. We are just
> providing a basic version of auto-bonding in the kernel. If there are
> extra requirements on policy, or naming - the whole thing is better
> solved in user space.
OK so it seems that you would be happy with a combination of the module
parameter disabling failover completely and renaming primary in kernel?
Did I get it right?
--
MST
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Jakub Kicinski @ 2019-02-28 23:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, Jiri Pirko, Samudrala, Sridhar, virtualization,
Siwei Liu, liran.alon, Netdev, si-wei liu, David Miller
In-Reply-To: <20190228151349-mutt-send-email-mst@kernel.org>
On Thu, 28 Feb 2019 15:14:55 -0500, Michael S. Tsirkin wrote:
> On Thu, Feb 28, 2019 at 11:56:41AM -0800, Jakub Kicinski wrote:
> > On Thu, 28 Feb 2019 14:36:56 -0500, Michael S. Tsirkin wrote:
> > > > It is a bit of a the chicken or the egg situation ;) But users can
> > > > just blacklist, too. Anyway, I think this is far better than module
> > > > parameters
> > >
> > > Sorry I'm a bit confused. What is better than what?
> >
> > I mean that blacklist net_failover or module param to disable
> > net_failover and handle in user space are better than trying to solve
> > the renaming at kernel level (either by adding module params that make
> > the kernel rename devices or letting user space change names of running
> > devices if they are slaves).
> >
> > > > for twiddling kernel-based interface naming policy.. :S
> > >
> > > I see your point. But my point is slave names don't really matter, only
> > > master name matters. So I am not sure there's any policy worth talking
> > > about here.
> >
> > Oh yes, I don't disagree with you, but others seems to want to rename
> > the auto-bonded lower devices. Which can be done trivially if it was
> > a daemon in user space instantiating the auto-bond. We are just
> > providing a basic version of auto-bonding in the kernel. If there are
> > extra requirements on policy, or naming - the whole thing is better
> > solved in user space.
>
> OK so it seems that you would be happy with a combination of the module
> parameter disabling failover completely and renaming primary in kernel?
> Did I get it right?
Not 100%, I'm personally not convinced that renaming primary in the
kernel is okay.
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Siwei Liu @ 2019-03-01 0:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexander Duyck, Jiri Pirko, Michael S. Tsirkin,
Samudrala, Sridhar, virtualization, liran.alon, Netdev,
si-wei liu, David Miller
In-Reply-To: <20190228115641.7afe6f09@cakuba.netronome.com>
On Thu, Feb 28, 2019 at 11:56 AM Jakub Kicinski <kubakici@wp.pl> wrote:
>
> On Thu, 28 Feb 2019 14:36:56 -0500, Michael S. Tsirkin wrote:
> > > It is a bit of a the chicken or the egg situation ;) But users can
> > > just blacklist, too. Anyway, I think this is far better than module
> > > parameters
> >
> > Sorry I'm a bit confused. What is better than what?
>
> I mean that blacklist net_failover or module param to disable
> net_failover and handle in user space are better than trying to solve
> the renaming at kernel level (either by adding module params that make
> the kernel rename devices or letting user space change names of running
> devices if they are slaves).
Before I was aksed to revive this old mail thread, I knew the
discussion could end up with something like this. Yes, theoretically
there's a point - basically you don't believe kernel should take risk
in fixing the issue, so you push back the hope to something in
hypothesis that actually wasn't done and hard to get done in reality.
It's not too different than saying "hey, what you're asking for is
simply wrong, don't do it! Go back to modify userspace to create a
bond or team instead!" FWIW I want to emphasize that the debate for
what should be the right place to implement this failover facility:
userspace versus kernel, had been around for almost a decade, and no
real work ever happened in userspace to "standardize" this in the
Linux world. The truth is that it's quite amount of complex work to
get it implemented right at userspace in reality: what Michael
mentions about making dracut auto-bonding aware is just tip of the
iceberg. Basically one would need to modify all the existing network
config tools to treat them well with this new auto-bonding concept:
handle duplicate MACs, differentiate it with regular bond/team, fix
boot time dependency of network boot and etc. Moreover, it's not a
single distro's effort from cloud provider's perspective, at least not
as simple as to say just move it to a daemon systemd/NM then work is
done. We (Oracle) had done extensive work in the past year to help
align various userspace components and work with distro vendors to
patch shipped packages to make them work with the failover 3-netdev
model. The work that needs to be done with userspace auto-bonding
would be more involved than just that, with quite trivial value (just
naming?) in turn that I suspect any developer in userspace could be
motivated.
So, simply put, no, we have zero interest in this direction. If
upstream believes this is the final conclusion, I think we can stop
discussing.
Thanks,
-Siwei
>
> > > for twiddling kernel-based interface naming policy.. :S
> >
> > I see your point. But my point is slave names don't really matter, only
> > master name matters. So I am not sure there's any policy worth talking
> > about here.
>
> Oh yes, I don't disagree with you, but others seems to want to rename
> the auto-bonded lower devices. Which can be done trivially if it was
> a daemon in user space instantiating the auto-bond. We are just
> providing a basic version of auto-bonding in the kernel. If there are
> extra requirements on policy, or naming - the whole thing is better
> solved in user space.
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Jakub Kicinski @ 2019-03-01 1:05 UTC (permalink / raw)
To: Siwei Liu
Cc: Alexander Duyck, Jiri Pirko, Michael S. Tsirkin,
Samudrala, Sridhar, virtualization, liran.alon, Netdev,
si-wei liu, David Miller
In-Reply-To: <CADGSJ239j-fG_vifkj-gTnAb_RXtBEbnQCFdZGimcgPXh35CMA@mail.gmail.com>
On Thu, 28 Feb 2019 16:20:28 -0800, Siwei Liu wrote:
> On Thu, Feb 28, 2019 at 11:56 AM Jakub Kicinski wrote:
> > On Thu, 28 Feb 2019 14:36:56 -0500, Michael S. Tsirkin wrote:
> > > > It is a bit of a the chicken or the egg situation ;) But users can
> > > > just blacklist, too. Anyway, I think this is far better than module
> > > > parameters
> > >
> > > Sorry I'm a bit confused. What is better than what?
> >
> > I mean that blacklist net_failover or module param to disable
> > net_failover and handle in user space are better than trying to solve
> > the renaming at kernel level (either by adding module params that make
> > the kernel rename devices or letting user space change names of running
> > devices if they are slaves).
>
> Before I was aksed to revive this old mail thread, I knew the
> discussion could end up with something like this. Yes, theoretically
> there's a point - basically you don't believe kernel should take risk
> in fixing the issue, so you push back the hope to something in
> hypothesis that actually wasn't done and hard to get done in reality.
> It's not too different than saying "hey, what you're asking for is
> simply wrong, don't do it! Go back to modify userspace to create a
> bond or team instead!" FWIW I want to emphasize that the debate for
> what should be the right place to implement this failover facility:
> userspace versus kernel, had been around for almost a decade, and no
> real work ever happened in userspace to "standardize" this in the
> Linux world.
Let me offer you my very subjective opinion of why "no real work ever
happened in user space". The actors who have primary interest to get
the auto-bonding working are HW vendors trying to either convince
customers to use SR-IOV, or being pressured by customers to make SR-IOV
easier to consume. HW vendors hire driver developers, not user space
developers. So the solution we arrive at is in the kernel for a non
technical reason (Conway's law, sort of).
$ cd NetworkManager/
$ git log --pretty=format:"%ae" | \
grep '\(mellanox\|intel\|broadcom\|netronome\)' | sort | uniq -c
81 andrew.zaborowski@intel.com
2 David.Woodhouse@intel.com
2 ismo.puustinen@intel.com
1 michael.i.doherty@intel.com
Andrew works on WiFi.
I have asked the NetworkManager folks to implement this feature last
year when net_failover got dangerously close to getting merged, and
they said they were never approached with this request before, much less
offered code that solve it. Unfortunately before they got around to it
net_failover was merged already, and they didn't proceed.
So to my knowledge nobody ever tried to solve this in user space.
I don't think net_failover is particularly terrible, or that renaming
of primary in the kernel is the end of the world, but I'd appreciate if
you could point me to efforts to solve it upstream in user space
components, or acknowledge that nobody actually tried that.
^ permalink raw reply
* Re: [PATCH] drm/virtio: Allow userspace to mmap() framebuffer
From: Gerd Hoffmann @ 2019-03-01 5:51 UTC (permalink / raw)
To: Joshua Watt
Cc: David Airlie, linux-kernel, Daniel Vetter, dri-devel,
virtualization
In-Reply-To: <20190228164741.15590-1-JPEWhacker@gmail.com>
On Thu, Feb 28, 2019 at 10:47:41AM -0600, Joshua Watt wrote:
> Reports the size of the virtgpu framebuffer to userspace and installs
> the deferred I/O handlers so that userspace can mmap() and write to it.
Fixed already, as side effect of switching virtio to the generic fbdev
emulation. Patches are in the drm-misc-next branch and should land
upstream in the 5.1 merge window.
cheers,
Gerd
^ permalink raw reply
* [PATCH v7 3/4] drm/qxl: remove conflicting framebuffers earlier
From: Gerd Hoffmann @ 2019-03-01 9:25 UTC (permalink / raw)
To: dri-devel
Cc: David Airlie, intel-gfx, open list,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, daniel,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, Dave Airlie
In-Reply-To: <20190301092502.30948-1-kraxel@redhat.com>
Add error checking while being at it.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/qxl/qxl_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index bb81e310eb6d..578d867a81d5 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -79,6 +79,10 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret)
goto free_dev;
+ ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
+ if (ret)
+ goto disable_pci;
+
ret = qxl_device_init(qdev, &qxl_driver, pdev);
if (ret)
goto disable_pci;
@@ -94,7 +98,6 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (ret)
goto modeset_cleanup;
- drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl");
drm_fbdev_generic_setup(&qdev->ddev, 32);
return 0;
--
2.9.3
^ permalink raw reply related
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-03-01 13:27 UTC (permalink / raw)
To: si-wei liu
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Samudrala, Sridhar, virtualization, Siwei Liu, liran.alon, Netdev,
David Miller
In-Reply-To: <8a387954-1e21-947b-a5a9-c49adaea2e81@oracle.com>
On Thu, Feb 28, 2019 at 05:30:56PM -0800, si-wei liu wrote:
>
>
> On 2/28/2019 6:26 AM, Michael S. Tsirkin wrote:
> > On Thu, Feb 28, 2019 at 01:32:12AM -0800, si-wei liu wrote:
> > > > > Will the
> > > > > change break userspace further?
> > > > >
> > > > > -Siwei
> > > > Didn't you show userspace is already broken. You can't "further
> > > > break it", rename already fails.
> > > It's a race, userspace tends to give slave a user(space) desired name but
> > > sometimes may fail due to this race. Today if failover master is not up,
> > > rename would succeed anyway. While what you proposed prohibits user from
> > > providing a name in all circumstances if I understand you correctly. That's
> > > what I meant of breaking userspace further. On the other hand, you seem to
> > > tighten the kernel default naming to udev predictable names, which is
> > > derived from only recent systemd-udevd, while there exists many possible
> > > userspace naming schemes out of that. Users today who deliberately chooses
> > > to disable predictable naming (net.ifnames=0 biosdevname=0) and fall back to
> > > kernel provided names would expect the ethX pattern, with this change
> > > admin/user scripts which matches the ethX pattern could potentially break.
> > Whatever crashes with a name not matching ethX will crash on the
> > standby interface *anyway*.
> With udev predictable naming disabled they should not. It's not hard for
> user to look for device attribute to persistent the name well, in a
> consistent and reliable way.
Well that's special code for failover already. So far we just
taught userspace to skip renaming slave interfaces.
> >
> > So I think what you are saying is that someone might have already
> > written scripts and gotten them to work on v4.17 when STANDBY was
> > included and these scripts rely on ethX. Now these scripts
> > will break.
> The controversial part is the new kernel naming pattern. Initially I thought
> there shouldn't be such crazy scripts relying on the pattern, but when I
> worked on cloud-init it I realized that there's already a lot of software
> taking assumption around the 'eth0' name. In the past I've seen random
> scripts that parses the ethX name assumes (incorrectly) the name ends up
> with digits, or even the digits and name are 1:1 mapped. Of course, you can
> say these are bugs in scripts themselves.
No what I say is that they will crash on rename of standby too.
> Anyway, I'll let others in the netdev to comment on this new scheme, maybe
> that's the concern of merely myself. The good part of your proposal is that
> we can get consistent slave name, which still plays its role until we move
> towards making slave names less relevant, i.e. ideally a 1-netdev model. I
> think we both agree that the master matters more than the slave names.
> >
> > Maybe it is still early enough (just half a year passed) that the
> > number of these users would be small. So how about a kernel config
> > option and maybe a module parameter to rename the primary? People can
> > then opt in to the old broken behaviour.
> Were I could I would ask why a similar opt-in (kernel config or module
> parameter) couldn't be implemented to open up the rename restriction on
> slave, net_failover in particular. What I felt about this rename restriction
> was more because of historical reason than anything else, while net_failover
> is comparatively a new type of link that we are now designing proper use
> case it should support, and can get it shaped to whatever it fits. My
> personal view is that the slave can't be renamed when master is running is
> just implementation details that got incorrectly exposed to userspace apps
> for many years. It's old behavior with historical reason for sure, but I
> don't think this applies to net_failover.
>
> (FWIW as one previous bond maintainer for another OS, we relieved the rename
> restriction slaves 13 year ago, while no single complaint or issue was ever
> raised because of this change over the years, neither from the customers of
> tens of millions of installation base, nor the FOSS software running atop.
> Of course, Linux is different so that experience doesn't count.)
>
> Thanks,
> -Siwei
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox