[Qemu-commits] [qemu/qemu] 650181: virtio-net: fix removal of failover device

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 650181007a027034620995eb3d5044cea1ec9b49
      
https://github.com/qemu/qemu/commit/650181007a027034620995eb3d5044cea1ec9b49
  Author: Juan Quintela <quintela@redhat.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M hw/net/virtio-net.c

  Log Message:
  -----------
  virtio-net: fix removal of failover device

If you have a networking device and its virtio failover device, and
you remove them in this order:
- virtio device
- the real device

You get qemu crash.
See bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820120

Bug exist on qemu 4.2 and 5.0.
But in 5.0 don't shows because commit
77b06bba62034a87cc61a9c8de1309ae3e527d97

somehow papers over it.

CC: Jason Wang <jasowang@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: 9a8d949245a0a3f90842d1611f56df6ae705560d
      
https://github.com/qemu/qemu/commit/9a8d949245a0a3f90842d1611f56df6ae705560d
  Author: Andrew <andrew@daynix.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M hw/net/net_tx_pkt.c

  Log Message:
  -----------
  hw/net: Added CSO for IPv6

Added fix for checksum offload for IPv6 if a backend doesn't
have a virtual header.
This patch is a part of IPv6 fragmentation.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: a2e5cb7a87206b916506697d22dc31aa0a43d8fe
      
https://github.com/qemu/qemu/commit/a2e5cb7a87206b916506697d22dc31aa0a43d8fe
  Author: Zhang Chen <chen.zhang@intel.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M net/colo-compare.c
    M qemu-options.hx

  Log Message:
  -----------
  net/colo-compare.c: Expose compare "max_queue_size" to users

This patch allow users to set the "max_queue_size" according
to their environment.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: 2b28a7ef90d79ed757f6b1cc512840389dc25748
      
https://github.com/qemu/qemu/commit/2b28a7ef90d79ed757f6b1cc512840389dc25748
  Author: Zhang Chen <chen.zhang@intel.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M qemu-options.hx

  Log Message:
  -----------
  qemu-options.hx: Clean up and fix typo for colo-compare

Fix some typo and optimized some descriptions.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: 894022e616016fe81745753f14adfbd680a1c7ee
      
https://github.com/qemu/qemu/commit/894022e616016fe81745753f14adfbd680a1c7ee
  Author: Laurent Vivier <lvivier@redhat.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M include/qemu/sockets.h
    M net/socket.c
    M net/tap.c
    M util/oslib-posix.c
    M util/oslib-win32.c

  Log Message:
  -----------
  net: check if the file descriptor is valid before using it

qemu_set_nonblock() checks that the file descriptor can be used and, if
not, crashes QEMU. An assert() is used for that. The use of assert() is
used to detect programming error and the coredump will allow to debug
the problem.

But in the case of the tap device, this assert() can be triggered by
a misconfiguration by the user. At startup, it's not a real problem, but it
can also happen during the hot-plug of a new device, and here it's a
problem because we can crash a perfectly healthy system.

For instance:
 # ip link add link virbr0 name macvtap0 type macvtap mode bridge
 # ip link set macvtap0 up
 # TAP=/dev/tap$(ip -o link show macvtap0 | cut -d: -f1)
 # qemu-system-x86_64 -machine q35 -device pcie-root-port,id=pcie-root-port-0 
-monitor stdio 9<> $TAP
 (qemu) netdev_add type=tap,id=hostnet0,vhost=on,fd=9
 (qemu) device_add 
driver=virtio-net-pci,netdev=hostnet0,id=net0,bus=pcie-root-port-0
 (qemu) device_del net0
 (qemu) netdev_del hostnet0
 (qemu) netdev_add type=tap,id=hostnet1,vhost=on,fd=9
 qemu-system-x86_64: .../util/oslib-posix.c:247: qemu_set_nonblock: Assertion 
`f != -1' failed.
 Aborted (core dumped)

To avoid that, add a function, qemu_try_set_nonblock(), that allows to report 
the
problem without crashing.

In the same way, we also update the function for vhostfd in net_init_tap_one() 
and
for fd in net_init_socket() (both descriptors are provided by the user and can
be wrong).

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: e7b347d0bf640adb1c998d317eaf44d2d7cbd973
      
https://github.com/qemu/qemu/commit/e7b347d0bf640adb1c998d317eaf44d2d7cbd973
  Author: Daniel P. Berrange <berrange@redhat.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M net/tap-bsd.c
    M net/tap-linux.c
    M net/tap-solaris.c
    M net/tap-stub.c
    M net/tap.c
    M net/tap_int.h

  Log Message:
  -----------
  net: detect errors from probing vnet hdr flag for TAP devices

When QEMU sets up a tap based network device backend, it mostly ignores errors
reported from various ioctl() calls it makes, assuming the TAP file descriptor
is valid. This assumption can easily be violated when the user is passing in a
pre-opened file descriptor. At best, the ioctls may fail with a -EBADF, but if
the user passes in a bogus FD number that happens to clash with a FD number that
QEMU has opened internally for another reason, a wide variety of errnos may
result, as the TUNGETIFF ioctl number may map to a completely different command
on a different type of file.

By ignoring all these errors, QEMU sets up a zombie network backend that will
never pass any data. Even worse, when QEMU shuts down, or that network backend
is hot-removed, it will close this bogus file descriptor, which could belong to
another QEMU device backend.

There's no obvious guaranteed reliable way to detect that a FD genuinely is a
TAP device, as opposed to a UNIX socket, or pipe, or something else. Checking
the errno from probing vnet hdr flag though, does catch the big common cases.
ie calling TUNGETIFF will return EBADF for an invalid FD, and ENOTTY when FD is
a UNIX socket, or pipe which catches accidental collisions with FDs used for
stdio, or monitor socket.

Previously the example below where bogus fd 9 collides with the FD used for the
chardev saw:

$ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \
  -chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \
  -monitor stdio -vnc :0
qemu-system-x86_64: -netdev tap,id=hostnet0,fd=9: TUNGETIFF ioctl() failed: 
Inappropriate ioctl for device
TUNSETOFFLOAD ioctl() failed: Bad address
QEMU 2.9.1 monitor - type 'help' for more information
(qemu) Warning: netdev hostnet0 has no peer

which gives a running QEMU with a zombie network backend.

With this change applied we get an error message and QEMU immediately exits
before carrying on and making a bigger disaster:

$ ./x86_64-softmmu/qemu-system-x86_64 -netdev tap,id=hostnet0,fd=9 \
  -chardev socket,id=charchannel0,path=/tmp/qga,server,nowait \
  -monitor stdio -vnc :0
qemu-system-x86_64: -netdev tap,id=hostnet0,vhost=on,fd=9: Unable to query 
TUNGETIFF on FD 9: Inappropriate ioctl for device

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Tested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 20171027085548.3472-1-berrange@redhat.com
[lv: to simplify, don't check on EINVAL with TUNGETIFF as it exists since 
v2.6.27]
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: a134321ef676723768973537bb9b49365ae2062e
      
https://github.com/qemu/qemu/commit/a134321ef676723768973537bb9b49365ae2062e
  Author: erik-smit <erik.lucas.smit@gmail.com>
  Date:   2020-07-15 (Wed, 15 Jul 2020)

  Changed paths:
    M hw/net/ftgmac100.c

  Log Message:
  -----------
  ftgmac100: fix dblac write test

The test of the write of the dblac register was testing the old value
instead of the new value. This would accept the write of an invalid value
but subsequently refuse any following valid writes.

Signed-off-by: erik-smit <erik.lucas.smit@gmail.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>


  Commit: ee5128bb00f90dd301991d80d1db5224ce924c84
      
https://github.com/qemu/qemu/commit/ee5128bb00f90dd301991d80d1db5224ce924c84
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2020-07-16 (Thu, 16 Jul 2020)

  Changed paths:
    M hw/net/ftgmac100.c
    M hw/net/net_tx_pkt.c
    M hw/net/virtio-net.c
    M include/qemu/sockets.h
    M net/colo-compare.c
    M net/socket.c
    M net/tap-bsd.c
    M net/tap-linux.c
    M net/tap-solaris.c
    M net/tap-stub.c
    M net/tap.c
    M net/tap_int.h
    M qemu-options.hx
    M util/oslib-posix.c
    M util/oslib-win32.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into 
staging

# gpg: Signature made Wed 15 Jul 2020 14:49:07 BST
# gpg:                using RSA key EF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat) 
<jasowang@redhat.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg:          It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F  3562 EF04 965B 398D 6211

* remotes/jasowang/tags/net-pull-request:
  ftgmac100: fix dblac write test
  net: detect errors from probing vnet hdr flag for TAP devices
  net: check if the file descriptor is valid before using it
  qemu-options.hx: Clean up and fix typo for colo-compare
  net/colo-compare.c: Expose compare "max_queue_size" to users
  hw/net: Added CSO for IPv6
  virtio-net: fix removal of failover device

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/8746309137ba...ee5128bb00f9