[v3] net/virtio-user: check negotiated features before set
Checks
Commit Message
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.
Signed-off-by: eric zhang <eric.zhang@windriver.com>
---
v3:
* make other offloading features depend on CSUM
* check IFF_VNET_HDR support when handling VHOST_GET_FEATURES
---
v2:
* don't return failure when failed to set offload to tap
* check if offloads available when handling VHOST_GET_FEATURES
---
drivers/net/virtio/virtio_user/vhost_kernel.c | 18 +++++---
drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 56 +++++++++++++++++------
drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 2 +-
3 files changed, 54 insertions(+), 22 deletions(-)
Comments
On Wed, Aug 29, 2018 at 11:55:21AM -0400, eric zhang wrote:
> 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
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
Thanks!
>
> Signed-off-by: eric zhang <eric.zhang@windriver.com>
>
> ---
> v3:
> * make other offloading features depend on CSUM
> * check IFF_VNET_HDR support when handling VHOST_GET_FEATURES
>
> ---
> v2:
> * don't return failure when failed to set offload to tap
> * check if offloads available when handling VHOST_GET_FEATURES
> ---
> drivers/net/virtio/virtio_user/vhost_kernel.c | 18 +++++---
> drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 56 +++++++++++++++++------
> drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 2 +-
> 3 files changed, 54 insertions(+), 22 deletions(-)
>
On 08/29/2018 05:55 PM, eric zhang wrote:
> 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.
>
> Signed-off-by: eric zhang <eric.zhang@windriver.com>
>
> ---
> v3:
> * make other offloading features depend on CSUM
> * check IFF_VNET_HDR support when handling VHOST_GET_FEATURES
>
> ---
> v2:
> * don't return failure when failed to set offload to tap
> * check if offloads available when handling VHOST_GET_FEATURES
> ---
> drivers/net/virtio/virtio_user/vhost_kernel.c | 18 +++++---
> drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 56 +++++++++++++++++------
> drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 2 +-
> 3 files changed, 54 insertions(+), 22 deletions(-)
>
Applied to dpdk-next-virtio/master, adding Fixes: line
as suggested by Tiwei.
Thanks,
Maxime
@@ -189,8 +189,8 @@ struct vhost_memory_kernel {
(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;
@@ -209,7 +209,7 @@ struct vhost_memory_kernel {
}
close(tapfd);
- return tap_features & IFF_MULTI_QUEUE;
+ return tap_features;
}
static int
@@ -223,6 +223,7 @@ struct vhost_memory_kernel {
struct vhost_memory_kernel *vm = NULL;
int vhostfd;
unsigned int queue_sel;
+ unsigned int features;
PMD_DRV_LOG(INFO, "%s", vhost_msg_strings[req]);
@@ -276,17 +277,20 @@ struct vhost_memory_kernel {
}
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);
}
@@ -381,7 +385,7 @@ struct vhost_memory_kernel {
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;
@@ -45,21 +45,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
@@ -119,13 +153,7 @@
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;
@@ -65,4 +65,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);