From 1db4d2330bc849a19b9f18479ae7a5a75bc01df2 Mon Sep 17 00:00:00 2001 From: Eric Zhang Date: Wed, 29 Aug 2018 11:55:21 -0400 Subject: [PATCH] net/virtio-user: check negotiated features before set This patch checks negotiated features to see if necessary to offload before set the tap device offload capabilities. It also checks if kernel support the TUNSETOFFLOAD operation. Fixes: 5e97e4202563 ("net/virtio-user: enable offloading") Cc: stable@dpdk.org Signed-off-by: Eric Zhang Reviewed-by: Tiwei Bie --- drivers/net/virtio/virtio_user/vhost_kernel.c | 18 +++--- .../net/virtio/virtio_user/vhost_kernel_tap.c | 56 ++++++++++++++----- .../net/virtio/virtio_user/vhost_kernel_tap.h | 2 +- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c index b2444096c2..d1be821626 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel.c +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c @@ -147,8 +147,8 @@ prepare_vhost_memory_kernel(void) (1ULL << VIRTIO_NET_F_HOST_TSO6) | \ (1ULL << VIRTIO_NET_F_CSUM)) -static int -tap_supporte_mq(void) +static unsigned int +tap_support_features(void) { int tapfd; unsigned int tap_features; @@ -167,7 +167,7 @@ tap_supporte_mq(void) } close(tapfd); - return tap_features & IFF_MULTI_QUEUE; + return tap_features; } static int @@ -181,6 +181,7 @@ vhost_kernel_ioctl(struct virtio_user_dev *dev, struct vhost_memory_kernel *vm = NULL; int vhostfd; unsigned int queue_sel; + unsigned int features; PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]); @@ -234,17 +235,20 @@ vhost_kernel_ioctl(struct virtio_user_dev *dev, } if (!ret && req_kernel == VHOST_GET_FEATURES) { + features = tap_support_features(); /* with tap as the backend, all these features are supported * but not claimed by vhost-net, so we add them back when * reporting to upper layer. */ - *((uint64_t *)arg) |= VHOST_KERNEL_GUEST_OFFLOADS_MASK; - *((uint64_t *)arg) |= VHOST_KERNEL_HOST_OFFLOADS_MASK; + if (features & IFF_VNET_HDR) { + *((uint64_t *)arg) |= VHOST_KERNEL_GUEST_OFFLOADS_MASK; + *((uint64_t *)arg) |= VHOST_KERNEL_HOST_OFFLOADS_MASK; + } /* vhost_kernel will not declare this feature, but it does * support multi-queue. */ - if (tap_supporte_mq()) + if (features & IFF_MULTI_QUEUE) *(uint64_t *)arg |= (1ull << VIRTIO_NET_F_MQ); } @@ -339,7 +343,7 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev *dev, hdr_size = sizeof(struct virtio_net_hdr); tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq, - (char *)dev->mac_addr); + (char *)dev->mac_addr, dev->features); if (tapfd < 0) { PMD_DRV_LOG(ERR, "fail to open tap for vhost kernel"); return -1; diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c index 9ea7ade742..a3faf1d0c4 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c @@ -16,21 +16,55 @@ #include "vhost_kernel_tap.h" #include "../virtio_logs.h" +#include "../virtio_pci.h" + +static int +vhost_kernel_tap_set_offload(int fd, uint64_t features) +{ + unsigned int offload = 0; + + if (features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) { + offload |= TUN_F_CSUM; + if (features & (1ULL << VIRTIO_NET_F_GUEST_TSO4)) + offload |= TUN_F_TSO4; + if (features & (1ULL << VIRTIO_NET_F_GUEST_TSO6)) + offload |= TUN_F_TSO6; + if (features & ((1ULL << VIRTIO_NET_F_GUEST_TSO4) | + (1ULL << VIRTIO_NET_F_GUEST_TSO6)) && + (features & (1ULL << VIRTIO_NET_F_GUEST_ECN))) + offload |= TUN_F_TSO_ECN; + if (features & (1ULL << VIRTIO_NET_F_GUEST_UFO)) + offload |= TUN_F_UFO; + } + + if (offload != 0) { + /* Check if our kernel supports TUNSETOFFLOAD */ + if (ioctl(fd, TUNSETOFFLOAD, 0) != 0 && errno == EINVAL) { + PMD_DRV_LOG(ERR, "Kernel does't support TUNSETOFFLOAD\n"); + return -ENOTSUP; + } + + if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) { + offload &= ~TUN_F_UFO; + if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) { + PMD_DRV_LOG(ERR, "TUNSETOFFLOAD ioctl() failed: %s\n", + strerror(errno)); + return -1; + } + } + } + + return 0; +} int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq, - const char *mac) + const char *mac, uint64_t features) { unsigned int tap_features; int sndbuf = INT_MAX; struct ifreq ifr; int tapfd; - unsigned int offload = - TUN_F_CSUM | - TUN_F_TSO4 | - TUN_F_TSO6 | - TUN_F_TSO_ECN | - TUN_F_UFO; /* TODO: * 1. verify we can get/set vnet_hdr_len, tap_probe_vnet_hdr_len @@ -90,13 +124,7 @@ vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq, goto error; } - /* TODO: before set the offload capabilities, we'd better (1) check - * negotiated features to see if necessary to offload; (2) query tap - * to see if it supports the offload capabilities. - */ - if (ioctl(tapfd, TUNSETOFFLOAD, offload) != 0) - PMD_DRV_LOG(ERR, "TUNSETOFFLOAD ioctl() failed: %s", - strerror(errno)); + vhost_kernel_tap_set_offload(tapfd, features); memset(&ifr, 0, sizeof(ifr)); ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER; diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h index 01a026f509..e0e95b4f59 100644 --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h @@ -36,4 +36,4 @@ #define PATH_NET_TUN "/dev/net/tun" int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq, - const char *mac); + const char *mac, uint64_t features); -- 2.20.1