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 <address@hidden>
> ---
> 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,