[PATCH 0/3] reference implementation of RSS

[PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

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.

This series requires previously submitted and accepted
patch to be applied:
https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html

Yuri Benditovich (3):
  virtio-net: introduce RSS RX steering feature
  virtio-net: implement RSS configuration command
  virtio-net: implement RX RSS processing

 hw/net/trace-events                         |   3 +
 hw/net/virtio-net.c                         | 234 +++++++++++++++++++-VIRTIO_NET_F_RSS
 include/hw/virtio/virtio-net.h              |  12 +
 include/standard-headers/linux/virtio_net.h |  37 +++-
 4 files changed, 273 insertions(+), 13 deletions(-)

-- 
2.17.1

[PATCH 1/3] virtio-net: introduce RSS RX steering feature

[Yuri Benditovich]

Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
 include/standard-headers/linux/virtio_net.h | 37 +++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h
index 260c3681d7..3bc100afe3 100644
--- a/include/standard-headers/linux/virtio_net.h
+++ b/include/standard-headers/linux/virtio_net.h
@@ -57,6 +57,7 @@
 					 * Steering */
 #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
 
+#define VIRTIO_NET_F_RSS    	  60	/* Supports RSS RX steering */
 #define VIRTIO_NET_F_STANDBY	  62	/* Act as standby for another device
 					 * with the same MAC.
 					 */
@@ -69,6 +70,16 @@
 #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE	2	/* Announcement is needed */
 
+#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)
+
 struct virtio_net_config {
 	/* The config defining mac address (if VIRTIO_NET_F_MAC) */
 	uint8_t mac[ETH_ALEN];
@@ -92,6 +103,14 @@ struct virtio_net_config {
 	 * Any other value stands for unknown.
 	 */
 	uint8_t duplex;
+
+	/* maximal size of RSS key */
+	uint8_t rss_max_key_size;
+	/* maximal number of indirection table entries */
+	uint16_t rss_max_indirection_table_length;
+	/* bitmask of supported VIRTIO_NET_RSS_HASH_ types */
+	uint32_t supported_hash_types;
+
 } QEMU_PACKED;
 
 /*
@@ -237,15 +256,29 @@ struct virtio_net_ctrl_mac {
  * Accordingly, driver should not transmit new packets  on virtqueues other than
  * specified.
  */
+#define VIRTIO_NET_CTRL_MQ   4
+ #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
+ #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
+
+/* for VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET */
 struct virtio_net_ctrl_mq {
 	__virtio16 virtqueue_pairs;
 };
 
-#define VIRTIO_NET_CTRL_MQ   4
- #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
 
+/* for VIRTIO_NET_CTRL_MQ_RSS_CONFIG */
+struct virtio_net_rss_config {
+    uint32_t hash_types;
+    uint16_t indirection_table_mask;
+    uint16_t unclassified_queue;
+    uint16_t indirection_table[1/* + indirection_table_mask*/];
+    uint16_t max_tx_vq;
+    uint8_t hash_key_length;
+    uint8_t hash_key_data[/*hash_key_length*/];
+};
+
 /*
  * Control network offloads
  *
-- 
2.17.1

Re: [PATCH 1/3] virtio-net: introduce RSS RX steering feature

[Michael S. Tsirkin]

On Wed, Feb 26, 2020 at 07:48:07PM +0200, Yuri Benditovich wrote:
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> ---
>  include/standard-headers/linux/virtio_net.h | 37 +++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)


It will take a bit until next merge window when the linux
headers will be updated.
Until that happens you can just put these defines where
they are used.

> diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h
> index 260c3681d7..3bc100afe3 100644
> --- a/include/standard-headers/linux/virtio_net.h
> +++ b/include/standard-headers/linux/virtio_net.h
> @@ -57,6 +57,7 @@
>  					 * Steering */
>  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
>  
> +#define VIRTIO_NET_F_RSS    	  60	/* Supports RSS RX steering */
>  #define VIRTIO_NET_F_STANDBY	  62	/* Act as standby for another device
>  					 * with the same MAC.
>  					 */
> @@ -69,6 +70,16 @@
>  #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
>  #define VIRTIO_NET_S_ANNOUNCE	2	/* Announcement is needed */
>  
> +#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)
> +
>  struct virtio_net_config {
>  	/* The config defining mac address (if VIRTIO_NET_F_MAC) */
>  	uint8_t mac[ETH_ALEN];
> @@ -92,6 +103,14 @@ struct virtio_net_config {
>  	 * Any other value stands for unknown.
>  	 */
>  	uint8_t duplex;
> +
> +	/* maximal size of RSS key */
> +	uint8_t rss_max_key_size;
> +	/* maximal number of indirection table entries */
> +	uint16_t rss_max_indirection_table_length;
> +	/* bitmask of supported VIRTIO_NET_RSS_HASH_ types */
> +	uint32_t supported_hash_types;
> +
>  } QEMU_PACKED;
>  
>  /*
> @@ -237,15 +256,29 @@ struct virtio_net_ctrl_mac {
>   * Accordingly, driver should not transmit new packets  on virtqueues other than
>   * specified.
>   */
> +#define VIRTIO_NET_CTRL_MQ   4
> + #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
> + #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
> +
> +/* for VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET */
>  struct virtio_net_ctrl_mq {
>  	__virtio16 virtqueue_pairs;
>  };
>  
> -#define VIRTIO_NET_CTRL_MQ   4
> - #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
>  
> +/* for VIRTIO_NET_CTRL_MQ_RSS_CONFIG */
> +struct virtio_net_rss_config {
> +    uint32_t hash_types;
> +    uint16_t indirection_table_mask;
> +    uint16_t unclassified_queue;
> +    uint16_t indirection_table[1/* + indirection_table_mask*/];
> +    uint16_t max_tx_vq;
> +    uint8_t hash_key_length;
> +    uint8_t hash_key_data[/*hash_key_length*/];
> +};
> +
>  /*
>   * Control network offloads
>   *
> -- 
> 2.17.1

Re: [PATCH 1/3] virtio-net: introduce RSS RX steering feature

[Yuri Benditovich]

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

On Thu, Mar 5, 2020 at 3:21 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Wed, Feb 26, 2020 at 07:48:07PM +0200, Yuri Benditovich wrote:
> > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > ---
> >  include/standard-headers/linux/virtio_net.h | 37 +++++++++++++++++++--
> >  1 file changed, 35 insertions(+), 2 deletions(-)
>
>
> It will take a bit until next merge window when the linux
> headers will be updated.
> Until that happens you can just put these defines where
> they are used.
>

We also extend the configuration virtio_net_config structure and its
field 'supported_hash_types' uses these defines. Please advise.


>
> > diff --git a/include/standard-headers/linux/virtio_net.h
> b/include/standard-headers/linux/virtio_net.h
> > index 260c3681d7..3bc100afe3 100644
> > --- a/include/standard-headers/linux/virtio_net.h
> > +++ b/include/standard-headers/linux/virtio_net.h
> > @@ -57,6 +57,7 @@
> >                                        * Steering */
> >  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23        /* Set MAC address */
> >
> > +#define VIRTIO_NET_F_RSS       60    /* Supports RSS RX steering */
> >  #define VIRTIO_NET_F_STANDBY   62    /* Act as standby for another
> device
> >                                        * with the same MAC.
> >                                        */
> > @@ -69,6 +70,16 @@
> >  #define VIRTIO_NET_S_LINK_UP 1       /* Link is up */
> >  #define VIRTIO_NET_S_ANNOUNCE        2       /* Announcement is needed
> */
> >
> > +#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)
> > +
> >  struct virtio_net_config {
> >       /* The config defining mac address (if VIRTIO_NET_F_MAC) */
> >       uint8_t mac[ETH_ALEN];
> > @@ -92,6 +103,14 @@ struct virtio_net_config {
> >        * Any other value stands for unknown.
> >        */
> >       uint8_t duplex;
> > +
> > +     /* maximal size of RSS key */
> > +     uint8_t rss_max_key_size;
> > +     /* maximal number of indirection table entries */
> > +     uint16_t rss_max_indirection_table_length;
> > +     /* bitmask of supported VIRTIO_NET_RSS_HASH_ types */
> > +     uint32_t supported_hash_types;
> > +
> >  } QEMU_PACKED;
> >
> >  /*
> > @@ -237,15 +256,29 @@ struct virtio_net_ctrl_mac {
> >   * Accordingly, driver should not transmit new packets  on virtqueues
> other than
> >   * specified.
> >   */
> > +#define VIRTIO_NET_CTRL_MQ   4
> > + #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
> > + #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
> > +
> > +/* for VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET */
> >  struct virtio_net_ctrl_mq {
> >       __virtio16 virtqueue_pairs;
> >  };
> >
> > -#define VIRTIO_NET_CTRL_MQ   4
> > - #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
> >   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
> >   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
> >
> > +/* for VIRTIO_NET_CTRL_MQ_RSS_CONFIG */
> > +struct virtio_net_rss_config {
> > +    uint32_t hash_types;
> > +    uint16_t indirection_table_mask;
> > +    uint16_t unclassified_queue;
> > +    uint16_t indirection_table[1/* + indirection_table_mask*/];
> > +    uint16_t max_tx_vq;
> > +    uint8_t hash_key_length;
> > +    uint8_t hash_key_data[/*hash_key_length*/];
> > +};
> > +
> >  /*
> >   * Control network offloads
> >   *
> > --
> > 2.17.1
>
>

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

Re: [PATCH 1/3] virtio-net: introduce RSS RX steering feature

[Michael S. Tsirkin]

On Fri, Mar 06, 2020 at 11:29:50AM +0200, Yuri Benditovich wrote:
> 
> 
> On Thu, Mar 5, 2020 at 3:21 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Wed, Feb 26, 2020 at 07:48:07PM +0200, Yuri Benditovich wrote:
>     > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
>     > ---
>     >  include/standard-headers/linux/virtio_net.h | 37 +++++++++++++++++++--
>     >  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> 
>     It will take a bit until next merge window when the linux
>     headers will be updated.
>     Until that happens you can just put these defines where
>     they are used.
> 
> 
> We also extend the configuration virtio_net_config structure and its
> field 'supported_hash_types' uses these defines. Please advise.


Make a private copy of it with a different name for now.
Add a comment so we don't forget to remove down the road.

> 
> 
>     > diff --git a/include/standard-headers/linux/virtio_net.h b/include/
>     standard-headers/linux/virtio_net.h
>     > index 260c3681d7..3bc100afe3 100644
>     > --- a/include/standard-headers/linux/virtio_net.h
>     > +++ b/include/standard-headers/linux/virtio_net.h
>     > @@ -57,6 +57,7 @@
>     >                                        * Steering */
>     >  #define VIRTIO_NET_F_CTRL_MAC_ADDR 23        /* Set MAC address */
>     > 
>     > +#define VIRTIO_NET_F_RSS       60    /* Supports RSS RX steering */
>     >  #define VIRTIO_NET_F_STANDBY   62    /* Act as standby for another
>     device
>     >                                        * with the same MAC.
>     >                                        */
>     > @@ -69,6 +70,16 @@
>     >  #define VIRTIO_NET_S_LINK_UP 1       /* Link is up */
>     >  #define VIRTIO_NET_S_ANNOUNCE        2       /* Announcement is needed *
>     /
>     > 
>     > +#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)
>     > +
>     >  struct virtio_net_config {
>     >       /* The config defining mac address (if VIRTIO_NET_F_MAC) */
>     >       uint8_t mac[ETH_ALEN];
>     > @@ -92,6 +103,14 @@ struct virtio_net_config {
>     >        * Any other value stands for unknown.
>     >        */
>     >       uint8_t duplex;
>     > +
>     > +     /* maximal size of RSS key */
>     > +     uint8_t rss_max_key_size;
>     > +     /* maximal number of indirection table entries */
>     > +     uint16_t rss_max_indirection_table_length;
>     > +     /* bitmask of supported VIRTIO_NET_RSS_HASH_ types */
>     > +     uint32_t supported_hash_types;
>     > +
>     >  } QEMU_PACKED;
>     > 
>     >  /*
>     > @@ -237,15 +256,29 @@ struct virtio_net_ctrl_mac {
>     >   * Accordingly, driver should not transmit new packets  on virtqueues
>     other than
>     >   * specified.
>     >   */
>     > +#define VIRTIO_NET_CTRL_MQ   4
>     > + #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
>     > + #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1
>     > +
>     > +/* for VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET */
>     >  struct virtio_net_ctrl_mq {
>     >       __virtio16 virtqueue_pairs;
>     >  };
>     > 
>     > -#define VIRTIO_NET_CTRL_MQ   4
>     > - #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0
>     >   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN        1
>     >   #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX        0x8000
>     > 
>     > +/* for VIRTIO_NET_CTRL_MQ_RSS_CONFIG */
>     > +struct virtio_net_rss_config {
>     > +    uint32_t hash_types;
>     > +    uint16_t indirection_table_mask;
>     > +    uint16_t unclassified_queue;
>     > +    uint16_t indirection_table[1/* + indirection_table_mask*/];
>     > +    uint16_t max_tx_vq;
>     > +    uint8_t hash_key_length;
>     > +    uint8_t hash_key_data[/*hash_key_length*/];
>     > +};
>     > +
>     >  /*
>     >   * Control network offloads
>     >   *
>     > --
>     > 2.17.1
> 
> 

[PATCH 2/3] virtio-net: implement RSS configuration command

[Yuri Benditovich]

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 4939321493..0f47b7c450 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 d7d3ad6dc7..c5d21675a9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -77,6 +77,16 @@
    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)
 
@@ -108,6 +118,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)},
     {}
 };
 
@@ -138,6 +150,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);
 }
 
@@ -701,6 +718,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;
 
@@ -860,6 +878,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,
@@ -1136,25 +1155,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 ||
@@ -3209,6 +3337,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

[PATCH 3/3] virtio-net: implement RX RSS processing

[Yuri Benditovich]

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 c5d21675a9..adf7b88d7a 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
 
@@ -1515,8 +1516,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);
@@ -1530,6 +1601,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;
@@ -1624,7 +1703,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,
@@ -3200,6 +3279,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)
@@ -3236,6 +3317,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

Re: [PATCH 3/3] virtio-net: implement RX RSS processing

[Michael S. Tsirkin]

On Wed, Feb 26, 2020 at 07:48:09PM +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 c5d21675a9..adf7b88d7a 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
>  
> @@ -1515,8 +1516,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;


BTW we really need to fix up hw/net/net_rx_pkt.h to match qemu
coding style.
Could you do it pls?

> +        }
> +    }
> +    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);
> @@ -1530,6 +1601,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;
> @@ -1624,7 +1703,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,
> @@ -3200,6 +3279,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)
> @@ -3236,6 +3317,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

Re: [PATCH 3/3] virtio-net: implement RX RSS processing

[Yuri Benditovich]

On Thu, Mar 5, 2020 at 3:20 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Feb 26, 2020 at 07:48:09PM +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 c5d21675a9..adf7b88d7a 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
> >
> > @@ -1515,8 +1516,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;
>
>
> BTW we really need to fix up hw/net/net_rx_pkt.h to match qemu
> coding style.
> Could you do it pls?
>

Can you please point on exact style problem in net_rx_pkt.h?

> > +        }
> > +    }
> > +    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);
> > @@ -1530,6 +1601,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;
> > @@ -1624,7 +1703,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,
> > @@ -3200,6 +3279,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)
> > @@ -3236,6 +3317,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
>

Re: [PATCH 3/3] virtio-net: implement RX RSS processing

[Michael S. Tsirkin]

On Thu, Mar 05, 2020 at 09:54:31PM +0200, Yuri Benditovich wrote:
> On Thu, Mar 5, 2020 at 3:20 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Feb 26, 2020 at 07:48:09PM +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 c5d21675a9..adf7b88d7a 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
> > >
> > > @@ -1515,8 +1516,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;
> >
> >
> > BTW we really need to fix up hw/net/net_rx_pkt.h to match qemu
> > coding style.
> > Could you do it pls?
> >
> 
> Can you please point on exact style problem in net_rx_pkt.h?

Two issues that I noticed:

- Use of "struct" instead of a typedef with struct names.
- Mixed case for enum values instead of upper case.



> > > +        }
> > > +    }
> > > +    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);
> > > @@ -1530,6 +1601,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;
> > > @@ -1624,7 +1703,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,
> > > @@ -3200,6 +3279,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)
> > > @@ -3236,6 +3317,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
> >

Re: [PATCH 3/3] virtio-net: implement RX RSS processing

[Yuri Benditovich]

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

On Thu, Mar 5, 2020 at 10:02 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> On Thu, Mar 05, 2020 at 09:54:31PM +0200, Yuri Benditovich wrote:
> > On Thu, Mar 5, 2020 at 3:20 PM Michael S. Tsirkin <mst@redhat.com>
> wrote:
> > >
> > > On Wed, Feb 26, 2020 at 07:48:09PM +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 c5d21675a9..adf7b88d7a 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
> > > >
> > > > @@ -1515,8 +1516,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;
> > >
> > >
> > > BTW we really need to fix up hw/net/net_rx_pkt.h to match qemu
> > > coding style.
> > > Could you do it pls?
> > >
> >
> > Can you please point on exact style problem in net_rx_pkt.h?
>
> Two issues that I noticed:
>
> - Use of "struct" instead of a typedef with struct names.
> - Mixed case for enum values instead of upper case.
>
>
I will discuss it with the maintainer of net_rx_pkt.
Note that struct without typedef especially allowed by qemu coding style
doc.
Note also that the doc rather requires camel case for enums and both
uppercase and camel case are widely used in the code.


>
>
> > > > +        }
> > > > +    }
> > > > +    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);
> > > > @@ -1530,6 +1601,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;
> > > > @@ -1624,7 +1703,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,
> > > > @@ -3200,6 +3279,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)
> > > > @@ -3236,6 +3317,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
> > >
>
>

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

Re: [PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

ping

On Wed, Feb 26, 2020 at 7:48 PM Yuri Benditovich
<yuri.benditovich@daynix.com> wrote:
>
> 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.
>
> This series requires previously submitted and accepted
> patch to be applied:
> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
>
> Yuri Benditovich (3):
>   virtio-net: introduce RSS RX steering feature
>   virtio-net: implement RSS configuration command
>   virtio-net: implement RX RSS processing
>
>  hw/net/trace-events                         |   3 +
>  hw/net/virtio-net.c                         | 234 +++++++++++++++++++-VIRTIO_NET_F_RSS
>  include/hw/virtio/virtio-net.h              |  12 +
>  include/standard-headers/linux/virtio_net.h |  37 +++-
>  4 files changed, 273 insertions(+), 13 deletions(-)
>
> --
> 2.17.1
>

Re: [PATCH 0/3] reference implementation of RSS

[Jason Wang]

On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> 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.
>
> This series requires previously submitted and accepted
> patch to be applied:
> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
>
> Yuri Benditovich (3):
>    virtio-net: introduce RSS RX steering feature
>    virtio-net: implement RSS configuration command
>    virtio-net: implement RX RSS processing
>
>   hw/net/trace-events                         |   3 +
>   hw/net/virtio-net.c                         | 234 +++++++++++++++++++-VIRTIO_NET_F_RSS
>   include/hw/virtio/virtio-net.h              |  12 +
>   include/standard-headers/linux/virtio_net.h |  37 +++-
>   4 files changed, 273 insertions(+), 13 deletions(-)
>

One question before the reviewing.

Do we need to deal with migration (which I think yes)?

Thanks

Re: [PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

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

On Fri, Mar 6, 2020 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:

>
> On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > 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.
> >
> > This series requires previously submitted and accepted
> > patch to be applied:
> > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> >
> > Yuri Benditovich (3):
> >    virtio-net: introduce RSS RX steering feature
> >    virtio-net: implement RSS configuration command
> >    virtio-net: implement RX RSS processing
> >
> >   hw/net/trace-events                         |   3 +
> >   hw/net/virtio-net.c                         | 234
> +++++++++++++++++++-VIRTIO_NET_F_RSS
> >   include/hw/virtio/virtio-net.h              |  12 +
> >   include/standard-headers/linux/virtio_net.h |  37 +++-
> >   4 files changed, 273 insertions(+), 13 deletions(-)
> >
>
> One question before the reviewing.
>
> Do we need to deal with migration (which I think yes)?
>

I think we don't (yet) as this is a reference implementation and the main
goal is to provide the functional reference for hardware solution.
I agree with the general direction that for complete support of RSS and
hash delivery the best way is to do that in kernel using bpf.
For that, IMO, the bpf should be a part of the kernel (it uses skb fields)
and the tap should receive just RSS parameters for it.
At this stage we definitely will need to add migration support and
propagation of RSS parameters.


>
> Thanks
>
>

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

Re: [PATCH 0/3] reference implementation of RSS

[Michael S. Tsirkin]

On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> 
> 
> On Fri, Mar 6, 2020 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:
> 
> 
>     On 2020/2/27 上午1:48, Yuri Benditovich wrote:
>     > 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.
>     >
>     > This series requires previously submitted and accepted
>     > patch to be applied:
>     > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
>     >
>     > Yuri Benditovich (3):
>     >    virtio-net: introduce RSS RX steering feature
>     >    virtio-net: implement RSS configuration command
>     >    virtio-net: implement RX RSS processing
>     >
>     >   hw/net/trace-events                         |   3 +
>     >   hw/net/virtio-net.c                         | 234
>     +++++++++++++++++++-VIRTIO_NET_F_RSS
>     >   include/hw/virtio/virtio-net.h              |  12 +
>     >   include/standard-headers/linux/virtio_net.h |  37 +++-
>     >   4 files changed, 273 insertions(+), 13 deletions(-)
>     >
> 
>     One question before the reviewing.
> 
>     Do we need to deal with migration (which I think yes)?
> 
> 
> I think we don't (yet) as this is a reference implementation and the main goal
> is to provide the functional reference for hardware solution.


That's fine, but then we must block migration, and add appropriate code
comments. Just silently losing data isn't a good idea.

> I agree with the general direction that for complete support of RSS and hash
> delivery the best way is to do that in kernel using bpf.
> For that, IMO, the bpf should be a part of the kernel (it uses skb fields) and
> the tap should receive just RSS parameters for it.
> At this stage we definitely will need to add migration support and propagation
> of RSS parameters.
>  
> 
> 
>     Thanks
> 
> 

Re: [PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> >
> >
> > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> >     On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> >     > 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.
> >     >
> >     > This series requires previously submitted and accepted
> >     > patch to be applied:
> >     > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> >     >
> >     > Yuri Benditovich (3):
> >     >    virtio-net: introduce RSS RX steering feature
> >     >    virtio-net: implement RSS configuration command
> >     >    virtio-net: implement RX RSS processing
> >     >
> >     >   hw/net/trace-events                         |   3 +
> >     >   hw/net/virtio-net.c                         | 234
> >     +++++++++++++++++++-VIRTIO_NET_F_RSS
> >     >   include/hw/virtio/virtio-net.h              |  12 +
> >     >   include/standard-headers/linux/virtio_net.h |  37 +++-
> >     >   4 files changed, 273 insertions(+), 13 deletions(-)
> >     >
> >
> >     One question before the reviewing.
> >
> >     Do we need to deal with migration (which I think yes)?
> >
> >
> > I think we don't (yet) as this is a reference implementation and the main goal
> > is to provide the functional reference for hardware solution.
>
>
> That's fine, but then we must block migration, and add appropriate code
> comments. Just silently losing data isn't a good idea.

Note that there is no data lost, just the configuration of RSS is not migrating.
So, qemu just will not redirect the data to different queues after migration.
I would add the migration prevention in the next series with
implementation of hash delivery to prevent different packet sizes in
driver and qemu.

>
> > I agree with the general direction that for complete support of RSS and hash
> > delivery the best way is to do that in kernel using bpf.
> > For that, IMO, the bpf should be a part of the kernel (it uses skb fields) and
> > the tap should receive just RSS parameters for it.
> > At this stage we definitely will need to add migration support and propagation
> > of RSS parameters.
> >
> >
> >
> >     Thanks
> >
> >
>

Re: [PATCH 0/3] reference implementation of RSS

[Michael S. Tsirkin]

On Sun, Mar 08, 2020 at 11:56:38AM +0200, Yuri Benditovich wrote:
> On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> > >
> > >
> > > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > >
> > >     On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > >     > 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.
> > >     >
> > >     > This series requires previously submitted and accepted
> > >     > patch to be applied:
> > >     > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > >     >
> > >     > Yuri Benditovich (3):
> > >     >    virtio-net: introduce RSS RX steering feature
> > >     >    virtio-net: implement RSS configuration command
> > >     >    virtio-net: implement RX RSS processing
> > >     >
> > >     >   hw/net/trace-events                         |   3 +
> > >     >   hw/net/virtio-net.c                         | 234
> > >     +++++++++++++++++++-VIRTIO_NET_F_RSS
> > >     >   include/hw/virtio/virtio-net.h              |  12 +
> > >     >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > >     >   4 files changed, 273 insertions(+), 13 deletions(-)
> > >     >
> > >
> > >     One question before the reviewing.
> > >
> > >     Do we need to deal with migration (which I think yes)?
> > >
> > >
> > > I think we don't (yet) as this is a reference implementation and the main goal
> > > is to provide the functional reference for hardware solution.
> >
> >
> > That's fine, but then we must block migration, and add appropriate code
> > comments. Just silently losing data isn't a good idea.
> 
> Note that there is no data lost, just the configuration of RSS is not migrating.
> So, qemu just will not redirect the data to different queues after migration.

Right. Unlike auto-mq, the spec doesn't say the direction is best effort though,
so that would be a spec violation.

> I would add the migration prevention in the next series with
> implementation of hash delivery to prevent different packet sizes in
> driver and qemu.

And hopefully full migration support will follow.

One other thing to check is vhost - I didn't check
what happens with this patchset but
I think at a minimum we need to fail vhost init,
until the backend implements the feature biit.


> >
> > > I agree with the general direction that for complete support of RSS and hash
> > > delivery the best way is to do that in kernel using bpf.
> > > For that, IMO, the bpf should be a part of the kernel (it uses skb fields) and
> > > the tap should receive just RSS parameters for it.
> > > At this stage we definitely will need to add migration support and propagation
> > > of RSS parameters.
> > >
> > >
> > >
> > >     Thanks
> > >
> > >
> >

Re: [PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

On Sun, Mar 8, 2020 at 2:17 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Mar 08, 2020 at 11:56:38AM +0200, Yuri Benditovich wrote:
> > On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> > > >
> > > >
> > > > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > >
> > > >     On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > > >     > 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.
> > > >     >
> > > >     > This series requires previously submitted and accepted
> > > >     > patch to be applied:
> > > >     > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > > >     >
> > > >     > Yuri Benditovich (3):
> > > >     >    virtio-net: introduce RSS RX steering feature
> > > >     >    virtio-net: implement RSS configuration command
> > > >     >    virtio-net: implement RX RSS processing
> > > >     >
> > > >     >   hw/net/trace-events                         |   3 +
> > > >     >   hw/net/virtio-net.c                         | 234
> > > >     +++++++++++++++++++-VIRTIO_NET_F_RSS
> > > >     >   include/hw/virtio/virtio-net.h              |  12 +
> > > >     >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > > >     >   4 files changed, 273 insertions(+), 13 deletions(-)
> > > >     >
> > > >
> > > >     One question before the reviewing.
> > > >
> > > >     Do we need to deal with migration (which I think yes)?
> > > >
> > > >
> > > > I think we don't (yet) as this is a reference implementation and the main goal
> > > > is to provide the functional reference for hardware solution.
> > >
> > >
> > > That's fine, but then we must block migration, and add appropriate code
> > > comments. Just silently losing data isn't a good idea.
> >
> > Note that there is no data lost, just the configuration of RSS is not migrating.
> > So, qemu just will not redirect the data to different queues after migration.
>
> Right. Unlike auto-mq, the spec doesn't say the direction is best effort though,
> so that would be a spec violation.
>
> > I would add the migration prevention in the next series with
> > implementation of hash delivery to prevent different packet sizes in
> > driver and qemu.
>
> And hopefully full migration support will follow.
>
> One other thing to check is vhost - I didn't check
> what happens with this patchset but
> I think at a minimum we need to fail vhost init,
> until the backend implements the feature biit.

RSS feature currently is not indicated in case vhost is on.
The same will be with hash report.

>
>
> > >
> > > > I agree with the general direction that for complete support of RSS and hash
> > > > delivery the best way is to do that in kernel using bpf.
> > > > For that, IMO, the bpf should be a part of the kernel (it uses skb fields) and
> > > > the tap should receive just RSS parameters for it.
> > > > At this stage we definitely will need to add migration support and propagation
> > > > of RSS parameters.
> > > >
> > > >
> > > >
> > > >     Thanks
> > > >
> > > >
> > >
>

Re: [PATCH 0/3] reference implementation of RSS

[Michael S. Tsirkin]

On Sun, Mar 08, 2020 at 02:44:59PM +0200, Yuri Benditovich wrote:
> On Sun, Mar 8, 2020 at 2:17 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Sun, Mar 08, 2020 at 11:56:38AM +0200, Yuri Benditovich wrote:
> > > On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> > > > >
> > > > >
> > > > > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > >
> > > > >     On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > > > >     > 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.
> > > > >     >
> > > > >     > This series requires previously submitted and accepted
> > > > >     > patch to be applied:
> > > > >     > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > > > >     >
> > > > >     > Yuri Benditovich (3):
> > > > >     >    virtio-net: introduce RSS RX steering feature
> > > > >     >    virtio-net: implement RSS configuration command
> > > > >     >    virtio-net: implement RX RSS processing
> > > > >     >
> > > > >     >   hw/net/trace-events                         |   3 +
> > > > >     >   hw/net/virtio-net.c                         | 234
> > > > >     +++++++++++++++++++-VIRTIO_NET_F_RSS
> > > > >     >   include/hw/virtio/virtio-net.h              |  12 +
> > > > >     >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > > > >     >   4 files changed, 273 insertions(+), 13 deletions(-)
> > > > >     >
> > > > >
> > > > >     One question before the reviewing.
> > > > >
> > > > >     Do we need to deal with migration (which I think yes)?
> > > > >
> > > > >
> > > > > I think we don't (yet) as this is a reference implementation and the main goal
> > > > > is to provide the functional reference for hardware solution.
> > > >
> > > >
> > > > That's fine, but then we must block migration, and add appropriate code
> > > > comments. Just silently losing data isn't a good idea.
> > >
> > > Note that there is no data lost, just the configuration of RSS is not migrating.
> > > So, qemu just will not redirect the data to different queues after migration.
> >
> > Right. Unlike auto-mq, the spec doesn't say the direction is best effort though,
> > so that would be a spec violation.
> >
> > > I would add the migration prevention in the next series with
> > > implementation of hash delivery to prevent different packet sizes in
> > > driver and qemu.
> >
> > And hopefully full migration support will follow.
> >
> > One other thing to check is vhost - I didn't check
> > what happens with this patchset but
> > I think at a minimum we need to fail vhost init,
> > until the backend implements the feature biit.
> 
> RSS feature currently is not indicated in case vhost is on.
> The same will be with hash report.

IIRC with vhost features not listed are assumed to be
implemented by qemu and not to need backend support.

Maybe we should change that to make things more robust
in the future ... Jason, Marc-André am I right? what do you think?



> >
> >
> > > >
> > > > > I agree with the general direction that for complete support of RSS and hash
> > > > > delivery the best way is to do that in kernel using bpf.
> > > > > For that, IMO, the bpf should be a part of the kernel (it uses skb fields) and
> > > > > the tap should receive just RSS parameters for it.
> > > > > At this stage we definitely will need to add migration support and propagation
> > > > > of RSS parameters.
> > > > >
> > > > >
> > > > >
> > > > >     Thanks
> > > > >
> > > > >
> > > >
> >