* [dpdk-stable] [PATCH] net/virtio-user: check negotiated features before set
@ 2019-01-22 16:40 eric zhang
2019-01-23 5:58 ` Yongseok Koh
0 siblings, 1 reply; 2+ messages in thread
From: eric zhang @ 2019-01-22 16:40 UTC (permalink / raw)
Cc: stable
From: Eric Zhang <eric.zhang@windriver.com>
[ backported from upstream commit 1db4d2330bc849a19b9f18479ae7a5a75bc01df2 ]
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 <eric.zhang@windriver.com>
Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
---
drivers/net/virtio/virtio_user/vhost_kernel.c | 19 ++++---
drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 68 ++++++++++++++++++-----
drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 3 +-
3 files changed, 68 insertions(+), 22 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
index 68d28b1..3502001 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
@@ -189,8 +189,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;
@@ -209,7 +209,7 @@ tap_supporte_mq(void)
}
close(tapfd);
- return tap_features & IFF_MULTI_QUEUE;
+ return tap_features;
}
static int
@@ -223,6 +223,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]);
@@ -276,17 +277,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);
}
@@ -380,7 +384,8 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev *dev,
else
hdr_size = sizeof(struct virtio_net_hdr);
- tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq);
+ tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq,
+ (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 689a5cf..e9ee774 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
@@ -36,26 +36,64 @@
#include <sys/stat.h>
#include <fcntl.h>
#include <net/if.h>
+#include <net/if_arp.h>
#include <errno.h>
#include <string.h>
#include <limits.h>
+#include <rte_ether.h>
+
#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)
+vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
+ 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
@@ -115,13 +153,15 @@ 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;
+ memcpy(ifr.ifr_hwaddr.sa_data, mac, ETHER_ADDR_LEN);
+ if (ioctl(tapfd, SIOCSIFHWADDR, (void *)&ifr) == -1) {
+ PMD_DRV_LOG(ERR, "SIOCSIFHWADDR failed: %s", strerror(errno));
+ goto error;
+ }
if (!(*p_ifname))
*p_ifname = strdup(ifr.ifr_name);
diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
index eae340c..ea7a6c9 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
+++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
@@ -64,4 +64,5 @@
/* Constants */
#define PATH_NET_TUN "/dev/net/tun"
-int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq);
+int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
+ const char *mac, uint64_t features);
--
2.7.4
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [dpdk-stable] [PATCH] net/virtio-user: check negotiated features before set
2019-01-22 16:40 [dpdk-stable] [PATCH] net/virtio-user: check negotiated features before set eric zhang
@ 2019-01-23 5:58 ` Yongseok Koh
0 siblings, 0 replies; 2+ messages in thread
From: Yongseok Koh @ 2019-01-23 5:58 UTC (permalink / raw)
To: eric zhang; +Cc: stable
> On Jan 23, 2019, at 1:40 AM, eric zhang <eric.zhang@windriver.com> wrote:
>
> From: Eric Zhang <eric.zhang@windriver.com>
>
> [ backported from upstream commit 1db4d2330bc849a19b9f18479ae7a5a75bc01df2 ]
>
> 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 <eric.zhang@windriver.com>
> Reviewed-by: Tiwei Bie <tiwei.bie@intel.com>
> ---
Applied to stable/17.11
thanks,
Yongseok
> drivers/net/virtio/virtio_user/vhost_kernel.c | 19 ++++---
> drivers/net/virtio/virtio_user/vhost_kernel_tap.c | 68 ++++++++++++++++++-----
> drivers/net/virtio/virtio_user/vhost_kernel_tap.h | 3 +-
> 3 files changed, 68 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index 68d28b1..3502001 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -189,8 +189,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;
> @@ -209,7 +209,7 @@ tap_supporte_mq(void)
> }
>
> close(tapfd);
> - return tap_features & IFF_MULTI_QUEUE;
> + return tap_features;
> }
>
> static int
> @@ -223,6 +223,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]);
>
> @@ -276,17 +277,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);
> }
>
> @@ -380,7 +384,8 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev *dev,
> else
> hdr_size = sizeof(struct virtio_net_hdr);
>
> - tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq);
> + tapfd = vhost_kernel_open_tap(&dev->ifname, hdr_size, req_mq,
> + (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 689a5cf..e9ee774 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.c
> @@ -36,26 +36,64 @@
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <net/if.h>
> +#include <net/if_arp.h>
> #include <errno.h>
> #include <string.h>
> #include <limits.h>
>
> +#include <rte_ether.h>
> +
> #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)
> +vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
> + 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
> @@ -115,13 +153,15 @@ 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;
> + memcpy(ifr.ifr_hwaddr.sa_data, mac, ETHER_ADDR_LEN);
> + if (ioctl(tapfd, SIOCSIFHWADDR, (void *)&ifr) == -1) {
> + PMD_DRV_LOG(ERR, "SIOCSIFHWADDR failed: %s", strerror(errno));
> + goto error;
> + }
>
> if (!(*p_ifname))
> *p_ifname = strdup(ifr.ifr_name);
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
> index eae340c..ea7a6c9 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel_tap.h
> @@ -64,4 +64,5 @@
> /* Constants */
> #define PATH_NET_TUN "/dev/net/tun"
>
> -int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq);
> +int vhost_kernel_open_tap(char **p_ifname, int hdr_size, int req_mq,
> + const char *mac, uint64_t features);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-01-23 5:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 16:40 [dpdk-stable] [PATCH] net/virtio-user: check negotiated features before set eric zhang
2019-01-23 5:58 ` Yongseok Koh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).