Virtio-Net driver is not respecting VIRTIO_NET_F_MRG_RXBUF

virtio-win / kvm-guest-drivers-windows

Windows paravirtualized drivers for QEMU\KVM

https://www.linux-kvm.org/page/WindowsGuestDrivers

BSD 3-Clause "New" or "Revised" License

1.91k stars 377 forks source link

Virtio-Net driver is not respecting VIRTIO_NET_F_MRG_RXBUF #1090

Open ddaney-fungible opened 2 months ago

ddaney-fungible commented 2 months ago

If a Virtio-Net PCI function (Device) does not advertise the VIRTIO_NET_F_MRG_RXBUF then the driver must supply a single descriptor RX buffer large enough to contain the virtio_net_hdr and the RX packet contents.

The Windows driver does not do this, it supplies a two descriptor buffer, one for the virtio_net_hdr and another for the packet contents, this behavior is explicitly prohibited by the specification.

YanVugenfirer commented 2 months ago

I think this is an unfortunate remnant of the support for the pre-1.0 virtio spec.

ybendito commented 2 months ago

Probably was broken long time ago. Probably the driver should fail to start if this feature is not present until this is fixed. Thank you for pointing this out.

YanVugenfirer commented 2 months ago

@ddaney-fungible Out of curiosity - do you experience issues with the non-compliant driver on HW or other than QEMU\KVM hypervisor?

ddaney-fungible commented 2 months ago

@ddaney-fungible Out of curiosity - do you experience issues with the non-compliant driver on HW or other than QEMU\KVM hypervisor?

Yes. The failure was seen in a virtio-net function that is not part of Qemu. On the system in question EDK2 and Linux drivers seem to work well, but it was observed that this Windows driver was supplying buffers in two chunks, one of size 12 for the header and the second for the packet payload.

ybendito commented 2 months ago

@ddaney-fungible Can you please share with us the device features as suggested during feature negotiation? (better in hex) Thanks.

ddaney-fungible commented 2 months ago

@ddaney-fungible Can you please share with us the host features as suggested during feature negotiation? (better in hex) Thanks.

device_feature[0] = 0x00010029 defice_feature[1] = 0x00000057

Driver is responding with:

driver_feature[0] = 0x00010029 driver_feature[1] = 0x00000007

ybendito commented 2 months ago

@ddaney-fungible Just some notes to the feature set: The device sets VIRTIO_NET_F_CTRL_GUEST_OFFLOADS(2) but this does not have any effect as the device does not have a control queue and does not indicate support for any guest offloads, The device sets VIRTIO_NET_F_GSO(6) which does not have any effect as this bit is somehow may make sense only for legacy device The features 4 and 48 are not defined in the spec and so ignored but in further editions of the spec they may designate something unexpected All this, of course, is not related to the fact that the driver currently ignores VIRTIO_NET_F_MRG_RXBUF

ddaney-fungible commented 2 months ago

I think there is a misunderstanding of how to interpret those values.

device_feature[0] = VIRTIO_NET_F_CSUM | VIRTIO_NET_F_MTU | VIRTIO_NET_F_MAC | VIRTIO_NET_F_STATUS

device_feature[1] = VIRTIO_F_VERSION_1 | VIRTIO_F_NOTIFICATION_DATA | VIRTIO_F_ACCESS_PLATFORM | VIRTIO_F_ORDER_PLATFORM | VIRTIO_F_RING_PACKED

ybendito commented 2 months ago

@ddaney-fungible Sorry, by mistake I swapped them when read