All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] reference implementation of RSS
@ 2020-03-09  8:34 Yuri Benditovich
  2020-03-09  8:34 ` [PATCH v2 1/4] virtio-net: introduce RSS and hash report features Yuri Benditovich
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-09  8:34 UTC (permalink / raw)
  To: qemu-devel, mst, jasowang; +Cc: yan

Support for VIRTIO_NET_F_RSS feature in QEMU for reference
purpose. Implements Toeplitz hash calculation for incoming
packets according to configuration provided by driver.

Changes from v1:
Changes in standard virtio_net.h moved to virtio-net.c until
standard Linux header is updated and merged to QEMU
Added migration blocker if RSS is negotiated

Yuri Benditovich (4):
  virtio-net: introduce RSS and hash report features
  virtio-net: implement RSS configuration command
  virtio-net: implement RX RSS processing
  virtio-net: block migration if RSS feature negotiated

 hw/net/trace-events            |   3 +
 hw/net/virtio-net.c            | 347 +++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-net.h |  13 ++
 3 files changed, 352 insertions(+), 11 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/4] virtio-net: introduce RSS and hash report features
  2020-03-09  8:34 [PATCH v2 0/4] reference implementation of RSS Yuri Benditovich
@ 2020-03-09  8:34 ` Yuri Benditovich
  2020-03-09  8:34 ` [PATCH v2 2/4] virtio-net: implement RSS configuration command Yuri Benditovich
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-09  8:34 UTC (permalink / raw)
  To: qemu-devel, mst, jasowang; +Cc: yan

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3627bb1717..9545b0e84f 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -71,6 +71,101 @@
 #define VIRTIO_NET_IP6_ADDR_SIZE   32      /* ipv6 saddr + daddr */
 #define VIRTIO_NET_MAX_IP6_PAYLOAD VIRTIO_NET_MAX_TCP_PAYLOAD
 
+/* TODO: remove after virtio-net header update */
+#if !defined(VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+#define VIRTIO_NET_F_HASH_REPORT    57  /* Supports hash report */
+#define VIRTIO_NET_F_RSS            60  /* Supports RSS RX steering */
+
+/* supported/enabled hash types */
+#define VIRTIO_NET_RSS_HASH_TYPE_IPv4          (1 << 0)
+#define VIRTIO_NET_RSS_HASH_TYPE_TCPv4         (1 << 1)
+#define VIRTIO_NET_RSS_HASH_TYPE_UDPv4         (1 << 2)
+#define VIRTIO_NET_RSS_HASH_TYPE_IPv6          (1 << 3)
+#define VIRTIO_NET_RSS_HASH_TYPE_TCPv6         (1 << 4)
+#define VIRTIO_NET_RSS_HASH_TYPE_UDPv6         (1 << 5)
+#define VIRTIO_NET_RSS_HASH_TYPE_IP_EX         (1 << 6)
+#define VIRTIO_NET_RSS_HASH_TYPE_TCP_EX        (1 << 7)
+#define VIRTIO_NET_RSS_HASH_TYPE_UDP_EX        (1 << 8)
+
+#define __le16 uint16_t
+#define __le32 uint32_t
+#define __u8   uint8_t
+#define __u16  uint16_t
+#define __u32  uint32_t
+
+struct virtio_net_config_with_rss {
+    /* The config defining mac address (if VIRTIO_NET_F_MAC) */
+    __u8 mac[ETH_ALEN];
+    /* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
+    __u16 status;
+    /*
+     * Maximum number of each of transmit and receive queues;
+     * see VIRTIO_NET_F_MQ and VIRTIO_NET_CTRL_MQ.
+     * Legal values are between 1 and 0x8000
+     */
+    __u16 max_virtqueue_pairs;
+    /* Default maximum transmit unit advice */
+    __u16 mtu;
+    /*
+     * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
+     * Any other value stands for unknown.
+     */
+    __u32 speed;
+    /*
+     * 0x00 - half duplex
+     * 0x01 - full duplex
+     * Any other value stands for unknown.
+     */
+    __u8 duplex;
+    /* maximum size of RSS key */
+    __u8 rss_max_key_size;
+    /* maximum number of indirection table entries */
+    __le16 rss_max_indirection_table_length;
+    /* bitmask of supported VIRTIO_NET_RSS_HASH_ types */
+    __le32 supported_hash_types;
+} __attribute__((packed));
+
+#define virtio_net_config virtio_net_config_with_rss
+
+struct virtio_net_hdr_v1_hash {
+    struct virtio_net_hdr_v1 hdr;
+    __le32 hash_value;
+#define VIRTIO_NET_HASH_REPORT_NONE            0
+#define VIRTIO_NET_HASH_REPORT_IPv4            1
+#define VIRTIO_NET_HASH_REPORT_TCPv4           2
+#define VIRTIO_NET_HASH_REPORT_UDPv4           3
+#define VIRTIO_NET_HASH_REPORT_IPv6            4
+#define VIRTIO_NET_HASH_REPORT_TCPv6           5
+#define VIRTIO_NET_HASH_REPORT_UDPv6           6
+#define VIRTIO_NET_HASH_REPORT_IPv6_EX         7
+#define VIRTIO_NET_HASH_REPORT_TCPv6_EX        8
+#define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
+    __le16 hash_report;
+    __le16 padding;
+};
+
+/*
+ * The command VIRTIO_NET_CTRL_MQ_RSS_CONFIG has the same effect as
+ * VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET does and additionally configures
+ * the receive steering to use a hash calculated for incoming packet
+ * to decide on receive virtqueue to place the packet. The command
+ * also provides parameters to calculate a hash and receive virtqueue.
+ */
+struct virtio_net_rss_config {
+    __le32 hash_types;
+    __le16 indirection_table_mask;
+    __le16 unclassified_queue;
+    __le16 indirection_table[1/* + indirection_table_mask */];
+    __le16 max_tx_vq;
+    __u8 hash_key_length;
+    __u8 hash_key_data[/* hash_key_length */];
+};
+
+#define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
+#define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2
+
+#endif
+
 /* Purge coalesced packets timer interval, This value affects the performance
    a lot, and should be tuned carefully, '300000'(300us) is the recommended
    value to pass the WHQL test, '50000' can gain 2x netperf throughput with
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/4] virtio-net: implement RSS configuration command
  2020-03-09  8:34 [PATCH v2 0/4] reference implementation of RSS Yuri Benditovich
  2020-03-09  8:34 ` [PATCH v2 1/4] virtio-net: introduce RSS and hash report features Yuri Benditovich
@ 2020-03-09  8:34 ` Yuri Benditovich
  2020-03-10  3:02   ` Jason Wang
  2020-03-09  8:34 ` [PATCH v2 3/4] virtio-net: implement RX RSS processing Yuri Benditovich
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-09  8:34 UTC (permalink / raw)
  To: qemu-devel, mst, jasowang; +Cc: yan

Optionally report RSS feature.
Handle RSS configuration command and keep RSS parameters
in virtio-net device context.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/trace-events            |   3 +
 hw/net/virtio-net.c            | 148 +++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-net.h |  11 +++
 3 files changed, 153 insertions(+), 9 deletions(-)

diff --git a/hw/net/trace-events b/hw/net/trace-events
index a1da98a643..9823480d91 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -371,6 +371,9 @@ virtio_net_announce_notify(void) ""
 virtio_net_announce_timer(int round) "%d"
 virtio_net_handle_announce(int round) "%d"
 virtio_net_post_load_device(void)
+virtio_net_rss_disable(void)
+virtio_net_rss_error(int error_case) "case %d"
+virtio_net_rss_enable(uint32_t p1, uint16_t p2, uint8_t p3) "hashes 0x%x, table of %d, key of %d"
 
 # tulip.c
 tulip_reg_write(uint64_t addr, const char *name, int size, uint64_t val) "addr 0x%02"PRIx64" (%s) size %d value 0x%08"PRIx64
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9545b0e84f..27071eccd2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -172,6 +172,16 @@ struct virtio_net_rss_config {
    tso/gso/gro 'off'. */
 #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
 
+#define VIRTIO_NET_RSS_SUPPORTED_HASHES (VIRTIO_NET_RSS_HASH_TYPE_IPv4 | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_TCPv4 | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_UDPv4 | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_IPv6 | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_TCPv6 | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_UDPv6 | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_IP_EX | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | \
+                                         VIRTIO_NET_RSS_HASH_TYPE_UDP_EX)
+
 /* temporary until standard header include it */
 #if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
 
@@ -203,6 +213,8 @@ static VirtIOFeature feature_sizes[] = {
      .end = endof(struct virtio_net_config, mtu)},
     {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX,
      .end = endof(struct virtio_net_config, duplex)},
+    {.flags = 1ULL << VIRTIO_NET_F_RSS,
+     .end = endof(struct virtio_net_config, supported_hash_types)},
     {}
 };
 
@@ -233,6 +245,11 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
     memcpy(netcfg.mac, n->mac, ETH_ALEN);
     virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
     netcfg.duplex = n->net_conf.duplex;
+    netcfg.rss_max_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
+    virtio_stw_p(vdev, &netcfg.rss_max_indirection_table_length,
+                 VIRTIO_NET_RSS_MAX_TABLE_LEN);
+    virtio_stl_p(vdev, &netcfg.supported_hash_types,
+                 VIRTIO_NET_RSS_SUPPORTED_HASHES);
     memcpy(config, &netcfg, n->config_size);
 }
 
@@ -796,6 +813,7 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
         return features;
     }
 
+    virtio_clear_feature(&features, VIRTIO_NET_F_RSS);
     features = vhost_net_get_features(get_vhost_net(nc->peer), features);
     vdev->backend_features = features;
 
@@ -955,6 +973,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
     }
 
     virtio_net_set_multiqueue(n,
+                              virtio_has_feature(features, VIRTIO_NET_F_RSS) ||
                               virtio_has_feature(features, VIRTIO_NET_F_MQ));
 
     virtio_net_set_mrg_rx_bufs(n,
@@ -1231,25 +1250,134 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
     }
 }
 
+static void virtio_net_disable_rss(VirtIONet *n)
+{
+    if (n->rss_data.enabled) {
+        trace_virtio_net_rss_disable();
+    }
+    n->rss_data.enabled = false;
+}
+
+static uint16_t virtio_net_handle_rss(VirtIONet *n,
+                                      struct iovec *iov, unsigned int iov_cnt)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
+    struct virtio_net_rss_config cfg;
+    size_t s, offset = 0, size_get;
+    uint16_t queues, i;
+    struct {
+        uint16_t us;
+        uint8_t b;
+    } QEMU_PACKED temp;
+    int err;
+
+    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_RSS)) {
+        err = 1;
+        goto error;
+    }
+    size_get = offsetof(struct virtio_net_rss_config, indirection_table);
+    s = iov_to_buf(iov, iov_cnt, offset, &cfg, size_get);
+    if (s != size_get) {
+        err = 2;
+        goto error;
+    }
+    n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
+    n->rss_data.indirections_len =
+        virtio_lduw_p(vdev, &cfg.indirection_table_mask);
+    n->rss_data.indirections_len++;
+    if (!is_power_of_2(n->rss_data.indirections_len)) {
+        err = 3;
+        goto error;
+    }
+    if (n->rss_data.indirections_len > VIRTIO_NET_RSS_MAX_TABLE_LEN) {
+        err = 4;
+        goto error;
+    }
+    n->rss_data.default_queue =
+        virtio_lduw_p(vdev, &cfg.unclassified_queue);
+    if (n->rss_data.default_queue >= n->max_queues) {
+        err = 5;
+        goto error;
+    }
+    offset += size_get;
+    size_get = sizeof(uint16_t) * n->rss_data.indirections_len;
+    s = iov_to_buf(iov, iov_cnt, offset, n->rss_data.indirections, size_get);
+    if (s != size_get) {
+        err = 10;
+        goto error;
+    }
+    for (i = 0; i < n->rss_data.indirections_len; ++i) {
+        uint16_t val = n->rss_data.indirections[i];
+        n->rss_data.indirections[i] = virtio_lduw_p(vdev, &val);
+    }
+    offset += size_get;
+    size_get = sizeof(temp);
+    s = iov_to_buf(iov, iov_cnt, offset, &temp, size_get);
+    if (s != size_get) {
+        err = 11;
+        goto error;
+    }
+    queues = virtio_lduw_p(vdev, &temp.us);
+    if (queues == 0 || queues > n->max_queues) {
+        err = 12;
+        goto error;
+    }
+    if (temp.b > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
+        err = 13;
+        goto error;
+    }
+    if (!temp.b && n->rss_data.hash_types) {
+        err = 20;
+        goto error;
+    }
+    if (!temp.b && !n->rss_data.hash_types) {
+        virtio_net_disable_rss(n);
+        return queues;
+    }
+    offset += size_get;
+    size_get = temp.b;
+    s = iov_to_buf(iov, iov_cnt, offset, n->rss_data.key, size_get);
+    if (s != size_get) {
+        err = 21;
+        goto error;
+    }
+    n->rss_data.enabled = true;
+    trace_virtio_net_rss_enable(n->rss_data.hash_types,
+                                n->rss_data.indirections_len,
+                                temp.b);
+    return queues;
+error:
+    warn_report("%s: error_case %d", __func__, err);
+    trace_virtio_net_rss_error(err);
+    virtio_net_disable_rss(n);
+    return 0;
+}
+
 static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
                                 struct iovec *iov, unsigned int iov_cnt)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
-    struct virtio_net_ctrl_mq mq;
-    size_t s;
     uint16_t queues;
 
-    s = iov_to_buf(iov, iov_cnt, 0, &mq, sizeof(mq));
-    if (s != sizeof(mq)) {
-        return VIRTIO_NET_ERR;
-    }
+    virtio_net_disable_rss(n);
+    if (cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
+        queues = virtio_net_handle_rss(n, iov, iov_cnt);
+    } else if (cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
+        struct virtio_net_ctrl_mq mq;
+        size_t s;
+        if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ)) {
+            return VIRTIO_NET_ERR;
+        }
+        s = iov_to_buf(iov, iov_cnt, 0, &mq, sizeof(mq));
+        if (s != sizeof(mq)) {
+            return VIRTIO_NET_ERR;
+        }
+        queues = virtio_lduw_p(vdev, &mq.virtqueue_pairs);
 
-    if (cmd != VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
+    } else {
         return VIRTIO_NET_ERR;
     }
 
-    queues = virtio_lduw_p(vdev, &mq.virtqueue_pairs);
-
     if (queues < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
         queues > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
         queues > n->max_queues ||
@@ -3304,6 +3432,8 @@ static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT64("ctrl_guest_offloads", VirtIONet, host_features,
                     VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true),
     DEFINE_PROP_BIT64("mq", VirtIONet, host_features, VIRTIO_NET_F_MQ, false),
+    DEFINE_PROP_BIT64("rss", VirtIONet, host_features,
+                    VIRTIO_NET_F_RSS, false),
     DEFINE_PROP_BIT64("guest_rsc_ext", VirtIONet, host_features,
                     VIRTIO_NET_F_RSC_EXT, false),
     DEFINE_PROP_UINT32("rsc_interval", VirtIONet, rsc_timeout,
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 96c68d4a92..cf16f5192e 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -126,6 +126,9 @@ typedef struct VirtioNetRscChain {
 /* Maximum packet size we can receive from tap device: header + 64k */
 #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 * KiB))
 
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
+
 typedef struct VirtIONetQueue {
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
@@ -199,6 +202,14 @@ struct VirtIONet {
     bool failover;
     DeviceListener primary_listener;
     Notifier migration_state;
+    struct {
+        bool    enabled;
+        uint32_t hash_types;
+        uint8_t key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+        uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+        uint16_t indirections_len;
+        uint16_t default_queue;
+    } rss_data;
 };
 
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/4] virtio-net: implement RX RSS processing
  2020-03-09  8:34 [PATCH v2 0/4] reference implementation of RSS Yuri Benditovich
  2020-03-09  8:34 ` [PATCH v2 1/4] virtio-net: introduce RSS and hash report features Yuri Benditovich
  2020-03-09  8:34 ` [PATCH v2 2/4] virtio-net: implement RSS configuration command Yuri Benditovich
@ 2020-03-09  8:34 ` Yuri Benditovich
  2020-03-10  3:10   ` Jason Wang
  2020-03-10  6:13   ` Michael S. Tsirkin
  2020-03-09  8:34 ` [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated Yuri Benditovich
  2020-03-09  8:59 ` [PATCH v2 0/4] reference implementation of RSS no-reply
  4 siblings, 2 replies; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-09  8:34 UTC (permalink / raw)
  To: qemu-devel, mst, jasowang; +Cc: yan

If VIRTIO_NET_F_RSS negotiated and RSS is enabled, process
incoming packets, calculate packet's hash and place the
packet into respective RX virtqueue.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c            | 86 +++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-net.h |  1 +
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 27071eccd2..abc41fdb16 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -42,6 +42,7 @@
 #include "trace.h"
 #include "monitor/qdev.h"
 #include "hw/pci/pci.h"
+#include "net_rx_pkt.h"
 
 #define VIRTIO_NET_VM_VERSION    11
 
@@ -1610,8 +1611,78 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
     return 0;
 }
 
+static uint8_t virtio_net_get_hash_type(bool isip4,
+                                        bool isip6,
+                                        bool isudp,
+                                        bool istcp,
+                                        uint32_t types)
+{
+    uint32_t mask;
+    if (isip4) {
+        if (istcp && (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv4)) {
+            return NetPktRssIpV4Tcp;
+        }
+        if (isudp && (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv4)) {
+            return NetPktRssIpV4Udp;
+        }
+        if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+            return NetPktRssIpV4;
+        }
+    } else if (isip6) {
+        mask = VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+        if (istcp && (types & mask)) {
+            return (types & VIRTIO_NET_RSS_HASH_TYPE_TCP_EX) ?
+                NetPktRssIpV6TcpEx : NetPktRssIpV6Tcp;
+        }
+        mask = VIRTIO_NET_RSS_HASH_TYPE_UDP_EX | VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+        if (isudp && (types & mask)) {
+            return (types & VIRTIO_NET_RSS_HASH_TYPE_UDP_EX) ?
+                NetPktRssIpV6UdpEx : NetPktRssIpV6Udp;
+        }
+        mask = VIRTIO_NET_RSS_HASH_TYPE_IP_EX | VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+        if (types & mask) {
+            return (types & VIRTIO_NET_RSS_HASH_TYPE_IP_EX) ?
+                NetPktRssIpV6Ex : NetPktRssIpV6;
+        }
+    }
+    return 0xff;
+}
+
+static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
+                                  size_t size)
+{
+    VirtIONet *n = qemu_get_nic_opaque(nc);
+    unsigned int index = nc->queue_index, new_index;
+    struct NetRxPkt *pkt = n->rss_data.pkt;
+    uint8_t net_hash_type;
+    uint32_t hash;
+    bool isip4, isip6, isudp, istcp;
+    net_rx_pkt_set_protocols(pkt, buf + n->host_hdr_len,
+                             size - n->host_hdr_len);
+    net_rx_pkt_get_protocols(pkt, &isip4, &isip6, &isudp, &istcp);
+    if (isip4 && (net_rx_pkt_get_ip4_info(pkt)->fragment)) {
+        istcp = isudp = false;
+    }
+    if (isip6 && (net_rx_pkt_get_ip6_info(pkt)->fragment)) {
+        istcp = isudp = false;
+    }
+    net_hash_type = virtio_net_get_hash_type(isip4, isip6, isudp, istcp,
+                                             n->rss_data.hash_types);
+    if (net_hash_type > NetPktRssIpV6UdpEx) {
+        return n->rss_data.default_queue;
+    }
+
+    hash = net_rx_pkt_calc_rss_hash(pkt, net_hash_type, n->rss_data.key);
+    new_index = hash & (n->rss_data.indirections_len - 1);
+    new_index = n->rss_data.indirections[new_index];
+    if (index == new_index) {
+        return -1;
+    }
+    return new_index;
+}
+
 static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
-                                      size_t size)
+                                      size_t size, bool no_rss)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
     VirtIONetQueue *q = virtio_net_get_subqueue(nc);
@@ -1625,6 +1696,14 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
         return -1;
     }
 
+    if (!no_rss && n->rss_data.enabled) {
+        int index = virtio_net_process_rss(nc, buf, size);
+        if (index >= 0) {
+            NetClientState *nc2 = qemu_get_subqueue(n->nic, index);
+            return virtio_net_receive_rcu(nc2, buf, size, true);
+        }
+    }
+
     /* hdr_len refers to the header we supply to the guest */
     if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
         return 0;
@@ -1719,7 +1798,7 @@ static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
 {
     RCU_READ_LOCK_GUARD();
 
-    return virtio_net_receive_rcu(nc, buf, size);
+    return virtio_net_receive_rcu(nc, buf, size, false);
 }
 
 static void virtio_net_rsc_extract_unit4(VirtioNetRscChain *chain,
@@ -3295,6 +3374,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
 
     QTAILQ_INIT(&n->rsc_chains);
     n->qdev = dev;
+
+    net_rx_pkt_init(&n->rss_data.pkt, false);
 }
 
 static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
@@ -3331,6 +3412,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_net_rsc_cleanup(n);
+    net_rx_pkt_uninit(n->rss_data.pkt);
     virtio_cleanup(vdev);
 }
 
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index cf16f5192e..45670dd054 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -209,6 +209,7 @@ struct VirtIONet {
         uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
         uint16_t indirections_len;
         uint16_t default_queue;
+        struct NetRxPkt *pkt;
     } rss_data;
 };
 
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated
  2020-03-09  8:34 [PATCH v2 0/4] reference implementation of RSS Yuri Benditovich
                   ` (2 preceding siblings ...)
  2020-03-09  8:34 ` [PATCH v2 3/4] virtio-net: implement RX RSS processing Yuri Benditovich
@ 2020-03-09  8:34 ` Yuri Benditovich
  2020-03-10  3:12   ` Jason Wang
  2020-03-09  8:59 ` [PATCH v2 0/4] reference implementation of RSS no-reply
  4 siblings, 1 reply; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-09  8:34 UTC (permalink / raw)
  To: qemu-devel, mst, jasowang; +Cc: yan

Block migration for reference implementation of
RSS feature in QEMU. When we add support for RSS
on backend side, we'll implement migration of
current RSS settings.

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 hw/net/virtio-net.c            | 18 ++++++++++++++++++
 include/hw/virtio/virtio-net.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index abc41fdb16..943d1931a2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -37,6 +37,7 @@
 #include "qapi/qapi-events-migration.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/misc.h"
+#include "migration/blocker.h"
 #include "standard-headers/linux/ethtool.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
@@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev)
     n->announce_timer.round = 0;
     n->status &= ~VIRTIO_NET_S_ANNOUNCE;
 
+    if (n->migration_blocker) {
+        migrate_del_blocker(n->migration_blocker);
+        error_free(n->migration_blocker);
+        n->migration_blocker = NULL;
+    }
+
     /* Flush any MAC and VLAN filter table state */
     n->mac_table.in_use = 0;
     n->mac_table.first_multi = 0;
@@ -1003,6 +1010,17 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
         vhost_net_ack_features(get_vhost_net(nc->peer), features);
     }
 
+    if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
+        if (!n->migration_blocker) {
+            error_setg(&n->migration_blocker, "virtio-net: RSS negotiated");
+            migrate_add_blocker(n->migration_blocker, &err);
+            if (err) {
+                error_report_err(err);
+                err = NULL;
+            }
+        }
+    }
+
     if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
         memset(n->vlans, 0, MAX_VLAN >> 3);
     } else {
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 45670dd054..fba768ba9b 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -180,6 +180,7 @@ struct VirtIONet {
     virtio_net_conf net_conf;
     NICConf nic_conf;
     DeviceState *qdev;
+    Error *migration_blocker;
     int multiqueue;
     uint16_t max_queues;
     uint16_t curr_queues;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 0/4] reference implementation of RSS
  2020-03-09  8:34 [PATCH v2 0/4] reference implementation of RSS Yuri Benditovich
                   ` (3 preceding siblings ...)
  2020-03-09  8:34 ` [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated Yuri Benditovich
@ 2020-03-09  8:59 ` no-reply
  4 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2020-03-09  8:59 UTC (permalink / raw)
  To: yuri.benditovich; +Cc: yan, jasowang, qemu-devel, mst

Patchew URL: https://patchew.org/QEMU/20200309083438.2389-1-yuri.benditovich@daynix.com/



Hi,

This series failed the docker-clang@ubuntu build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-ubuntu V=1 NETWORK=1
time make docker-test-clang@ubuntu SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  LINK    qemu-bridge-helper
  LINK    virtiofsd
  LINK    vhost-user-input
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  GEN     lm32-softmmu/hmp-commands.h
  GEN     lm32-softmmu/hmp-commands-info.h
  GEN     lm32-softmmu/config-devices.h
---
  CC      mips-softmmu/hw/vfio/pci-quirks.o
  CC      alpha-softmmu/hw/virtio/virtio.o
  CC      i386-softmmu/hw/vfio/common.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      arm-softmmu/hw/scsi/virtio-scsi.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      mipsel-softmmu/balloon.o
  CC      mips-softmmu/hw/vfio/display.o
  CC      alpha-softmmu/hw/virtio/vhost.o
---
  CC      mipsel-softmmu/hw/mips/mips_malta.o
  CC      arm-softmmu/hw/arm/bcm2836.o
  CC      nios2-softmmu/target/nios2/monitor.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  GEN     trace/generated-helpers.c
  CC      mips64-softmmu/target/mips/machine.o
  CC      i386-softmmu/target/i386/bpt_helper.o
---
  GEN     ppc64-softmmu/config-target.h
  CC      ppc64-softmmu/exec.o
  CC      i386-softmmu/target/i386/seg_helper.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      mipsel-softmmu/qapi/qapi-events-misc-target.o
  CC      arm-softmmu/hw/arm/aspeed_ast2600.o
  CC      aarch64-softmmu/hw/arm/fsl-imx25.o
---
  CC      ppc64-softmmu/exec-vary.o
  CC      arm-softmmu/hw/arm/msf2-soc.o
  CC      ppc-softmmu/hw/scsi/vhost-scsi.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      aarch64-softmmu/hw/arm/kzm.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      i386-softmmu/target/i386/arch_dump.o
  CC      mipsel-softmmu/qapi/qapi-init-commands.o
  CC      ppc64-softmmu/tcg/tcg.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      arm-softmmu/hw/arm/musca.o
  CC      mipsel-softmmu/softmmu/vl.o
  CC      ppc-softmmu/hw/scsi/vhost-user-scsi.o
---
  CC      mipsel-softmmu/target/mips/translate.o
  CC      aarch64-softmmu/hw/arm/fsl-imx6.o
  CC      i386-softmmu/target/i386/kvm.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      ppc-softmmu/hw/vfio/common.o
  CC      ppc64-softmmu/tcg/tcg-op-vec.o
  GEN     riscv32-softmmu/hmp-commands.h
---
  CC      riscv64-softmmu/hw/char/virtio-serial-bus.o
  CC      arm-softmmu/target/arm/m_helper.o
  CC      ppc-softmmu/hw/ppc/rs6000_mc.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      tricore-softmmu/tcg/tcg.o
  CC      s390x-softmmu/accel/tcg/translator.o
  CC      sparc-softmmu/accel/tcg/translate-all.o
---
  CC      ppc-softmmu/hw/ppc/e500.o
  CC      aarch64-softmmu/target/arm/helper-a64.o
  CC      arm-softmmu/gdbstub-xml.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      sparc-softmmu/hw/intc/grlib_irqmp.o
  GEN     aarch64-softmmu/target/arm/decode-sve.inc.c
  CC      sh4eb-softmmu/hw/display/virtio-gpu-base.o
---
  CC      riscv64-softmmu/hw/virtio/vhost-user-fs.o
  CC      unicore32-softmmu/fpu/softfloat.o
  CC      ppc64-softmmu/hw/vfio/amd-xgbe.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      riscv32-softmmu/hw/virtio/virtio-blk-pci.o
  CC      s390x-softmmu/hw/virtio/vhost-backend.o
  CC      unicore32-softmmu/disas.o
---
  CC      s390x-softmmu/hw/virtio/virtio-crypto-pci.o
  CC      unicore32-softmmu/accel/stubs/hax-stub.o
  CC      sh4eb-softmmu/hw/virtio/vhost-user-fs-pci.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      sparc64-softmmu/hw/virtio/vhost-user-fs-pci.o
  CC      sh4eb-softmmu/hw/virtio/virtio-iommu.o
  CC      riscv32-softmmu/hw/riscv/sifive_e_prci.o
---
  CC      riscv64-softmmu/hw/riscv/sifive_e.o
  CC      riscv64-softmmu/hw/riscv/sifive_e_prci.o
  CC      unicore32-softmmu/qapi/qapi-events-machine-target.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      riscv64-softmmu/hw/riscv/sifive_clint.o
  CC      riscv64-softmmu/hw/riscv/sifive_gpio.o
  CC      s390x-softmmu/hw/virtio/virtio-serial-pci.o
---
  CC      sparc64-softmmu/target/sparc/win_helper.o
  CC      ppc64-softmmu/hw/ppc/spapr_ovec.o
  CC      sh4eb-softmmu/target/sh4/cpu.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      riscv64-softmmu/target/riscv/fpu_helper.o
  LINK    riscv32-softmmu/qemu-system-riscv32
  CC      x86_64-softmmu/accel/stubs/hvf-stub.o
---
  LINK    sh4eb-softmmu/qemu-system-sh4eb
  CC      sparc64-softmmu/softmmu/main.o
  CC      riscv64-softmmu/target/riscv/translate.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      x86_64-softmmu/accel/tcg/cpu-exec-common.o
  CC      sparc64-softmmu/trace/generated-helpers.o
  CC      ppc64-softmmu/hw/ppc/pnv_xscom.o
  CC      ppc64-softmmu/hw/ppc/pnv_core.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      riscv64-softmmu/trace/generated-helpers.o
  GEN     xtensa-softmmu/hmp-commands.h
  GEN     xtensa-softmmu/hmp-commands-info.h
---
  CC      xtensa-softmmu/disas.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-3d.o
  CC      ppc64-softmmu/hw/ppc/ppc4xx_devs.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      xtensa-softmmu/arch_init.o
  GEN     aarch64-linux-user/config-target.h
  CC      x86_64-softmmu/hw/display/vhost-user-gpu.o
  CC      aarch64-linux-user/exec.o
  CC      ppc64-softmmu/hw/ppc/sam460ex.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      xtensa-softmmu/cpus.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-pci.o
  CC      xtensaeb-softmmu/exec-vary.o
  CC      ppc64-softmmu/hw/ppc/prep.o
  CC      xtensaeb-softmmu/tcg/tcg.o
  CC      xtensa-softmmu/gdbstub.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      x86_64-softmmu/hw/display/vhost-user-gpu-pci.o
  CC      ppc64-softmmu/hw/ppc/prep_systemio.o
  CC      ppc64-softmmu/hw/ppc/rs6000_mc.o
---
  CC      cris-linux-user/accel/tcg/user-exec.o
  GEN     arm-linux-user/target/arm/decode-vfp-uncond.inc.c
  CC      aarch64-linux-user/target/arm/iwmmxt_helper.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      hppa-linux-user/accel/stubs/hvf-stub.o
  GEN     armeb-linux-user/target/arm/decode-vfp-uncond.inc.c
  GEN     armeb-linux-user/target/arm/decode-a32.inc.c
---
  CC      xtensa-softmmu/hw/virtio/virtio-input-pci.o
  CC      i386-linux-user/accel/stubs/whpx-stub.o
  CC      xtensa-softmmu/hw/virtio/virtio-rng-pci.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
  CC      mips-linux-user/tcg/tcg.o
  CC      m68k-linux-user/disas.o
  CC      hppa-linux-user/target/hppa/gdbstub.o
---
  CC      microblazeel-linux-user/tcg/optimize.o
  CC      mips-linux-user/fpu/softfloat.o
  CC      x86_64-softmmu/hw/i386/port92.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [Makefile:207: qemu-system-s390x] Error 1
make: *** [Makefile:530: s390x-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  CC      x86_64-softmmu/hw/i386/kvmvapic.o
  CC      m68k-linux-user/accel/tcg/cpu-exec-common.o
---
  LINK    mips64-linux-user/qemu-mips64
  LINK    xtensaeb-softmmu/qemu-system-xtensaeb
  LINK    x86_64-softmmu/qemu-system-x86_64
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of `rpc_createerr@@GLIBC_2.2.5' overridden by definition from /lib/x86_64-linux-gnu/libc.so.6
rm tests/qemu-iotests/socket_scm_helper.o
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=8214e750af21459baf3a5692f45951b0', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-8fx86d27/src/docker-src.2020-03-09-04.52.32.16002:/var/tmp/qemu:z,ro', 'qemu:ubuntu', '/var/tmp/qemu/run', 'test-clang']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=8214e750af21459baf3a5692f45951b0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-8fx86d27/src'
make: *** [docker-run-test-clang@ubuntu] Error 2

real    6m42.157s
user    0m8.567s


The full log is available at
http://patchew.org/logs/20200309083438.2389-1-yuri.benditovich@daynix.com/testing.docker-clang@ubuntu/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/4] virtio-net: implement RSS configuration command
  2020-03-09  8:34 ` [PATCH v2 2/4] virtio-net: implement RSS configuration command Yuri Benditovich
@ 2020-03-10  3:02   ` Jason Wang
  2020-03-10 10:29     ` Yuri Benditovich
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2020-03-10  3:02 UTC (permalink / raw)
  To: Yuri Benditovich, qemu-devel, mst; +Cc: yan


On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> Optionally report RSS feature.
> Handle RSS configuration command and keep RSS parameters
> in virtio-net device context.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>   hw/net/trace-events            |   3 +
>   hw/net/virtio-net.c            | 148 +++++++++++++++++++++++++++++++--
>   include/hw/virtio/virtio-net.h |  11 +++
>   3 files changed, 153 insertions(+), 9 deletions(-)
>
> diff --git a/hw/net/trace-events b/hw/net/trace-events
> index a1da98a643..9823480d91 100644
> --- a/hw/net/trace-events
> +++ b/hw/net/trace-events
> @@ -371,6 +371,9 @@ virtio_net_announce_notify(void) ""
>   virtio_net_announce_timer(int round) "%d"
>   virtio_net_handle_announce(int round) "%d"
>   virtio_net_post_load_device(void)
> +virtio_net_rss_disable(void)
> +virtio_net_rss_error(int error_case) "case %d"
> +virtio_net_rss_enable(uint32_t p1, uint16_t p2, uint8_t p3) "hashes 0x%x, table of %d, key of %d"
>   
>   # tulip.c
>   tulip_reg_write(uint64_t addr, const char *name, int size, uint64_t val) "addr 0x%02"PRIx64" (%s) size %d value 0x%08"PRIx64
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 9545b0e84f..27071eccd2 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -172,6 +172,16 @@ struct virtio_net_rss_config {
>      tso/gso/gro 'off'. */
>   #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
>   
> +#define VIRTIO_NET_RSS_SUPPORTED_HASHES (VIRTIO_NET_RSS_HASH_TYPE_IPv4 | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_TCPv4 | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_UDPv4 | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_IPv6 | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_TCPv6 | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_UDPv6 | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_IP_EX | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | \
> +                                         VIRTIO_NET_RSS_HASH_TYPE_UDP_EX)
> +
>   /* temporary until standard header include it */
>   #if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
>   
> @@ -203,6 +213,8 @@ static VirtIOFeature feature_sizes[] = {
>        .end = endof(struct virtio_net_config, mtu)},
>       {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX,
>        .end = endof(struct virtio_net_config, duplex)},
> +    {.flags = 1ULL << VIRTIO_NET_F_RSS,
> +     .end = endof(struct virtio_net_config, supported_hash_types)},
>       {}
>   };
>   
> @@ -233,6 +245,11 @@ static void virtio_net_get_config(VirtIODevice *vdev, uint8_t *config)
>       memcpy(netcfg.mac, n->mac, ETH_ALEN);
>       virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
>       netcfg.duplex = n->net_conf.duplex;
> +    netcfg.rss_max_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
> +    virtio_stw_p(vdev, &netcfg.rss_max_indirection_table_length,
> +                 VIRTIO_NET_RSS_MAX_TABLE_LEN);
> +    virtio_stl_p(vdev, &netcfg.supported_hash_types,
> +                 VIRTIO_NET_RSS_SUPPORTED_HASHES);
>       memcpy(config, &netcfg, n->config_size);
>   }
>   
> @@ -796,6 +813,7 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features,
>           return features;
>       }
>   
> +    virtio_clear_feature(&features, VIRTIO_NET_F_RSS);
>       features = vhost_net_get_features(get_vhost_net(nc->peer), features);
>       vdev->backend_features = features;
>   
> @@ -955,6 +973,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>       }
>   
>       virtio_net_set_multiqueue(n,
> +                              virtio_has_feature(features, VIRTIO_NET_F_RSS) ||
>                                 virtio_has_feature(features, VIRTIO_NET_F_MQ));
>   
>       virtio_net_set_mrg_rx_bufs(n,
> @@ -1231,25 +1250,134 @@ static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
>       }
>   }
>   
> +static void virtio_net_disable_rss(VirtIONet *n)
> +{
> +    if (n->rss_data.enabled) {
> +        trace_virtio_net_rss_disable();
> +    }
> +    n->rss_data.enabled = false;
> +}
> +
> +static uint16_t virtio_net_handle_rss(VirtIONet *n,
> +                                      struct iovec *iov, unsigned int iov_cnt)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
> +    struct virtio_net_rss_config cfg;
> +    size_t s, offset = 0, size_get;
> +    uint16_t queues, i;
> +    struct {
> +        uint16_t us;
> +        uint8_t b;
> +    } QEMU_PACKED temp;
> +    int err;
> +
> +    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> +        err = 1;
> +        goto error;
> +    }
> +    size_get = offsetof(struct virtio_net_rss_config, indirection_table);
> +    s = iov_to_buf(iov, iov_cnt, offset, &cfg, size_get);
> +    if (s != size_get) {
> +        err = 2;
> +        goto error;
> +    }
> +    n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
> +    n->rss_data.indirections_len =
> +        virtio_lduw_p(vdev, &cfg.indirection_table_mask);
> +    n->rss_data.indirections_len++;
> +    if (!is_power_of_2(n->rss_data.indirections_len)) {
> +        err = 3;
> +        goto error;
> +    }
> +    if (n->rss_data.indirections_len > VIRTIO_NET_RSS_MAX_TABLE_LEN) {
> +        err = 4;
> +        goto error;
> +    }
> +    n->rss_data.default_queue =
> +        virtio_lduw_p(vdev, &cfg.unclassified_queue);
> +    if (n->rss_data.default_queue >= n->max_queues) {
> +        err = 5;
> +        goto error;
> +    }
> +    offset += size_get;
> +    size_get = sizeof(uint16_t) * n->rss_data.indirections_len;
> +    s = iov_to_buf(iov, iov_cnt, offset, n->rss_data.indirections, size_get);
> +    if (s != size_get) {
> +        err = 10;
> +        goto error;
> +    }
> +    for (i = 0; i < n->rss_data.indirections_len; ++i) {
> +        uint16_t val = n->rss_data.indirections[i];
> +        n->rss_data.indirections[i] = virtio_lduw_p(vdev, &val);
> +    }
> +    offset += size_get;
> +    size_get = sizeof(temp);
> +    s = iov_to_buf(iov, iov_cnt, offset, &temp, size_get);
> +    if (s != size_get) {
> +        err = 11;
> +        goto error;
> +    }
> +    queues = virtio_lduw_p(vdev, &temp.us);
> +    if (queues == 0 || queues > n->max_queues) {
> +        err = 12;
> +        goto error;
> +    }
> +    if (temp.b > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> +        err = 13;
> +        goto error;
> +    }
> +    if (!temp.b && n->rss_data.hash_types) {
> +        err = 20;
> +        goto error;
> +    }
> +    if (!temp.b && !n->rss_data.hash_types) {
> +        virtio_net_disable_rss(n);
> +        return queues;
> +    }
> +    offset += size_get;
> +    size_get = temp.b;
> +    s = iov_to_buf(iov, iov_cnt, offset, n->rss_data.key, size_get);
> +    if (s != size_get) {
> +        err = 21;
> +        goto error;
> +    }
> +    n->rss_data.enabled = true;
> +    trace_virtio_net_rss_enable(n->rss_data.hash_types,
> +                                n->rss_data.indirections_len,
> +                                temp.b);
> +    return queues;
> +error:
> +    warn_report("%s: error_case %d", __func__, err);


I'm not sure using warn_report() is good for such guest triggerable 
behavior.


> +    trace_virtio_net_rss_error(err);


It looks to me it would be better to be verbose here (show temp.b or other)


> +    virtio_net_disable_rss(n);
> +    return 0;
> +}
> +
>   static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
>                                   struct iovec *iov, unsigned int iov_cnt)
>   {
>       VirtIODevice *vdev = VIRTIO_DEVICE(n);
> -    struct virtio_net_ctrl_mq mq;
> -    size_t s;
>       uint16_t queues;
>   
> -    s = iov_to_buf(iov, iov_cnt, 0, &mq, sizeof(mq));
> -    if (s != sizeof(mq)) {
> -        return VIRTIO_NET_ERR;
> -    }
> +    virtio_net_disable_rss(n);
> +    if (cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
> +        queues = virtio_net_handle_rss(n, iov, iov_cnt);
> +    } else if (cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {


It looks to me RSS and MQ are mutually exclusive, is this intentional?


> +        struct virtio_net_ctrl_mq mq;
> +        size_t s;
> +        if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ)) {
> +            return VIRTIO_NET_ERR;
> +        }
> +        s = iov_to_buf(iov, iov_cnt, 0, &mq, sizeof(mq));
> +        if (s != sizeof(mq)) {
> +            return VIRTIO_NET_ERR;
> +        }
> +        queues = virtio_lduw_p(vdev, &mq.virtqueue_pairs);
>   
> -    if (cmd != VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
> +    } else {
>           return VIRTIO_NET_ERR;
>       }
>   
> -    queues = virtio_lduw_p(vdev, &mq.virtqueue_pairs);
> -
>       if (queues < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
>           queues > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
>           queues > n->max_queues ||
> @@ -3304,6 +3432,8 @@ static Property virtio_net_properties[] = {
>       DEFINE_PROP_BIT64("ctrl_guest_offloads", VirtIONet, host_features,
>                       VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true),
>       DEFINE_PROP_BIT64("mq", VirtIONet, host_features, VIRTIO_NET_F_MQ, false),
> +    DEFINE_PROP_BIT64("rss", VirtIONet, host_features,
> +                    VIRTIO_NET_F_RSS, false),
>       DEFINE_PROP_BIT64("guest_rsc_ext", VirtIONet, host_features,
>                       VIRTIO_NET_F_RSC_EXT, false),
>       DEFINE_PROP_UINT32("rsc_interval", VirtIONet, rsc_timeout,
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 96c68d4a92..cf16f5192e 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -126,6 +126,9 @@ typedef struct VirtioNetRscChain {
>   /* Maximum packet size we can receive from tap device: header + 64k */
>   #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 * KiB))
>   
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> +
>   typedef struct VirtIONetQueue {
>       VirtQueue *rx_vq;
>       VirtQueue *tx_vq;
> @@ -199,6 +202,14 @@ struct VirtIONet {
>       bool failover;
>       DeviceListener primary_listener;
>       Notifier migration_state;
> +    struct {
> +        bool    enabled;
> +        uint32_t hash_types;
> +        uint8_t key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +        uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> +        uint16_t indirections_len;
> +        uint16_t default_queue;
> +    } rss_data;
>   };
>   
>   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] virtio-net: implement RX RSS processing
  2020-03-09  8:34 ` [PATCH v2 3/4] virtio-net: implement RX RSS processing Yuri Benditovich
@ 2020-03-10  3:10   ` Jason Wang
  2020-03-10 10:18     ` Yuri Benditovich
  2020-03-10  6:13   ` Michael S. Tsirkin
  1 sibling, 1 reply; 14+ messages in thread
From: Jason Wang @ 2020-03-10  3:10 UTC (permalink / raw)
  To: Yuri Benditovich, qemu-devel, mst; +Cc: yan


On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> If VIRTIO_NET_F_RSS negotiated and RSS is enabled, process
> incoming packets, calculate packet's hash and place the
> packet into respective RX virtqueue.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>   hw/net/virtio-net.c            | 86 +++++++++++++++++++++++++++++++++-
>   include/hw/virtio/virtio-net.h |  1 +
>   2 files changed, 85 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 27071eccd2..abc41fdb16 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -42,6 +42,7 @@
>   #include "trace.h"
>   #include "monitor/qdev.h"
>   #include "hw/pci/pci.h"
> +#include "net_rx_pkt.h"
>   
>   #define VIRTIO_NET_VM_VERSION    11
>   
> @@ -1610,8 +1611,78 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
>       return 0;
>   }
>   
> +static uint8_t virtio_net_get_hash_type(bool isip4,
> +                                        bool isip6,
> +                                        bool isudp,
> +                                        bool istcp,
> +                                        uint32_t types)
> +{
> +    uint32_t mask;
> +    if (isip4) {
> +        if (istcp && (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv4)) {
> +            return NetPktRssIpV4Tcp;
> +        }
> +        if (isudp && (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv4)) {
> +            return NetPktRssIpV4Udp;
> +        }
> +        if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> +            return NetPktRssIpV4;
> +        }
> +    } else if (isip6) {
> +        mask = VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> +        if (istcp && (types & mask)) {
> +            return (types & VIRTIO_NET_RSS_HASH_TYPE_TCP_EX) ?
> +                NetPktRssIpV6TcpEx : NetPktRssIpV6Tcp;
> +        }
> +        mask = VIRTIO_NET_RSS_HASH_TYPE_UDP_EX | VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> +        if (isudp && (types & mask)) {
> +            return (types & VIRTIO_NET_RSS_HASH_TYPE_UDP_EX) ?
> +                NetPktRssIpV6UdpEx : NetPktRssIpV6Udp;
> +        }
> +        mask = VIRTIO_NET_RSS_HASH_TYPE_IP_EX | VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +        if (types & mask) {
> +            return (types & VIRTIO_NET_RSS_HASH_TYPE_IP_EX) ?
> +                NetPktRssIpV6Ex : NetPktRssIpV6;
> +        }
> +    }
> +    return 0xff;
> +}
> +
> +static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
> +                                  size_t size)
> +{
> +    VirtIONet *n = qemu_get_nic_opaque(nc);
> +    unsigned int index = nc->queue_index, new_index;
> +    struct NetRxPkt *pkt = n->rss_data.pkt;
> +    uint8_t net_hash_type;
> +    uint32_t hash;
> +    bool isip4, isip6, isudp, istcp;
> +    net_rx_pkt_set_protocols(pkt, buf + n->host_hdr_len,
> +                             size - n->host_hdr_len);
> +    net_rx_pkt_get_protocols(pkt, &isip4, &isip6, &isudp, &istcp);
> +    if (isip4 && (net_rx_pkt_get_ip4_info(pkt)->fragment)) {
> +        istcp = isudp = false;
> +    }
> +    if (isip6 && (net_rx_pkt_get_ip6_info(pkt)->fragment)) {
> +        istcp = isudp = false;
> +    }
> +    net_hash_type = virtio_net_get_hash_type(isip4, isip6, isudp, istcp,
> +                                             n->rss_data.hash_types);
> +    if (net_hash_type > NetPktRssIpV6UdpEx) {
> +        return n->rss_data.default_queue;
> +    }
> +
> +    hash = net_rx_pkt_calc_rss_hash(pkt, net_hash_type, n->rss_data.key);
> +    new_index = hash & (n->rss_data.indirections_len - 1);
> +    new_index = n->rss_data.indirections[new_index];
> +    if (index == new_index) {
> +        return -1;
> +    }
> +    return new_index;
> +}
> +
>   static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> -                                      size_t size)
> +                                      size_t size, bool no_rss)
>   {
>       VirtIONet *n = qemu_get_nic_opaque(nc);
>       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> @@ -1625,6 +1696,14 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>           return -1;
>       }
>   
> +    if (!no_rss && n->rss_data.enabled) {
> +        int index = virtio_net_process_rss(nc, buf, size);
> +        if (index >= 0) {
> +            NetClientState *nc2 = qemu_get_subqueue(n->nic, index);
> +            return virtio_net_receive_rcu(nc2, buf, size, true);
> +        }
> +    }


In the long run, we need to implement steering ops and allow device 
model to implement their own policy instead of doing hack like this.

Thanks


> +
>       /* hdr_len refers to the header we supply to the guest */
>       if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
>           return 0;
> @@ -1719,7 +1798,7 @@ static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
>   {
>       RCU_READ_LOCK_GUARD();
>   
> -    return virtio_net_receive_rcu(nc, buf, size);
> +    return virtio_net_receive_rcu(nc, buf, size, false);
>   }
>   
>   static void virtio_net_rsc_extract_unit4(VirtioNetRscChain *chain,
> @@ -3295,6 +3374,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>   
>       QTAILQ_INIT(&n->rsc_chains);
>       n->qdev = dev;
> +
> +    net_rx_pkt_init(&n->rss_data.pkt, false);
>   }
>   
>   static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
> @@ -3331,6 +3412,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
>       g_free(n->vqs);
>       qemu_del_nic(n->nic);
>       virtio_net_rsc_cleanup(n);
> +    net_rx_pkt_uninit(n->rss_data.pkt);
>       virtio_cleanup(vdev);
>   }
>   
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index cf16f5192e..45670dd054 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -209,6 +209,7 @@ struct VirtIONet {
>           uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
>           uint16_t indirections_len;
>           uint16_t default_queue;
> +        struct NetRxPkt *pkt;
>       } rss_data;
>   };
>   



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated
  2020-03-09  8:34 ` [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated Yuri Benditovich
@ 2020-03-10  3:12   ` Jason Wang
  2020-03-10  6:17     ` Michael S. Tsirkin
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2020-03-10  3:12 UTC (permalink / raw)
  To: Yuri Benditovich, qemu-devel, mst; +Cc: yan


On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> Block migration for reference implementation of
> RSS feature in QEMU. When we add support for RSS
> on backend side, we'll implement migration of
> current RSS settings.
>
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>   hw/net/virtio-net.c            | 18 ++++++++++++++++++
>   include/hw/virtio/virtio-net.h |  1 +
>   2 files changed, 19 insertions(+)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index abc41fdb16..943d1931a2 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -37,6 +37,7 @@
>   #include "qapi/qapi-events-migration.h"
>   #include "hw/virtio/virtio-access.h"
>   #include "migration/misc.h"
> +#include "migration/blocker.h"
>   #include "standard-headers/linux/ethtool.h"
>   #include "sysemu/sysemu.h"
>   #include "trace.h"
> @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev)
>       n->announce_timer.round = 0;
>       n->status &= ~VIRTIO_NET_S_ANNOUNCE;
>   
> +    if (n->migration_blocker) {
> +        migrate_del_blocker(n->migration_blocker);
> +        error_free(n->migration_blocker);
> +        n->migration_blocker = NULL;
> +    }
> +
>       /* Flush any MAC and VLAN filter table state */
>       n->mac_table.in_use = 0;
>       n->mac_table.first_multi = 0;
> @@ -1003,6 +1010,17 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
>           vhost_net_ack_features(get_vhost_net(nc->peer), features);
>       }
>   
> +    if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
> +        if (!n->migration_blocker) {
> +            error_setg(&n->migration_blocker, "virtio-net: RSS negotiated");
> +            migrate_add_blocker(n->migration_blocker, &err);
> +            if (err) {
> +                error_report_err(err);
> +                err = NULL;
> +            }
> +        }
> +    }


It looks to me we should add the blocker unconditionally based on 
virtio_host_has_feature() instead of depending the negotiated feature here.

Thanks


> +
>       if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
>           memset(n->vlans, 0, MAX_VLAN >> 3);
>       } else {
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index 45670dd054..fba768ba9b 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -180,6 +180,7 @@ struct VirtIONet {
>       virtio_net_conf net_conf;
>       NICConf nic_conf;
>       DeviceState *qdev;
> +    Error *migration_blocker;
>       int multiqueue;
>       uint16_t max_queues;
>       uint16_t curr_queues;



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] virtio-net: implement RX RSS processing
  2020-03-09  8:34 ` [PATCH v2 3/4] virtio-net: implement RX RSS processing Yuri Benditovich
  2020-03-10  3:10   ` Jason Wang
@ 2020-03-10  6:13   ` Michael S. Tsirkin
  1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2020-03-10  6:13 UTC (permalink / raw)
  To: Yuri Benditovich; +Cc: yan, jasowang, qemu-devel

On Mon, Mar 09, 2020 at 10:34:37AM +0200, Yuri Benditovich wrote:
> If VIRTIO_NET_F_RSS negotiated and RSS is enabled, process
> incoming packets, calculate packet's hash and place the
> packet into respective RX virtqueue.
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  hw/net/virtio-net.c            | 86 +++++++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-net.h |  1 +
>  2 files changed, 85 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 27071eccd2..abc41fdb16 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -42,6 +42,7 @@
>  #include "trace.h"
>  #include "monitor/qdev.h"
>  #include "hw/pci/pci.h"
> +#include "net_rx_pkt.h"
>  
>  #define VIRTIO_NET_VM_VERSION    11
>  
> @@ -1610,8 +1611,78 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
>      return 0;
>  }
>  
> +static uint8_t virtio_net_get_hash_type(bool isip4,
> +                                        bool isip6,
> +                                        bool isudp,
> +                                        bool istcp,
> +                                        uint32_t types)
> +{
> +    uint32_t mask;
> +    if (isip4) {


An empty line after a declaration won't hurt, here and elsewhere.


> +        if (istcp && (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv4)) {
> +            return NetPktRssIpV4Tcp;
> +        }
> +        if (isudp && (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv4)) {
> +            return NetPktRssIpV4Udp;
> +        }
> +        if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> +            return NetPktRssIpV4;
> +        }
> +    } else if (isip6) {
> +        mask = VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | VIRTIO_NET_RSS_HASH_TYPE_TCPv6;

Mask is only used on this path, move declaration within if ()?

> +        if (istcp && (types & mask)) {
> +            return (types & VIRTIO_NET_RSS_HASH_TYPE_TCP_EX) ?
> +                NetPktRssIpV6TcpEx : NetPktRssIpV6Tcp;
> +        }
> +        mask = VIRTIO_NET_RSS_HASH_TYPE_UDP_EX | VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> +        if (isudp && (types & mask)) {
> +            return (types & VIRTIO_NET_RSS_HASH_TYPE_UDP_EX) ?
> +                NetPktRssIpV6UdpEx : NetPktRssIpV6Udp;
> +        }
> +        mask = VIRTIO_NET_RSS_HASH_TYPE_IP_EX | VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +        if (types & mask) {
> +            return (types & VIRTIO_NET_RSS_HASH_TYPE_IP_EX) ?
> +                NetPktRssIpV6Ex : NetPktRssIpV6;
> +        }
> +    }
> +    return 0xff;
> +}
> +
> +static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
> +                                  size_t size)
> +{
> +    VirtIONet *n = qemu_get_nic_opaque(nc);
> +    unsigned int index = nc->queue_index, new_index;
> +    struct NetRxPkt *pkt = n->rss_data.pkt;
> +    uint8_t net_hash_type;
> +    uint32_t hash;
> +    bool isip4, isip6, isudp, istcp;
> +    net_rx_pkt_set_protocols(pkt, buf + n->host_hdr_len,
> +                             size - n->host_hdr_len);
> +    net_rx_pkt_get_protocols(pkt, &isip4, &isip6, &isudp, &istcp);
> +    if (isip4 && (net_rx_pkt_get_ip4_info(pkt)->fragment)) {
> +        istcp = isudp = false;
> +    }
> +    if (isip6 && (net_rx_pkt_get_ip6_info(pkt)->fragment)) {
> +        istcp = isudp = false;
> +    }
> +    net_hash_type = virtio_net_get_hash_type(isip4, isip6, isudp, istcp,
> +                                             n->rss_data.hash_types);
> +    if (net_hash_type > NetPktRssIpV6UdpEx) {
> +        return n->rss_data.default_queue;
> +    }
> +
> +    hash = net_rx_pkt_calc_rss_hash(pkt, net_hash_type, n->rss_data.key);
> +    new_index = hash & (n->rss_data.indirections_len - 1);
> +    new_index = n->rss_data.indirections[new_index];
> +    if (index == new_index) {
> +        return -1;
> +    }
> +    return new_index;
> +}
> +
>  static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> -                                      size_t size)
> +                                      size_t size, bool no_rss)
>  {
>      VirtIONet *n = qemu_get_nic_opaque(nc);
>      VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> @@ -1625,6 +1696,14 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>          return -1;
>      }
>  
> +    if (!no_rss && n->rss_data.enabled) {
> +        int index = virtio_net_process_rss(nc, buf, size);
> +        if (index >= 0) {
> +            NetClientState *nc2 = qemu_get_subqueue(n->nic, index);
> +            return virtio_net_receive_rcu(nc2, buf, size, true);
> +        }
> +    }
> +
>      /* hdr_len refers to the header we supply to the guest */
>      if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
>          return 0;
> @@ -1719,7 +1798,7 @@ static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
>  {
>      RCU_READ_LOCK_GUARD();
>  
> -    return virtio_net_receive_rcu(nc, buf, size);
> +    return virtio_net_receive_rcu(nc, buf, size, false);
>  }
>  
>  static void virtio_net_rsc_extract_unit4(VirtioNetRscChain *chain,
> @@ -3295,6 +3374,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>  
>      QTAILQ_INIT(&n->rsc_chains);
>      n->qdev = dev;
> +
> +    net_rx_pkt_init(&n->rss_data.pkt, false);
>  }
>  
>  static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
> @@ -3331,6 +3412,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
>      g_free(n->vqs);
>      qemu_del_nic(n->nic);
>      virtio_net_rsc_cleanup(n);
> +    net_rx_pkt_uninit(n->rss_data.pkt);
>      virtio_cleanup(vdev);
>  }
>  
> diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> index cf16f5192e..45670dd054 100644
> --- a/include/hw/virtio/virtio-net.h
> +++ b/include/hw/virtio/virtio-net.h
> @@ -209,6 +209,7 @@ struct VirtIONet {
>          uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
>          uint16_t indirections_len;
>          uint16_t default_queue;
> +        struct NetRxPkt *pkt;
>      } rss_data;
>  };
>  
> -- 
> 2.17.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated
  2020-03-10  3:12   ` Jason Wang
@ 2020-03-10  6:17     ` Michael S. Tsirkin
  2020-03-10 10:26       ` Yuri Benditovich
  0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2020-03-10  6:17 UTC (permalink / raw)
  To: Jason Wang; +Cc: yan, Yuri Benditovich, qemu-devel

On Tue, Mar 10, 2020 at 11:12:05AM +0800, Jason Wang wrote:
> 
> On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> > Block migration for reference implementation of
> > RSS feature in QEMU. When we add support for RSS
> > on backend side, we'll implement migration of
> > current RSS settings.
> > 
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >   hw/net/virtio-net.c            | 18 ++++++++++++++++++
> >   include/hw/virtio/virtio-net.h |  1 +
> >   2 files changed, 19 insertions(+)
> > 
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index abc41fdb16..943d1931a2 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -37,6 +37,7 @@
> >   #include "qapi/qapi-events-migration.h"
> >   #include "hw/virtio/virtio-access.h"
> >   #include "migration/misc.h"
> > +#include "migration/blocker.h"
> >   #include "standard-headers/linux/ethtool.h"
> >   #include "sysemu/sysemu.h"
> >   #include "trace.h"
> > @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev)
> >       n->announce_timer.round = 0;
> >       n->status &= ~VIRTIO_NET_S_ANNOUNCE;
> > +    if (n->migration_blocker) {
> > +        migrate_del_blocker(n->migration_blocker);
> > +        error_free(n->migration_blocker);
> > +        n->migration_blocker = NULL;
> > +    }
> > +
> >       /* Flush any MAC and VLAN filter table state */
> >       n->mac_table.in_use = 0;
> >       n->mac_table.first_multi = 0;
> > @@ -1003,6 +1010,17 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> >           vhost_net_ack_features(get_vhost_net(nc->peer), features);
> >       }
> > +    if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
> > +        if (!n->migration_blocker) {
> > +            error_setg(&n->migration_blocker, "virtio-net: RSS negotiated");
> > +            migrate_add_blocker(n->migration_blocker, &err);
> > +            if (err) {
> > +                error_report_err(err);
> > +                err = NULL;
> > +            }
> > +        }
> > +    }
> 
> 
> It looks to me we should add the blocker unconditionally based on
> virtio_host_has_feature() instead of depending the negotiated feature here.
> 
> Thanks

I agree, this is a stopgap measure anyway.  If this is merged in its
current form, there should also be a big TODO here that we must fix this
ASAP, and maybe a warning produced.


> 
> > +
> >       if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
> >           memset(n->vlans, 0, MAX_VLAN >> 3);
> >       } else {
> > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> > index 45670dd054..fba768ba9b 100644
> > --- a/include/hw/virtio/virtio-net.h
> > +++ b/include/hw/virtio/virtio-net.h
> > @@ -180,6 +180,7 @@ struct VirtIONet {
> >       virtio_net_conf net_conf;
> >       NICConf nic_conf;
> >       DeviceState *qdev;
> > +    Error *migration_blocker;
> >       int multiqueue;
> >       uint16_t max_queues;
> >       uint16_t curr_queues;



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/4] virtio-net: implement RX RSS processing
  2020-03-10  3:10   ` Jason Wang
@ 2020-03-10 10:18     ` Yuri Benditovich
  0 siblings, 0 replies; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-10 10:18 UTC (permalink / raw)
  To: Jason Wang; +Cc: Yan Vugenfirer, qemu-devel, Michael S . Tsirkin

On Tue, Mar 10, 2020 at 5:10 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> > If VIRTIO_NET_F_RSS negotiated and RSS is enabled, process
> > incoming packets, calculate packet's hash and place the
> > packet into respective RX virtqueue.
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >   hw/net/virtio-net.c            | 86 +++++++++++++++++++++++++++++++++-
> >   include/hw/virtio/virtio-net.h |  1 +
> >   2 files changed, 85 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 27071eccd2..abc41fdb16 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -42,6 +42,7 @@
> >   #include "trace.h"
> >   #include "monitor/qdev.h"
> >   #include "hw/pci/pci.h"
> > +#include "net_rx_pkt.h"
> >
> >   #define VIRTIO_NET_VM_VERSION    11
> >
> > @@ -1610,8 +1611,78 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size)
> >       return 0;
> >   }
> >
> > +static uint8_t virtio_net_get_hash_type(bool isip4,
> > +                                        bool isip6,
> > +                                        bool isudp,
> > +                                        bool istcp,
> > +                                        uint32_t types)
> > +{
> > +    uint32_t mask;
> > +    if (isip4) {
> > +        if (istcp && (types & VIRTIO_NET_RSS_HASH_TYPE_TCPv4)) {
> > +            return NetPktRssIpV4Tcp;
> > +        }
> > +        if (isudp && (types & VIRTIO_NET_RSS_HASH_TYPE_UDPv4)) {
> > +            return NetPktRssIpV4Udp;
> > +        }
> > +        if (types & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > +            return NetPktRssIpV4;
> > +        }
> > +    } else if (isip6) {
> > +        mask = VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> > +        if (istcp && (types & mask)) {
> > +            return (types & VIRTIO_NET_RSS_HASH_TYPE_TCP_EX) ?
> > +                NetPktRssIpV6TcpEx : NetPktRssIpV6Tcp;
> > +        }
> > +        mask = VIRTIO_NET_RSS_HASH_TYPE_UDP_EX | VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> > +        if (isudp && (types & mask)) {
> > +            return (types & VIRTIO_NET_RSS_HASH_TYPE_UDP_EX) ?
> > +                NetPktRssIpV6UdpEx : NetPktRssIpV6Udp;
> > +        }
> > +        mask = VIRTIO_NET_RSS_HASH_TYPE_IP_EX | VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > +        if (types & mask) {
> > +            return (types & VIRTIO_NET_RSS_HASH_TYPE_IP_EX) ?
> > +                NetPktRssIpV6Ex : NetPktRssIpV6;
> > +        }
> > +    }
> > +    return 0xff;
> > +}
> > +
> > +static int virtio_net_process_rss(NetClientState *nc, const uint8_t *buf,
> > +                                  size_t size)
> > +{
> > +    VirtIONet *n = qemu_get_nic_opaque(nc);
> > +    unsigned int index = nc->queue_index, new_index;
> > +    struct NetRxPkt *pkt = n->rss_data.pkt;
> > +    uint8_t net_hash_type;
> > +    uint32_t hash;
> > +    bool isip4, isip6, isudp, istcp;
> > +    net_rx_pkt_set_protocols(pkt, buf + n->host_hdr_len,
> > +                             size - n->host_hdr_len);
> > +    net_rx_pkt_get_protocols(pkt, &isip4, &isip6, &isudp, &istcp);
> > +    if (isip4 && (net_rx_pkt_get_ip4_info(pkt)->fragment)) {
> > +        istcp = isudp = false;
> > +    }
> > +    if (isip6 && (net_rx_pkt_get_ip6_info(pkt)->fragment)) {
> > +        istcp = isudp = false;
> > +    }
> > +    net_hash_type = virtio_net_get_hash_type(isip4, isip6, isudp, istcp,
> > +                                             n->rss_data.hash_types);
> > +    if (net_hash_type > NetPktRssIpV6UdpEx) {
> > +        return n->rss_data.default_queue;
> > +    }
> > +
> > +    hash = net_rx_pkt_calc_rss_hash(pkt, net_hash_type, n->rss_data.key);
> > +    new_index = hash & (n->rss_data.indirections_len - 1);
> > +    new_index = n->rss_data.indirections[new_index];
> > +    if (index == new_index) {
> > +        return -1;
> > +    }
> > +    return new_index;
> > +}
> > +
> >   static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > -                                      size_t size)
> > +                                      size_t size, bool no_rss)
> >   {
> >       VirtIONet *n = qemu_get_nic_opaque(nc);
> >       VirtIONetQueue *q = virtio_net_get_subqueue(nc);
> > @@ -1625,6 +1696,14 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> >           return -1;
> >       }
> >
> > +    if (!no_rss && n->rss_data.enabled) {
> > +        int index = virtio_net_process_rss(nc, buf, size);
> > +        if (index >= 0) {
> > +            NetClientState *nc2 = qemu_get_subqueue(n->nic, index);
> > +            return virtio_net_receive_rcu(nc2, buf, size, true);
> > +        }
> > +    }
>
>
> In the long run, we need to implement steering ops and allow device
> model to implement their own policy instead of doing hack like this.
>

Are you talking about support for RSS in tap driver or about something
different?

> Thanks
>
>
> > +
> >       /* hdr_len refers to the header we supply to the guest */
> >       if (!virtio_net_has_buffers(q, size + n->guest_hdr_len - n->host_hdr_len)) {
> >           return 0;
> > @@ -1719,7 +1798,7 @@ static ssize_t virtio_net_do_receive(NetClientState *nc, const uint8_t *buf,
> >   {
> >       RCU_READ_LOCK_GUARD();
> >
> > -    return virtio_net_receive_rcu(nc, buf, size);
> > +    return virtio_net_receive_rcu(nc, buf, size, false);
> >   }
> >
> >   static void virtio_net_rsc_extract_unit4(VirtioNetRscChain *chain,
> > @@ -3295,6 +3374,8 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
> >
> >       QTAILQ_INIT(&n->rsc_chains);
> >       n->qdev = dev;
> > +
> > +    net_rx_pkt_init(&n->rss_data.pkt, false);
> >   }
> >
> >   static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
> > @@ -3331,6 +3412,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
> >       g_free(n->vqs);
> >       qemu_del_nic(n->nic);
> >       virtio_net_rsc_cleanup(n);
> > +    net_rx_pkt_uninit(n->rss_data.pkt);
> >       virtio_cleanup(vdev);
> >   }
> >
> > diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
> > index cf16f5192e..45670dd054 100644
> > --- a/include/hw/virtio/virtio-net.h
> > +++ b/include/hw/virtio/virtio-net.h
> > @@ -209,6 +209,7 @@ struct VirtIONet {
> >           uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> >           uint16_t indirections_len;
> >           uint16_t default_queue;
> > +        struct NetRxPkt *pkt;
> >       } rss_data;
> >   };
> >
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated
  2020-03-10  6:17     ` Michael S. Tsirkin
@ 2020-03-10 10:26       ` Yuri Benditovich
  0 siblings, 0 replies; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-10 10:26 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Yan Vugenfirer, Jason Wang, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]

IMO, this does not depend on features of vhost as soon as we're not able to
provide parameters to it.


On Tue, Mar 10, 2020 at 8:17 AM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Tue, Mar 10, 2020 at 11:12:05AM +0800, Jason Wang wrote:
> >
> > On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> > > Block migration for reference implementation of
> > > RSS feature in QEMU. When we add support for RSS
> > > on backend side, we'll implement migration of
> > > current RSS settings.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > > ---
> > >   hw/net/virtio-net.c            | 18 ++++++++++++++++++
> > >   include/hw/virtio/virtio-net.h |  1 +
> > >   2 files changed, 19 insertions(+)
> > >
> > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > > index abc41fdb16..943d1931a2 100644
> > > --- a/hw/net/virtio-net.c
> > > +++ b/hw/net/virtio-net.c
> > > @@ -37,6 +37,7 @@
> > >   #include "qapi/qapi-events-migration.h"
> > >   #include "hw/virtio/virtio-access.h"
> > >   #include "migration/misc.h"
> > > +#include "migration/blocker.h"
> > >   #include "standard-headers/linux/ethtool.h"
> > >   #include "sysemu/sysemu.h"
> > >   #include "trace.h"
> > > @@ -627,6 +628,12 @@ static void virtio_net_reset(VirtIODevice *vdev)
> > >       n->announce_timer.round = 0;
> > >       n->status &= ~VIRTIO_NET_S_ANNOUNCE;
> > > +    if (n->migration_blocker) {
> > > +        migrate_del_blocker(n->migration_blocker);
> > > +        error_free(n->migration_blocker);
> > > +        n->migration_blocker = NULL;
> > > +    }
> > > +
> > >       /* Flush any MAC and VLAN filter table state */
> > >       n->mac_table.in_use = 0;
> > >       n->mac_table.first_multi = 0;
> > > @@ -1003,6 +1010,17 @@ static void
> virtio_net_set_features(VirtIODevice *vdev, uint64_t features)
> > >           vhost_net_ack_features(get_vhost_net(nc->peer), features);
> > >       }
> > > +    if (virtio_has_feature(features, VIRTIO_NET_F_RSS)) {
> > > +        if (!n->migration_blocker) {
> > > +            error_setg(&n->migration_blocker, "virtio-net: RSS
> negotiated");
> > > +            migrate_add_blocker(n->migration_blocker, &err);
> > > +            if (err) {
> > > +                error_report_err(err);
> > > +                err = NULL;
> > > +            }
> > > +        }
> > > +    }
> >
> >
> > It looks to me we should add the blocker unconditionally based on
> > virtio_host_has_feature() instead of depending the negotiated feature
> here.
> >
> > Thanks
>
> I agree, this is a stopgap measure anyway.  If this is merged in its
> current form, there should also be a big TODO here that we must fix this
> ASAP, and maybe a warning produced.
>
>
> >
> > > +
> > >       if (virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) {
> > >           memset(n->vlans, 0, MAX_VLAN >> 3);
> > >       } else {
> > > diff --git a/include/hw/virtio/virtio-net.h
> b/include/hw/virtio/virtio-net.h
> > > index 45670dd054..fba768ba9b 100644
> > > --- a/include/hw/virtio/virtio-net.h
> > > +++ b/include/hw/virtio/virtio-net.h
> > > @@ -180,6 +180,7 @@ struct VirtIONet {
> > >       virtio_net_conf net_conf;
> > >       NICConf nic_conf;
> > >       DeviceState *qdev;
> > > +    Error *migration_blocker;
> > >       int multiqueue;
> > >       uint16_t max_queues;
> > >       uint16_t curr_queues;
>
>

[-- Attachment #2: Type: text/html, Size: 4674 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/4] virtio-net: implement RSS configuration command
  2020-03-10  3:02   ` Jason Wang
@ 2020-03-10 10:29     ` Yuri Benditovich
  0 siblings, 0 replies; 14+ messages in thread
From: Yuri Benditovich @ 2020-03-10 10:29 UTC (permalink / raw)
  To: Jason Wang; +Cc: Yan Vugenfirer, qemu-devel, Michael S . Tsirkin

[-- Attachment #1: Type: text/plain, Size: 11648 bytes --]

On Tue, Mar 10, 2020 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote:

>
> On 2020/3/9 下午4:34, Yuri Benditovich wrote:
> > Optionally report RSS feature.
> > Handle RSS configuration command and keep RSS parameters
> > in virtio-net device context.
> >
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >   hw/net/trace-events            |   3 +
> >   hw/net/virtio-net.c            | 148 +++++++++++++++++++++++++++++++--
> >   include/hw/virtio/virtio-net.h |  11 +++
> >   3 files changed, 153 insertions(+), 9 deletions(-)
> >
> > diff --git a/hw/net/trace-events b/hw/net/trace-events
> > index a1da98a643..9823480d91 100644
> > --- a/hw/net/trace-events
> > +++ b/hw/net/trace-events
> > @@ -371,6 +371,9 @@ virtio_net_announce_notify(void) ""
> >   virtio_net_announce_timer(int round) "%d"
> >   virtio_net_handle_announce(int round) "%d"
> >   virtio_net_post_load_device(void)
> > +virtio_net_rss_disable(void)
> > +virtio_net_rss_error(int error_case) "case %d"
> > +virtio_net_rss_enable(uint32_t p1, uint16_t p2, uint8_t p3) "hashes
> 0x%x, table of %d, key of %d"
> >
> >   # tulip.c
> >   tulip_reg_write(uint64_t addr, const char *name, int size, uint64_t
> val) "addr 0x%02"PRIx64" (%s) size %d value 0x%08"PRIx64
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 9545b0e84f..27071eccd2 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -172,6 +172,16 @@ struct virtio_net_rss_config {
> >      tso/gso/gro 'off'. */
> >   #define VIRTIO_NET_RSC_DEFAULT_INTERVAL 300000
> >
> > +#define VIRTIO_NET_RSS_SUPPORTED_HASHES (VIRTIO_NET_RSS_HASH_TYPE_IPv4
> | \
> > +                                         VIRTIO_NET_RSS_HASH_TYPE_TCPv4
> | \
> > +                                         VIRTIO_NET_RSS_HASH_TYPE_UDPv4
> | \
> > +                                         VIRTIO_NET_RSS_HASH_TYPE_IPv6
> | \
> > +                                         VIRTIO_NET_RSS_HASH_TYPE_TCPv6
> | \
> > +                                         VIRTIO_NET_RSS_HASH_TYPE_UDPv6
> | \
> > +                                         VIRTIO_NET_RSS_HASH_TYPE_IP_EX
> | \
> > +
>  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX | \
> > +
>  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX)
> > +
> >   /* temporary until standard header include it */
> >   #if !defined(VIRTIO_NET_HDR_F_RSC_INFO)
> >
> > @@ -203,6 +213,8 @@ static VirtIOFeature feature_sizes[] = {
> >        .end = endof(struct virtio_net_config, mtu)},
> >       {.flags = 1ULL << VIRTIO_NET_F_SPEED_DUPLEX,
> >        .end = endof(struct virtio_net_config, duplex)},
> > +    {.flags = 1ULL << VIRTIO_NET_F_RSS,
> > +     .end = endof(struct virtio_net_config, supported_hash_types)},
> >       {}
> >   };
> >
> > @@ -233,6 +245,11 @@ static void virtio_net_get_config(VirtIODevice
> *vdev, uint8_t *config)
> >       memcpy(netcfg.mac, n->mac, ETH_ALEN);
> >       virtio_stl_p(vdev, &netcfg.speed, n->net_conf.speed);
> >       netcfg.duplex = n->net_conf.duplex;
> > +    netcfg.rss_max_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
> > +    virtio_stw_p(vdev, &netcfg.rss_max_indirection_table_length,
> > +                 VIRTIO_NET_RSS_MAX_TABLE_LEN);
> > +    virtio_stl_p(vdev, &netcfg.supported_hash_types,
> > +                 VIRTIO_NET_RSS_SUPPORTED_HASHES);
> >       memcpy(config, &netcfg, n->config_size);
> >   }
> >
> > @@ -796,6 +813,7 @@ static uint64_t virtio_net_get_features(VirtIODevice
> *vdev, uint64_t features,
> >           return features;
> >       }
> >
> > +    virtio_clear_feature(&features, VIRTIO_NET_F_RSS);
> >       features = vhost_net_get_features(get_vhost_net(nc->peer),
> features);
> >       vdev->backend_features = features;
> >
> > @@ -955,6 +973,7 @@ static void virtio_net_set_features(VirtIODevice
> *vdev, uint64_t features)
> >       }
> >
> >       virtio_net_set_multiqueue(n,
> > +                              virtio_has_feature(features,
> VIRTIO_NET_F_RSS) ||
> >                                 virtio_has_feature(features,
> VIRTIO_NET_F_MQ));
> >
> >       virtio_net_set_mrg_rx_bufs(n,
> > @@ -1231,25 +1250,134 @@ static int virtio_net_handle_announce(VirtIONet
> *n, uint8_t cmd,
> >       }
> >   }
> >
> > +static void virtio_net_disable_rss(VirtIONet *n)
> > +{
> > +    if (n->rss_data.enabled) {
> > +        trace_virtio_net_rss_disable();
> > +    }
> > +    n->rss_data.enabled = false;
> > +}
> > +
> > +static uint16_t virtio_net_handle_rss(VirtIONet *n,
> > +                                      struct iovec *iov, unsigned int
> iov_cnt)
> > +{
> > +    VirtIODevice *vdev = VIRTIO_DEVICE(n);
> > +    struct virtio_net_rss_config cfg;
> > +    size_t s, offset = 0, size_get;
> > +    uint16_t queues, i;
> > +    struct {
> > +        uint16_t us;
> > +        uint8_t b;
> > +    } QEMU_PACKED temp;
> > +    int err;
> > +
> > +    if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > +        err = 1;
> > +        goto error;
> > +    }
> > +    size_get = offsetof(struct virtio_net_rss_config,
> indirection_table);
> > +    s = iov_to_buf(iov, iov_cnt, offset, &cfg, size_get);
> > +    if (s != size_get) {
> > +        err = 2;
> > +        goto error;
> > +    }
> > +    n->rss_data.hash_types = virtio_ldl_p(vdev, &cfg.hash_types);
> > +    n->rss_data.indirections_len =
> > +        virtio_lduw_p(vdev, &cfg.indirection_table_mask);
> > +    n->rss_data.indirections_len++;
> > +    if (!is_power_of_2(n->rss_data.indirections_len)) {
> > +        err = 3;
> > +        goto error;
> > +    }
> > +    if (n->rss_data.indirections_len > VIRTIO_NET_RSS_MAX_TABLE_LEN) {
> > +        err = 4;
> > +        goto error;
> > +    }
> > +    n->rss_data.default_queue =
> > +        virtio_lduw_p(vdev, &cfg.unclassified_queue);
> > +    if (n->rss_data.default_queue >= n->max_queues) {
> > +        err = 5;
> > +        goto error;
> > +    }
> > +    offset += size_get;
> > +    size_get = sizeof(uint16_t) * n->rss_data.indirections_len;
> > +    s = iov_to_buf(iov, iov_cnt, offset, n->rss_data.indirections,
> size_get);
> > +    if (s != size_get) {
> > +        err = 10;
> > +        goto error;
> > +    }
> > +    for (i = 0; i < n->rss_data.indirections_len; ++i) {
> > +        uint16_t val = n->rss_data.indirections[i];
> > +        n->rss_data.indirections[i] = virtio_lduw_p(vdev, &val);
> > +    }
> > +    offset += size_get;
> > +    size_get = sizeof(temp);
> > +    s = iov_to_buf(iov, iov_cnt, offset, &temp, size_get);
> > +    if (s != size_get) {
> > +        err = 11;
> > +        goto error;
> > +    }
> > +    queues = virtio_lduw_p(vdev, &temp.us);
> > +    if (queues == 0 || queues > n->max_queues) {
> > +        err = 12;
> > +        goto error;
> > +    }
> > +    if (temp.b > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> > +        err = 13;
> > +        goto error;
> > +    }
> > +    if (!temp.b && n->rss_data.hash_types) {
> > +        err = 20;
> > +        goto error;
> > +    }
> > +    if (!temp.b && !n->rss_data.hash_types) {
> > +        virtio_net_disable_rss(n);
> > +        return queues;
> > +    }
> > +    offset += size_get;
> > +    size_get = temp.b;
> > +    s = iov_to_buf(iov, iov_cnt, offset, n->rss_data.key, size_get);
> > +    if (s != size_get) {
> > +        err = 21;
> > +        goto error;
> > +    }
> > +    n->rss_data.enabled = true;
> > +    trace_virtio_net_rss_enable(n->rss_data.hash_types,
> > +                                n->rss_data.indirections_len,
> > +                                temp.b);
> > +    return queues;
> > +error:
> > +    warn_report("%s: error_case %d", __func__, err);
>
>
> I'm not sure using warn_report() is good for such guest triggerable
> behavior.
>
>
> > +    trace_virtio_net_rss_error(err);
>
>
> It looks to me it would be better to be verbose here (show temp.b or other)
>
>
> > +    virtio_net_disable_rss(n);
> > +    return 0;
> > +}
> > +
> >   static int virtio_net_handle_mq(VirtIONet *n, uint8_t cmd,
> >                                   struct iovec *iov, unsigned int
> iov_cnt)
> >   {
> >       VirtIODevice *vdev = VIRTIO_DEVICE(n);
> > -    struct virtio_net_ctrl_mq mq;
> > -    size_t s;
> >       uint16_t queues;
> >
> > -    s = iov_to_buf(iov, iov_cnt, 0, &mq, sizeof(mq));
> > -    if (s != sizeof(mq)) {
> > -        return VIRTIO_NET_ERR;
> > -    }
> > +    virtio_net_disable_rss(n);
> > +    if (cmd == VIRTIO_NET_CTRL_MQ_RSS_CONFIG) {
> > +        queues = virtio_net_handle_rss(n, iov, iov_cnt);
> > +    } else if (cmd == VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
>
>
> It looks to me RSS and MQ are mutually exclusive, is this intentional?
>

No they are not. The device can support RSS or MQ or both.
The driver can activate multiqueue by VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET.
The driver can activate multiqueue by  VIRTIO_NET_CTRL_MQ_RSS_CONFIG  .



>
>
> > +        struct virtio_net_ctrl_mq mq;
> > +        size_t s;
> > +        if (!virtio_vdev_has_feature(vdev, VIRTIO_NET_F_MQ)) {
> > +            return VIRTIO_NET_ERR;
> > +        }
> > +        s = iov_to_buf(iov, iov_cnt, 0, &mq, sizeof(mq));
> > +        if (s != sizeof(mq)) {
> > +            return VIRTIO_NET_ERR;
> > +        }
> > +        queues = virtio_lduw_p(vdev, &mq.virtqueue_pairs);
> >
> > -    if (cmd != VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET) {
> > +    } else {
> >           return VIRTIO_NET_ERR;
> >       }
> >
> > -    queues = virtio_lduw_p(vdev, &mq.virtqueue_pairs);
> > -
> >       if (queues < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> >           queues > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
> >           queues > n->max_queues ||
> > @@ -3304,6 +3432,8 @@ static Property virtio_net_properties[] = {
> >       DEFINE_PROP_BIT64("ctrl_guest_offloads", VirtIONet, host_features,
> >                       VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true),
> >       DEFINE_PROP_BIT64("mq", VirtIONet, host_features, VIRTIO_NET_F_MQ,
> false),
> > +    DEFINE_PROP_BIT64("rss", VirtIONet, host_features,
> > +                    VIRTIO_NET_F_RSS, false),
> >       DEFINE_PROP_BIT64("guest_rsc_ext", VirtIONet, host_features,
> >                       VIRTIO_NET_F_RSC_EXT, false),
> >       DEFINE_PROP_UINT32("rsc_interval", VirtIONet, rsc_timeout,
> > diff --git a/include/hw/virtio/virtio-net.h
> b/include/hw/virtio/virtio-net.h
> > index 96c68d4a92..cf16f5192e 100644
> > --- a/include/hw/virtio/virtio-net.h
> > +++ b/include/hw/virtio/virtio-net.h
> > @@ -126,6 +126,9 @@ typedef struct VirtioNetRscChain {
> >   /* Maximum packet size we can receive from tap device: header + 64k */
> >   #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 *
> KiB))
> >
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> > +
> >   typedef struct VirtIONetQueue {
> >       VirtQueue *rx_vq;
> >       VirtQueue *tx_vq;
> > @@ -199,6 +202,14 @@ struct VirtIONet {
> >       bool failover;
> >       DeviceListener primary_listener;
> >       Notifier migration_state;
> > +    struct {
> > +        bool    enabled;
> > +        uint32_t hash_types;
> > +        uint8_t key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +        uint16_t indirections[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > +        uint16_t indirections_len;
> > +        uint16_t default_queue;
> > +    } rss_data;
> >   };
> >
> >   void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
>
>

[-- Attachment #2: Type: text/html, Size: 14975 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-03-10 10:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09  8:34 [PATCH v2 0/4] reference implementation of RSS Yuri Benditovich
2020-03-09  8:34 ` [PATCH v2 1/4] virtio-net: introduce RSS and hash report features Yuri Benditovich
2020-03-09  8:34 ` [PATCH v2 2/4] virtio-net: implement RSS configuration command Yuri Benditovich
2020-03-10  3:02   ` Jason Wang
2020-03-10 10:29     ` Yuri Benditovich
2020-03-09  8:34 ` [PATCH v2 3/4] virtio-net: implement RX RSS processing Yuri Benditovich
2020-03-10  3:10   ` Jason Wang
2020-03-10 10:18     ` Yuri Benditovich
2020-03-10  6:13   ` Michael S. Tsirkin
2020-03-09  8:34 ` [PATCH v2 4/4] virtio-net: block migration if RSS feature negotiated Yuri Benditovich
2020-03-10  3:12   ` Jason Wang
2020-03-10  6:17     ` Michael S. Tsirkin
2020-03-10 10:26       ` Yuri Benditovich
2020-03-09  8:59 ` [PATCH v2 0/4] reference implementation of RSS no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.