On Tue, Mar 30, 2010 at 12:21:05PM -0700, David L Stevens wrote:
>
> This patch adds mergeable receive buffer support to qemu-kvm,
> to allow enabling it when vhost_net supports it.
>
> It also adds a missing call to vhost_net_ack_features() to
> push acked features to vhost_net.
>
> The patch is relative to Michael Tsirkin's qemu-kvm git tree.
>
> +-DLS
>
> Signed-off-by: David L Stevens <dlstevens@xxxxxxxxxx>
Patch is well-formed, thanks!
BTW, where qemu-km and qemu share code, it is better
to send patch against qemu.
Since qemu-kvm generally carries kernel headers it needs, it might also
be a good idea to add if_tun to the kvm header directory.
> diff -ruNp qemu-kvm.mst/hw/vhost_net.c qemu-kvm.dls/hw/vhost_net.c
> --- qemu-kvm.mst/hw/vhost_net.c 2010-03-03 13:39:07.000000000 -0800
> +++ qemu-kvm.dls/hw/vhost_net.c 2010-03-29 20:37:34.000000000 -0700
> @@ -5,6 +5,7 @@
> #include <sys/ioctl.h>
> #include <linux/vhost.h>
> #include <linux/virtio_ring.h>
> +#include <linux/if_tun.h>
> #include <netpacket/packet.h>
> #include <net/ethernet.h>
> #include <net/if.h>
> @@ -38,15 +39,6 @@ unsigned vhost_net_get_features(struct v
> return features;
> }
>
> -void vhost_net_ack_features(struct vhost_net *net, unsigned features)
> -{
> - net->dev.acked_features = net->dev.backend_features;
> - if (features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))
> - net->dev.acked_features |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
> - if (features & (1 << VIRTIO_RING_F_INDIRECT_DESC))
> - net->dev.acked_features |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
> -}
> -
> static int vhost_net_get_fd(VLANClientState *backend)
> {
> switch (backend->info->type) {
> @@ -58,6 +50,25 @@ static int vhost_net_get_fd(VLANClientSt
> }
> }
>
> +void vhost_net_ack_features(struct vhost_net *net, unsigned features)
> +{
> + int vnet_hdr_sz = sizeof(struct virtio_net_hdr);
> +
> + net->dev.acked_features = net->dev.backend_features;
> + if (features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))
> + net->dev.acked_features |= (1 << VIRTIO_F_NOTIFY_ON_EMPTY);
> + if (features & (1 << VIRTIO_RING_F_INDIRECT_DESC))
> + net->dev.acked_features |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
> + if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
> + net->dev.acked_features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
> + vnet_hdr_sz = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> + }
> +#ifdef TUNSETVNETHDRSZ
> + if (ioctl(vhost_net_get_fd(net->vc), TUNSETVNETHDRSZ, &vnet_hdr_sz) < 0)
> + perror("TUNSETVNETHDRSZ");
> +#endif /* TUNSETVNETHDRSZ */
Note that this will break injecting raw packets after migration
(that code assumes header size is sizeof(struct virtio_net_hdr)).
Need to fix it.
So I think we are better off adding tap_set_vnet_header_size.
That function would:
- Remember current value, return success and do nothing
if value is not changed.
- Try to change and return error if not.
- To get doubly sure, we can call TUNSETVNETHDRSZ with
sizeof(struct virtio_net_hdr) at startup. If we get success
once we should further on as well, otherwise we can exit.
An additional benefit is that tap driver will stay
localized in one file.
> +}
> +
> struct vhost_net *vhost_net_init(VLANClientState *backend, int devfd)
> {
> int r;
> diff -ruNp qemu-kvm.mst/hw/virtio-net.c qemu-kvm.dls/hw/virtio-net.c
> --- qemu-kvm.mst/hw/virtio-net.c 2010-03-03 13:39:07.000000000 -0800
> +++ qemu-kvm.dls/hw/virtio-net.c 2010-03-29 16:15:46.000000000 -0700
> @@ -211,12 +211,16 @@ static void virtio_net_set_features(Virt
> n->mergeable_rx_bufs = !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF));
>
> if (n->has_vnet_hdr) {
> + struct vhost_net *vhost_net = tap_get_vhost_net(n->nic->nc.peer);
> +
> tap_set_offload(n->nic->nc.peer,
> (features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> (features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> (features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> (features >> VIRTIO_NET_F_GUEST_ECN) & 1,
> (features >> VIRTIO_NET_F_GUEST_UFO) & 1);
> + if (vhost_net)
> + vhost_net_ack_features(vhost_net, features);
Good catch, thanks! I'll apply this separately.
> }
> }
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
