From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 933B0A0547; Wed, 29 Sep 2021 11:02:31 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D6F76410EA; Wed, 29 Sep 2021 11:02:30 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id 772C440E3C for ; Wed, 29 Sep 2021 11:02:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632906148; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=k3MAJBC+s8A06V/0OSe3uxuC5YHbGg5bs3qiLHdu4YE=; b=XxSngWsswllgHtNiraemIvpzx7DGqNfj7afTw83zFECbYaN/3uagGl4RsSbLym2RjGBjRB vhjxCA1AF+cPrqEO1OqFGHxJnkBi3Ee4kcPLrCNCelVRz1gc4A1PCXfXADBzg9XrP5Ie9s xJ36Kz7DmUfV0BgPQ/bhGqw7w0c2Uqg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-319-tQNbXtalMui4QG4exkZnpQ-1; Wed, 29 Sep 2021 05:02:27 -0400 X-MC-Unique: tQNbXtalMui4QG4exkZnpQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 06D3A19057A1; Wed, 29 Sep 2021 09:02:26 +0000 (UTC) Received: from [10.39.208.26] (unknown [10.39.208.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7528460877; Wed, 29 Sep 2021 09:02:04 +0000 (UTC) Message-ID: <40e752a5-1435-794b-bd66-ccab8061b7e3@redhat.com> Date: Wed, 29 Sep 2021 11:02:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 To: "Xia, Chenbo" , "dev@dpdk.org" , "amorenoz@redhat.com" , "david.marchand@redhat.com" , "andrew.rybchenko@oktetlabs.ru" , "Yigit, Ferruh" , "michaelba@nvidia.com" , "viacheslavo@nvidia.com" , "Li, Xiaoyun" Cc: "stable@dpdk.org" , "nelio.laranjeiro@6wind.com" , "yvugenfi@redhat.com" , "ybendito@redhat.com" References: <20210922095742.229262-1-maxime.coquelin@redhat.com> <20210922095742.229262-2-maxime.coquelin@redhat.com> From: Maxime Coquelin In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=maxime.coquelin@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 1/5] net/virtio: add initial RSS support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 9/23/21 09:35, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin >> Sent: Wednesday, September 22, 2021 5:58 PM >> To: dev@dpdk.org; Xia, Chenbo ; amorenoz@redhat.com; >> david.marchand@redhat.com; andrew.rybchenko@oktetlabs.ru; Yigit, Ferruh >> ; michaelba@nvidia.com; viacheslavo@nvidia.com; Li, >> Xiaoyun >> Cc: stable@dpdk.org; nelio.laranjeiro@6wind.com; yvugenfi@redhat.com; >> ybendito@redhat.com; Maxime Coquelin >> Subject: [PATCH v2 1/5] net/virtio: add initial RSS support >> >> Provide the capability to update the hash key, hash types >> and RETA table on the fly (without needing to stop/start >> the device). However, the key length and the number of RETA >> entries are fixed to 40B and 128 entries respectively. This >> is done in order to simplify the design, but may be >> revisited later as the Virtio spec provides this >> flexibility. >> >> Note that only VIRTIO_NET_F_RSS support is implemented, >> VIRTIO_NET_F_HASH_REPORT, which would enable reporting the >> packet RSS hash calculated by the device into mbuf.rss, is >> not yet supported. >> >> Regarding the default RSS configuration, it has been >> chosen to use the default Intel ixgbe key as default key, >> and default RETA is a simple modulo between the hash and >> the number of Rx queues. >> >> Signed-off-by: Maxime Coquelin >> --- >> doc/guides/nics/features/virtio.ini | 3 + >> doc/guides/nics/virtio.rst | 3 + >> doc/guides/rel_notes/release_21_11.rst | 6 + >> drivers/net/virtio/virtio.h | 31 ++- >> drivers/net/virtio/virtio_ethdev.c | 367 ++++++++++++++++++++++++- >> drivers/net/virtio/virtio_ethdev.h | 3 +- >> drivers/net/virtio/virtqueue.h | 21 ++ >> 7 files changed, 428 insertions(+), 6 deletions(-) >> >> diff --git a/doc/guides/nics/features/virtio.ini >> b/doc/guides/nics/features/virtio.ini >> index 48f6f393b1..a5eab4932f 100644 >> --- a/doc/guides/nics/features/virtio.ini >> +++ b/doc/guides/nics/features/virtio.ini >> @@ -14,6 +14,9 @@ Promiscuous mode = Y >> Allmulticast mode = Y >> Unicast MAC filter = Y >> Multicast MAC filter = Y >> +RSS hash = P >> +RSS key update = Y >> +RSS reta update = Y >> VLAN filter = Y >> Basic stats = Y >> Stats per queue = Y >> diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst >> index 82ce7399ce..98e0d012b7 100644 >> --- a/doc/guides/nics/virtio.rst >> +++ b/doc/guides/nics/virtio.rst >> @@ -73,6 +73,9 @@ In this release, the virtio PMD driver provides the basic >> functionality of packe >> >> * Virtio supports using port IO to get PCI resource when UIO module is not >> available. >> >> +* Virtio supports RSS Rx mode with 40B configurable hash key length, 128 >> + configurable RETA entries and configurable hash types. >> + >> Prerequisites >> ------------- >> >> diff --git a/doc/guides/rel_notes/release_21_11.rst >> b/doc/guides/rel_notes/release_21_11.rst >> index f5d16993db..2f9d81926b 100644 >> --- a/doc/guides/rel_notes/release_21_11.rst >> +++ b/doc/guides/rel_notes/release_21_11.rst >> @@ -96,6 +96,12 @@ New Features >> Added command-line options to specify total number of processes and >> current process ID. Each process owns subset of Rx and Tx queues. >> >> +* **Added initial RSS support to Virtio PMD.** >> + >> + Initial support for RSS receive mode has been added to the Virtio PMD, >> + with the capability for the application to configure the hash key, the >> + RETA and the hash types. Virtio hash reporting is yet to be added. >> + >> >> Removed Items >> ------------- >> diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h >> index 525e2dad4c..b4f21dc0c7 100644 >> --- a/drivers/net/virtio/virtio.h >> +++ b/drivers/net/virtio/virtio.h >> @@ -30,6 +30,7 @@ >> #define VIRTIO_NET_F_GUEST_ANNOUNCE 21 /* Guest can announce device on the >> network */ >> #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow >> Steering */ >> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ >> +#define VIRTIO_NET_F_RSS 60 /* RSS supported */ >> >> /* >> * Do we get callbacks when the ring is completely used, >> @@ -100,6 +101,29 @@ >> */ >> #define VIRTIO_MAX_INDIRECT ((int)(rte_mem_page_size() / 16)) >> >> +/* Virtio RSS hash types */ >> +#define VIRTIO_NET_HASH_TYPE_IPV4 (1 << 0) >> +#define VIRTIO_NET_HASH_TYPE_TCPV4 (1 << 1) >> +#define VIRTIO_NET_HASH_TYPE_UDPV4 (1 << 2) >> +#define VIRTIO_NET_HASH_TYPE_IPV6 (1 << 3) >> +#define VIRTIO_NET_HASH_TYPE_TCPV6 (1 << 4) >> +#define VIRTIO_NET_HASH_TYPE_UDPV6 (1 << 5) > > Spec uses 'v' instead of 'V' for macro definition. > Better to align it? I actually put it in uper case on purpose. I don't have a strong opinion on this, but prefer to keep defines in upper case. The spec lacks consistency on this, as VIRTIO_NET_HDR_GSO_TCPV6 is all upper case for example. >> +#define VIRTIO_NET_HASH_TYPE_IP_EX (1 << 6) >> +#define VIRTIO_NET_HASH_TYPE_TCP_EX (1 << 7) >> +#define VIRTIO_NET_HASH_TYPE_UDP_EX (1 << 8) >> + >> +#define VIRTIO_NET_HASH_TYPE_MASK ( \ >> + VIRTIO_NET_HASH_TYPE_IPV4 | \ >> + VIRTIO_NET_HASH_TYPE_TCPV4 | \ >> + VIRTIO_NET_HASH_TYPE_UDPV4 | \ >> + VIRTIO_NET_HASH_TYPE_IPV6 | \ >> + VIRTIO_NET_HASH_TYPE_TCPV6 | \ >> + VIRTIO_NET_HASH_TYPE_UDPV6 | \ >> + VIRTIO_NET_HASH_TYPE_IP_EX | \ >> + VIRTIO_NET_HASH_TYPE_TCP_EX | \ >> + VIRTIO_NET_HASH_TYPE_UDP_EX) >> + >> + >> /* >> * Maximum number of virtqueues per device. >> */ >> @@ -157,7 +181,9 @@ struct virtio_net_config { >> * Any other value stands for unknown. >> */ >> uint8_t duplex; >> - >> + uint8_t rss_max_key_size; >> + uint16_t rss_max_indirection_table_length; >> + uint32_t supported_hash_types; >> } __rte_packed; >> >> struct virtio_hw { >> @@ -190,6 +216,9 @@ struct virtio_hw { >> rte_spinlock_t state_lock; >> struct rte_mbuf **inject_pkts; >> uint16_t max_queue_pairs; >> + uint32_t rss_hash_types; >> + uint16_t *rss_reta; >> + uint8_t *rss_key; >> uint64_t req_guest_features; >> struct virtnet_ctl *cvq; >> }; >> diff --git a/drivers/net/virtio/virtio_ethdev.c >> b/drivers/net/virtio/virtio_ethdev.c >> index da1633d77e..10a9f708eb 100644 >> --- a/drivers/net/virtio/virtio_ethdev.c >> +++ b/drivers/net/virtio/virtio_ethdev.c >> @@ -51,6 +51,16 @@ static int virtio_dev_info_get(struct rte_eth_dev *dev, >> static int virtio_dev_link_update(struct rte_eth_dev *dev, >> int wait_to_complete); >> static int virtio_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask); >> +static int virtio_dev_rss_hash_update(struct rte_eth_dev *dev, >> + struct rte_eth_rss_conf *rss_conf); >> +static int virtio_dev_rss_hash_conf_get(struct rte_eth_dev *dev, >> + struct rte_eth_rss_conf *rss_conf); >> +static int virtio_dev_rss_reta_update(struct rte_eth_dev *dev, >> + struct rte_eth_rss_reta_entry64 *reta_conf, >> + uint16_t reta_size); >> +static int virtio_dev_rss_reta_query(struct rte_eth_dev *dev, >> + struct rte_eth_rss_reta_entry64 *reta_conf, >> + uint16_t reta_size); >> >> static void virtio_set_hwaddr(struct virtio_hw *hw); >> static void virtio_get_hwaddr(struct virtio_hw *hw); >> @@ -347,7 +357,38 @@ virtio_send_command(struct virtnet_ctl *cvq, struct >> virtio_pmd_ctrl *ctrl, >> } >> >> static int >> -virtio_set_multiple_queues(struct rte_eth_dev *dev, uint16_t nb_queues) >> +virtio_set_multiple_queues_rss(struct rte_eth_dev *dev, uint16_t nb_queues) >> +{ >> + struct virtio_hw *hw = dev->data->dev_private; >> + struct virtio_pmd_ctrl ctrl; >> + struct virtio_net_ctrl_rss rss; >> + int dlen[1], ret; > > Do we really need 'int dlen[1]' rather than 'int dlen'? > No we don't, did that for consistency with virtio_set_multiple_queues_auto, but I'm fixing both occurences in new revision. >> +static int >> +virtio_dev_rss_hash_conf_get(struct rte_eth_dev *dev, >> + struct rte_eth_rss_conf *rss_conf) >> +{ >> + struct virtio_hw *hw = dev->data->dev_private; >> + >> + if (!virtio_with_feature(hw, VIRTIO_NET_F_RSS)) >> + return -ENOTSUP; >> + >> + if (!rss_conf) >> + return -EINVAL; > > No need to check, it's done in ethdev. OK, removing it here and elsewhere. ... >> +/** >> + * RSS control >> + * >> + * The RSS feature >> + */ >> +#define VIRTIO_NET_RSS_RETA_SIZE 128 >> +#define VIRTIO_NET_RSS_KEY_SIZE 40 > > Better align the numbers? Will do. Thanks! Maxime > Thanks, > Chenbo