All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] RSS support for VirtioNet.
@ 2022-01-09 21:06 ` Andrew Melnychenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Virtio-net supports "hardware" RSS with toeplitz key.
Also, it allows receiving calculated hash in vheader
that may be used with RPS.
Added ethtools callbacks to manipulate RSS.

Technically hash calculation may be set only for
SRC+DST and SRC+DST+PORTSRC+PORTDST hashflows.
The completely disabling hash calculation for TCP or UDP
would disable hash calculation for IP.

RSS/RXHASH is disabled by default.

Changes since rfc:
* code refactored
* patches reformatted
* added feature validation

Andrew Melnychenko (4):
  drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
  drivers/net/virtio_net: Added basic RSS support.
  drivers/net/virtio_net: Added RSS hash report.
  drivers/net/virtio_net: Added RSS hash report control.

 drivers/net/virtio_net.c | 404 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 390 insertions(+), 14 deletions(-)

-- 
2.34.1


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

* [PATCH 0/4] RSS support for VirtioNet.
@ 2022-01-09 21:06 ` Andrew Melnychenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Virtio-net supports "hardware" RSS with toeplitz key.
Also, it allows receiving calculated hash in vheader
that may be used with RPS.
Added ethtools callbacks to manipulate RSS.

Technically hash calculation may be set only for
SRC+DST and SRC+DST+PORTSRC+PORTDST hashflows.
The completely disabling hash calculation for TCP or UDP
would disable hash calculation for IP.

RSS/RXHASH is disabled by default.

Changes since rfc:
* code refactored
* patches reformatted
* added feature validation

Andrew Melnychenko (4):
  drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
  drivers/net/virtio_net: Added basic RSS support.
  drivers/net/virtio_net: Added RSS hash report.
  drivers/net/virtio_net: Added RSS hash report control.

 drivers/net/virtio_net.c | 404 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 390 insertions(+), 14 deletions(-)

-- 
2.34.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
  2022-01-09 21:06 ` Andrew Melnychenko
@ 2022-01-09 21:06   ` Andrew Melnychenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

The header v1 provides additional info about RSS.
Added changes to computing proper header length.
In the next patches, the header may contain RSS hash info
for the hash population.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b107835242ad..66439ca488f4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -242,13 +242,13 @@ struct virtnet_info {
 };
 
 struct padded_vnet_hdr {
-	struct virtio_net_hdr_mrg_rxbuf hdr;
+	struct virtio_net_hdr_v1_hash hdr;
 	/*
 	 * hdr is in a separate sg buffer, and data sg buffer shares same page
 	 * with this header sg. This padding makes next sg 16 byte aligned
 	 * after the header.
 	 */
-	char padding[4];
+	char padding[12];
 };
 
 static bool is_xdp_frame(void *ptr)
@@ -395,7 +395,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	hdr_p = p;
 
 	hdr_len = vi->hdr_len;
-	if (vi->mergeable_rx_bufs)
+	if (vi->has_rss_hash_report)
+		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
+	else if (vi->mergeable_rx_bufs)
 		hdr_padded_len = sizeof(*hdr);
 	else
 		hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1266,7 +1268,8 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
 					  struct ewma_pkt_len *avg_pkt_len,
 					  unsigned int room)
 {
-	const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	struct virtnet_info *vi = rq->vq->vdev->priv;
+	const size_t hdr_len = vi->hdr_len;
 	unsigned int len;
 
 	if (room)
@@ -2849,7 +2852,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
  */
 static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqueue *vq)
 {
-	const unsigned int hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	const unsigned int hdr_len = vi->hdr_len;
 	unsigned int rq_size = virtqueue_get_vring_size(vq);
 	unsigned int packet_len = vi->big_packets ? IP_MAX_MTU : vi->dev->max_mtu;
 	unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
-- 
2.34.1


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

* [PATCH 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
@ 2022-01-09 21:06   ` Andrew Melnychenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

The header v1 provides additional info about RSS.
Added changes to computing proper header length.
In the next patches, the header may contain RSS hash info
for the hash population.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b107835242ad..66439ca488f4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -242,13 +242,13 @@ struct virtnet_info {
 };
 
 struct padded_vnet_hdr {
-	struct virtio_net_hdr_mrg_rxbuf hdr;
+	struct virtio_net_hdr_v1_hash hdr;
 	/*
 	 * hdr is in a separate sg buffer, and data sg buffer shares same page
 	 * with this header sg. This padding makes next sg 16 byte aligned
 	 * after the header.
 	 */
-	char padding[4];
+	char padding[12];
 };
 
 static bool is_xdp_frame(void *ptr)
@@ -395,7 +395,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	hdr_p = p;
 
 	hdr_len = vi->hdr_len;
-	if (vi->mergeable_rx_bufs)
+	if (vi->has_rss_hash_report)
+		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
+	else if (vi->mergeable_rx_bufs)
 		hdr_padded_len = sizeof(*hdr);
 	else
 		hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1266,7 +1268,8 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
 					  struct ewma_pkt_len *avg_pkt_len,
 					  unsigned int room)
 {
-	const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	struct virtnet_info *vi = rq->vq->vdev->priv;
+	const size_t hdr_len = vi->hdr_len;
 	unsigned int len;
 
 	if (room)
@@ -2849,7 +2852,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
  */
 static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqueue *vq)
 {
-	const unsigned int hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
+	const unsigned int hdr_len = vi->hdr_len;
 	unsigned int rq_size = virtqueue_get_vring_size(vq);
 	unsigned int packet_len = vi->big_packets ? IP_MAX_MTU : vi->dev->max_mtu;
 	unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
-- 
2.34.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
  2022-01-09 21:06 ` Andrew Melnychenko
@ 2022-01-09 21:06   ` Andrew Melnychenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Added features for RSS.
Added initialization, RXHASH feature and ethtool ops.
By default RSS/RXHASH is disabled.
Virtio RSS "IPv6 extensions" hashes disabled.
Added ethtools ops to set key and indirection table.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 184 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 66439ca488f4..21794731fc75 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -169,6 +169,28 @@ struct receive_queue {
 	struct xdp_rxq_info xdp_rxq;
 };
 
+/* This structure can contain rss message with maximum settings for indirection table and keysize
+ * Note, that default structure that describes RSS configuration virtio_net_rss_config
+ * contains same info but can't handle table values.
+ * In any case, structure would be passed to virtio hw through sg_buf split by parts
+ * because table sizes may be differ according to the device configuration.
+ */
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
+struct virtio_net_ctrl_rss {
+	struct {
+		__le32 hash_types;
+		__le16 indirection_table_mask;
+		__le16 unclassified_queue;
+	} __packed table_info;
+	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+	struct {
+		u16 max_tx_vq; /* queues */
+		u8 hash_key_length;
+	} __packed key_info;
+	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+};
+
 /* Control VQ buffers: protected by the rtnl lock */
 struct control_buf {
 	struct virtio_net_ctrl_hdr hdr;
@@ -178,6 +200,7 @@ struct control_buf {
 	u8 allmulti;
 	__virtio16 vid;
 	__virtio64 offloads;
+	struct virtio_net_ctrl_rss rss;
 };
 
 struct virtnet_info {
@@ -206,6 +229,12 @@ struct virtnet_info {
 	/* Host will merge rx buffers for big packets (shake it! shake it!) */
 	bool mergeable_rx_bufs;
 
+	/* Host supports rss and/or hash report */
+	bool has_rss;
+	u8 rss_key_size;
+	u16 rss_indir_table_size;
+	u32 rss_hash_types_supported;
+
 	/* Has control virtqueue */
 	bool has_cvq;
 
@@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	hdr_p = p;
 
 	hdr_len = vi->hdr_len;
-	if (vi->has_rss_hash_report)
-		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
-	else if (vi->mergeable_rx_bufs)
+	if (vi->mergeable_rx_bufs)
 		hdr_padded_len = sizeof(*hdr);
 	else
 		hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
 	ring->tx_pending = ring->tx_max_pending;
 }
 
+static bool virtnet_commit_rss_command(struct virtnet_info *vi)
+{
+	struct net_device *dev = vi->dev;
+	struct scatterlist sgs[4];
+	unsigned int sg_buf_size;
+
+	/* prepare sgs */
+	sg_init_table(sgs, 4);
+
+	sg_buf_size = sizeof(vi->ctrl->rss.table_info);
+	sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
+
+	sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+	sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
+
+	sg_buf_size = sizeof(vi->ctrl->rss.key_info);
+	sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
+
+	sg_buf_size = vi->rss_key_size;
+	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
+
+	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
+				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
+		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
+		return false;
+	}
+	return true;
+}
+
+static void virtnet_init_default_rss(struct virtnet_info *vi)
+{
+	u32 indir_val = 0;
+	int i = 0;
+
+	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
+	vi->ctrl->rss.table_info.unclassified_queue = 0;
+
+	for (; i < vi->rss_indir_table_size; ++i) {
+		indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
+		vi->ctrl->rss.indirection_table[i] = indir_val;
+	}
+
+	vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
+	vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
+
+	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
+}
+
 
 static void virtnet_get_drvinfo(struct net_device *dev,
 				struct ethtool_drvinfo *info)
@@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
 		vi->duplex = duplex;
 }
 
+static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
+{
+	return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
+}
+
+static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
+{
+	return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
+}
+
+static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int i;
+
+	if (indir) {
+		for (i = 0; i < vi->rss_indir_table_size; ++i)
+			indir[i] = vi->ctrl->rss.indirection_table[i];
+	}
+
+	if (key)
+		memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
+
+	if (hfunc)
+		*hfunc = ETH_RSS_HASH_TOP;
+
+	return 0;
+}
+
+static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int i;
+
+	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
+		return -EOPNOTSUPP;
+
+	if (indir) {
+		for (i = 0; i < vi->rss_indir_table_size; ++i)
+			vi->ctrl->rss.indirection_table[i] = indir[i];
+	}
+	if (key)
+		memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
+
+	virtnet_commit_rss_command(vi);
+
+	return 0;
+}
+
+static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int rc = 0;
+
+	switch (info->cmd) {
+	case ETHTOOL_GRXRINGS:
+		info->data = vi->curr_queue_pairs;
+		break;
+	default:
+		rc = -EOPNOTSUPP;
+	}
+
+	return rc;
+}
+
 static const struct ethtool_ops virtnet_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
 	.get_drvinfo = virtnet_get_drvinfo,
@@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 	.set_link_ksettings = virtnet_set_link_ksettings,
 	.set_coalesce = virtnet_set_coalesce,
 	.get_coalesce = virtnet_get_coalesce,
+	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
+	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
+	.get_rxfh = virtnet_get_rxfh,
+	.set_rxfh = virtnet_set_rxfh,
+	.get_rxnfc = virtnet_get_rxnfc,
 };
 
 static void virtnet_freeze_down(struct virtio_device *vdev)
@@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
 			     "VIRTIO_NET_F_CTRL_VQ") ||
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
-			     "VIRTIO_NET_F_CTRL_VQ"))) {
+			     "VIRTIO_NET_F_CTRL_VQ") ||
+	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
 		return false;
 	}
 
@@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
 	u16 max_queue_pairs;
 	int mtu;
 
-	/* Find if host supports multiqueue virtio_net device */
-	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
-				   struct virtio_net_config,
-				   max_virtqueue_pairs, &max_queue_pairs);
+	/* Find if host supports multiqueue/rss virtio_net device */
+	max_queue_pairs = 0;
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
+		max_queue_pairs =
+		     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
 
 	/* We need at least 2 queue's */
-	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
+	if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
 	    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
 	    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
 		max_queue_pairs = 1;
@@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
+		vi->has_rss = true;
+		vi->rss_indir_table_size =
+			virtio_cread16(vdev, offsetof(struct virtio_net_config,
+						      rss_max_indirection_table_length));
+		vi->rss_key_size =
+			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
+	}
+
+	if (vi->has_rss) {
+		vi->rss_hash_types_supported =
+		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
+		vi->rss_hash_types_supported &=
+				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
+				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
+				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
+
+		dev->hw_features |= NETIF_F_RXHASH;
+	}
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
 	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
 		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
@@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 		}
 	}
 
+	if (vi->has_rss) {
+		rtnl_lock();
+		virtnet_init_default_rss(vi);
+		rtnl_unlock();
+	}
+
 	err = register_netdev(dev);
 	if (err) {
 		pr_debug("virtio_net: registering device failed\n");
@@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
 	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
 	VIRTIO_NET_F_CTRL_MAC_ADDR, \
 	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
-	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
+	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
+	VIRTIO_NET_F_RSS
 
 static unsigned int features[] = {
 	VIRTNET_FEATURES,
-- 
2.34.1


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

* [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-01-09 21:06   ` Andrew Melnychenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Added features for RSS.
Added initialization, RXHASH feature and ethtool ops.
By default RSS/RXHASH is disabled.
Virtio RSS "IPv6 extensions" hashes disabled.
Added ethtools ops to set key and indirection table.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 184 insertions(+), 10 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 66439ca488f4..21794731fc75 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -169,6 +169,28 @@ struct receive_queue {
 	struct xdp_rxq_info xdp_rxq;
 };
 
+/* This structure can contain rss message with maximum settings for indirection table and keysize
+ * Note, that default structure that describes RSS configuration virtio_net_rss_config
+ * contains same info but can't handle table values.
+ * In any case, structure would be passed to virtio hw through sg_buf split by parts
+ * because table sizes may be differ according to the device configuration.
+ */
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
+struct virtio_net_ctrl_rss {
+	struct {
+		__le32 hash_types;
+		__le16 indirection_table_mask;
+		__le16 unclassified_queue;
+	} __packed table_info;
+	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+	struct {
+		u16 max_tx_vq; /* queues */
+		u8 hash_key_length;
+	} __packed key_info;
+	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+};
+
 /* Control VQ buffers: protected by the rtnl lock */
 struct control_buf {
 	struct virtio_net_ctrl_hdr hdr;
@@ -178,6 +200,7 @@ struct control_buf {
 	u8 allmulti;
 	__virtio16 vid;
 	__virtio64 offloads;
+	struct virtio_net_ctrl_rss rss;
 };
 
 struct virtnet_info {
@@ -206,6 +229,12 @@ struct virtnet_info {
 	/* Host will merge rx buffers for big packets (shake it! shake it!) */
 	bool mergeable_rx_bufs;
 
+	/* Host supports rss and/or hash report */
+	bool has_rss;
+	u8 rss_key_size;
+	u16 rss_indir_table_size;
+	u32 rss_hash_types_supported;
+
 	/* Has control virtqueue */
 	bool has_cvq;
 
@@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	hdr_p = p;
 
 	hdr_len = vi->hdr_len;
-	if (vi->has_rss_hash_report)
-		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
-	else if (vi->mergeable_rx_bufs)
+	if (vi->mergeable_rx_bufs)
 		hdr_padded_len = sizeof(*hdr);
 	else
 		hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
 	ring->tx_pending = ring->tx_max_pending;
 }
 
+static bool virtnet_commit_rss_command(struct virtnet_info *vi)
+{
+	struct net_device *dev = vi->dev;
+	struct scatterlist sgs[4];
+	unsigned int sg_buf_size;
+
+	/* prepare sgs */
+	sg_init_table(sgs, 4);
+
+	sg_buf_size = sizeof(vi->ctrl->rss.table_info);
+	sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
+
+	sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+	sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
+
+	sg_buf_size = sizeof(vi->ctrl->rss.key_info);
+	sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
+
+	sg_buf_size = vi->rss_key_size;
+	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
+
+	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
+				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
+		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
+		return false;
+	}
+	return true;
+}
+
+static void virtnet_init_default_rss(struct virtnet_info *vi)
+{
+	u32 indir_val = 0;
+	int i = 0;
+
+	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
+	vi->ctrl->rss.table_info.unclassified_queue = 0;
+
+	for (; i < vi->rss_indir_table_size; ++i) {
+		indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
+		vi->ctrl->rss.indirection_table[i] = indir_val;
+	}
+
+	vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
+	vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
+
+	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
+}
+
 
 static void virtnet_get_drvinfo(struct net_device *dev,
 				struct ethtool_drvinfo *info)
@@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
 		vi->duplex = duplex;
 }
 
+static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
+{
+	return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
+}
+
+static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
+{
+	return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
+}
+
+static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int i;
+
+	if (indir) {
+		for (i = 0; i < vi->rss_indir_table_size; ++i)
+			indir[i] = vi->ctrl->rss.indirection_table[i];
+	}
+
+	if (key)
+		memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
+
+	if (hfunc)
+		*hfunc = ETH_RSS_HASH_TOP;
+
+	return 0;
+}
+
+static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int i;
+
+	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
+		return -EOPNOTSUPP;
+
+	if (indir) {
+		for (i = 0; i < vi->rss_indir_table_size; ++i)
+			vi->ctrl->rss.indirection_table[i] = indir[i];
+	}
+	if (key)
+		memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
+
+	virtnet_commit_rss_command(vi);
+
+	return 0;
+}
+
+static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int rc = 0;
+
+	switch (info->cmd) {
+	case ETHTOOL_GRXRINGS:
+		info->data = vi->curr_queue_pairs;
+		break;
+	default:
+		rc = -EOPNOTSUPP;
+	}
+
+	return rc;
+}
+
 static const struct ethtool_ops virtnet_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
 	.get_drvinfo = virtnet_get_drvinfo,
@@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 	.set_link_ksettings = virtnet_set_link_ksettings,
 	.set_coalesce = virtnet_set_coalesce,
 	.get_coalesce = virtnet_get_coalesce,
+	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
+	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
+	.get_rxfh = virtnet_get_rxfh,
+	.set_rxfh = virtnet_set_rxfh,
+	.get_rxnfc = virtnet_get_rxnfc,
 };
 
 static void virtnet_freeze_down(struct virtio_device *vdev)
@@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
 			     "VIRTIO_NET_F_CTRL_VQ") ||
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
-			     "VIRTIO_NET_F_CTRL_VQ"))) {
+			     "VIRTIO_NET_F_CTRL_VQ") ||
+	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
 		return false;
 	}
 
@@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
 	u16 max_queue_pairs;
 	int mtu;
 
-	/* Find if host supports multiqueue virtio_net device */
-	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
-				   struct virtio_net_config,
-				   max_virtqueue_pairs, &max_queue_pairs);
+	/* Find if host supports multiqueue/rss virtio_net device */
+	max_queue_pairs = 0;
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
+		max_queue_pairs =
+		     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
 
 	/* We need at least 2 queue's */
-	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
+	if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
 	    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
 	    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
 		max_queue_pairs = 1;
@@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
+		vi->has_rss = true;
+		vi->rss_indir_table_size =
+			virtio_cread16(vdev, offsetof(struct virtio_net_config,
+						      rss_max_indirection_table_length));
+		vi->rss_key_size =
+			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
+	}
+
+	if (vi->has_rss) {
+		vi->rss_hash_types_supported =
+		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
+		vi->rss_hash_types_supported &=
+				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
+				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
+				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
+
+		dev->hw_features |= NETIF_F_RXHASH;
+	}
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
 	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
 		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
@@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 		}
 	}
 
+	if (vi->has_rss) {
+		rtnl_lock();
+		virtnet_init_default_rss(vi);
+		rtnl_unlock();
+	}
+
 	err = register_netdev(dev);
 	if (err) {
 		pr_debug("virtio_net: registering device failed\n");
@@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
 	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
 	VIRTIO_NET_F_CTRL_MAC_ADDR, \
 	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
-	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
+	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
+	VIRTIO_NET_F_RSS
 
 static unsigned int features[] = {
 	VIRTNET_FEATURES,
-- 
2.34.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
  2022-01-09 21:06 ` Andrew Melnychenko
@ 2022-01-09 21:06   ` Andrew Melnychenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Added features for RSS hash report.
If hash is provided - it sets to skb.
Added checks if rss and/or hash are enabled together.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 21794731fc75..6e7461b01f87 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -231,6 +231,7 @@ struct virtnet_info {
 
 	/* Host supports rss and/or hash report */
 	bool has_rss;
+	bool has_rss_hash_report;
 	u8 rss_key_size;
 	u16 rss_indir_table_size;
 	u32 rss_hash_types_supported;
@@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	hdr_p = p;
 
 	hdr_len = vi->hdr_len;
-	if (vi->mergeable_rx_bufs)
+	if (vi->has_rss_hash_report)
+		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
+	else if (vi->mergeable_rx_bufs)
 		hdr_padded_len = sizeof(*hdr);
 	else
 		hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 	struct net_device *dev = vi->dev;
 	struct sk_buff *skb;
 	struct virtio_net_hdr_mrg_rxbuf *hdr;
+	struct virtio_net_hdr_v1_hash *hdr_hash;
+	enum pkt_hash_types rss_hash_type;
 
 	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
 		pr_debug("%s: short packet %i\n", dev->name, len);
@@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 		return;
 
 	hdr = skb_vnet_hdr(skb);
+	if (dev->features & NETIF_F_RXHASH) {
+		hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
+
+		switch (hdr_hash->hash_report) {
+		case VIRTIO_NET_HASH_REPORT_TCPv4:
+		case VIRTIO_NET_HASH_REPORT_UDPv4:
+		case VIRTIO_NET_HASH_REPORT_TCPv6:
+		case VIRTIO_NET_HASH_REPORT_UDPv6:
+		case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
+		case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
+			rss_hash_type = PKT_HASH_TYPE_L4;
+			break;
+		case VIRTIO_NET_HASH_REPORT_IPv4:
+		case VIRTIO_NET_HASH_REPORT_IPv6:
+		case VIRTIO_NET_HASH_REPORT_IPv6_EX:
+			rss_hash_type = PKT_HASH_TYPE_L3;
+			break;
+		case VIRTIO_NET_HASH_REPORT_NONE:
+		default:
+			rss_hash_type = PKT_HASH_TYPE_NONE;
+		}
+		skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
+	}
 
 	if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
 	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
-				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
+				  vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
+				  : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
 		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
 		return false;
 	}
@@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
 			     "VIRTIO_NET_F_CTRL_VQ") ||
-	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
+	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
+	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
+			     "VIRTIO_NET_F_HASH_REPORT"))) {
 		return false;
 	}
 
@@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
+		vi->has_rss_hash_report = true;
+		vi->rss_indir_table_size = 1;
+		vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
+	}
+
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
 		vi->has_rss = true;
 		vi->rss_indir_table_size =
@@ -3364,7 +3401,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
 	}
 
-	if (vi->has_rss) {
+	if (vi->has_rss || vi->has_rss_hash_report) {
 		vi->rss_hash_types_supported =
 		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
 		vi->rss_hash_types_supported &=
@@ -3374,8 +3411,11 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 		dev->hw_features |= NETIF_F_RXHASH;
 	}
-	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
-	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+
+	if (vi->has_rss_hash_report)
+		vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
+	else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
+		 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
 		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	else
 		vi->hdr_len = sizeof(struct virtio_net_hdr);
@@ -3442,7 +3482,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 		}
 	}
 
-	if (vi->has_rss) {
+	if (vi->has_rss || vi->has_rss_hash_report) {
 		rtnl_lock();
 		virtnet_init_default_rss(vi);
 		rtnl_unlock();
@@ -3580,7 +3620,7 @@ static struct virtio_device_id id_table[] = {
 	VIRTIO_NET_F_CTRL_MAC_ADDR, \
 	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
 	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
-	VIRTIO_NET_F_RSS
+	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
 
 static unsigned int features[] = {
 	VIRTNET_FEATURES,
-- 
2.34.1


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

* [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
@ 2022-01-09 21:06   ` Andrew Melnychenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Added features for RSS hash report.
If hash is provided - it sets to skb.
Added checks if rss and/or hash are enabled together.

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 21794731fc75..6e7461b01f87 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -231,6 +231,7 @@ struct virtnet_info {
 
 	/* Host supports rss and/or hash report */
 	bool has_rss;
+	bool has_rss_hash_report;
 	u8 rss_key_size;
 	u16 rss_indir_table_size;
 	u32 rss_hash_types_supported;
@@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	hdr_p = p;
 
 	hdr_len = vi->hdr_len;
-	if (vi->mergeable_rx_bufs)
+	if (vi->has_rss_hash_report)
+		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
+	else if (vi->mergeable_rx_bufs)
 		hdr_padded_len = sizeof(*hdr);
 	else
 		hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 	struct net_device *dev = vi->dev;
 	struct sk_buff *skb;
 	struct virtio_net_hdr_mrg_rxbuf *hdr;
+	struct virtio_net_hdr_v1_hash *hdr_hash;
+	enum pkt_hash_types rss_hash_type;
 
 	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
 		pr_debug("%s: short packet %i\n", dev->name, len);
@@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
 		return;
 
 	hdr = skb_vnet_hdr(skb);
+	if (dev->features & NETIF_F_RXHASH) {
+		hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
+
+		switch (hdr_hash->hash_report) {
+		case VIRTIO_NET_HASH_REPORT_TCPv4:
+		case VIRTIO_NET_HASH_REPORT_UDPv4:
+		case VIRTIO_NET_HASH_REPORT_TCPv6:
+		case VIRTIO_NET_HASH_REPORT_UDPv6:
+		case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
+		case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
+			rss_hash_type = PKT_HASH_TYPE_L4;
+			break;
+		case VIRTIO_NET_HASH_REPORT_IPv4:
+		case VIRTIO_NET_HASH_REPORT_IPv6:
+		case VIRTIO_NET_HASH_REPORT_IPv6_EX:
+			rss_hash_type = PKT_HASH_TYPE_L3;
+			break;
+		case VIRTIO_NET_HASH_REPORT_NONE:
+		default:
+			rss_hash_type = PKT_HASH_TYPE_NONE;
+		}
+		skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
+	}
 
 	if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
 	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
 
 	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
-				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
+				  vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
+				  : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
 		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
 		return false;
 	}
@@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
 	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
 			     "VIRTIO_NET_F_CTRL_VQ") ||
-	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
+	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
+	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
+			     "VIRTIO_NET_F_HASH_REPORT"))) {
 		return false;
 	}
 
@@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
 		vi->mergeable_rx_bufs = true;
 
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
+		vi->has_rss_hash_report = true;
+		vi->rss_indir_table_size = 1;
+		vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
+	}
+
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
 		vi->has_rss = true;
 		vi->rss_indir_table_size =
@@ -3364,7 +3401,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
 	}
 
-	if (vi->has_rss) {
+	if (vi->has_rss || vi->has_rss_hash_report) {
 		vi->rss_hash_types_supported =
 		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
 		vi->rss_hash_types_supported &=
@@ -3374,8 +3411,11 @@ static int virtnet_probe(struct virtio_device *vdev)
 
 		dev->hw_features |= NETIF_F_RXHASH;
 	}
-	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
-	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+
+	if (vi->has_rss_hash_report)
+		vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
+	else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
+		 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
 		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
 	else
 		vi->hdr_len = sizeof(struct virtio_net_hdr);
@@ -3442,7 +3482,7 @@ static int virtnet_probe(struct virtio_device *vdev)
 		}
 	}
 
-	if (vi->has_rss) {
+	if (vi->has_rss || vi->has_rss_hash_report) {
 		rtnl_lock();
 		virtnet_init_default_rss(vi);
 		rtnl_unlock();
@@ -3580,7 +3620,7 @@ static struct virtio_device_id id_table[] = {
 	VIRTIO_NET_F_CTRL_MAC_ADDR, \
 	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
 	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
-	VIRTIO_NET_F_RSS
+	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
 
 static unsigned int features[] = {
 	VIRTNET_FEATURES,
-- 
2.34.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
  2022-01-09 21:06 ` Andrew Melnychenko
@ 2022-01-09 21:06   ` Andrew Melnychenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Now it's possible to control supported hashflows.
Also added hashflow set/get callbacks.
Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
TCP and UDP supports only:
ethtool -U eth0 rx-flow-hash tcp4 sd
    RXH_IP_SRC + RXH_IP_DST
ethtool -U eth0 rx-flow-hash tcp4 sdfn
    RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 159 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6e7461b01f87..1b8dd384483c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -235,6 +235,7 @@ struct virtnet_info {
 	u8 rss_key_size;
 	u16 rss_indir_table_size;
 	u32 rss_hash_types_supported;
+	u32 rss_hash_types_saved;
 
 	/* Has control virtqueue */
 	bool has_cvq;
@@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
 	int i = 0;
 
 	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+	vi->rss_hash_types_saved = vi->rss_hash_types_supported;
 	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
 	vi->ctrl->rss.table_info.unclassified_queue = 0;
 
@@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
 	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
 }
 
+static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+	info->data = 0;
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case TCP_V6_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case UDP_V4_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case UDP_V6_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case IPV4_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+
+		break;
+	case IPV6_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+
+		break;
+	default:
+		info->data = 0;
+		break;
+	}
+}
+
+static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+	u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
+	u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
+	u32 new_hashtypes = vi->rss_hash_types_saved;
+
+	if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
+	    (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
+		return false;
+	}
+
+	if (!is_iphash && is_porthash)
+		return false;
+
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+	case UDP_V4_FLOW:
+	case IPV4_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+		if (is_iphash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+
+		break;
+	case TCP_V6_FLOW:
+	case UDP_V6_FLOW:
+	case IPV6_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+		if (is_iphash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+
+		break;
+	default:
+		break;
+	}
+
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+
+		break;
+	case UDP_V4_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+
+		break;
+	case TCP_V6_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+
+		break;
+	case UDP_V6_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+
+		break;
+	default:
+		break;
+	}
+
+	if (new_hashtypes != vi->rss_hash_types_saved) {
+		vi->rss_hash_types_saved = new_hashtypes;
+		vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+		if (vi->dev->features & NETIF_F_RXHASH)
+			return virtnet_commit_rss_command(vi);
+	}
+
+	return true;
+}
 
 static void virtnet_get_drvinfo(struct net_device *dev,
 				struct ethtool_drvinfo *info)
@@ -2574,6 +2701,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
 	switch (info->cmd) {
 	case ETHTOOL_GRXRINGS:
 		info->data = vi->curr_queue_pairs;
+		break;
+	case ETHTOOL_GRXFH:
+		virtnet_get_hashflow(vi, info);
+		break;
+	default:
+		rc = -EOPNOTSUPP;
+	}
+
+	return rc;
+}
+
+static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int rc = 0;
+
+	switch (info->cmd) {
+	case ETHTOOL_SRXFH:
+		if (!virtnet_set_hashflow(vi, info))
+			rc = -EINVAL;
+
 		break;
 	default:
 		rc = -EOPNOTSUPP;
@@ -2602,6 +2750,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 	.get_rxfh = virtnet_get_rxfh,
 	.set_rxfh = virtnet_set_rxfh,
 	.get_rxnfc = virtnet_get_rxnfc,
+	.set_rxnfc = virtnet_set_rxnfc,
 };
 
 static void virtnet_freeze_down(struct virtio_device *vdev)
@@ -2854,6 +3003,16 @@ static int virtnet_set_features(struct net_device *dev,
 		vi->guest_offloads = offloads;
 	}
 
+	if ((dev->features ^ features) & NETIF_F_RXHASH) {
+		if (features & NETIF_F_RXHASH)
+			vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+		else
+			vi->ctrl->rss.table_info.hash_types = 0;
+
+		if (!virtnet_commit_rss_command(vi))
+			return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.34.1


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

* [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2022-01-09 21:06   ` Andrew Melnychenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnychenko @ 2022-01-09 21:06 UTC (permalink / raw)
  To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
  Cc: yan, yuri.benditovich

Now it's possible to control supported hashflows.
Also added hashflow set/get callbacks.
Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
TCP and UDP supports only:
ethtool -U eth0 rx-flow-hash tcp4 sd
    RXH_IP_SRC + RXH_IP_DST
ethtool -U eth0 rx-flow-hash tcp4 sdfn
    RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3

Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
 drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 159 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6e7461b01f87..1b8dd384483c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -235,6 +235,7 @@ struct virtnet_info {
 	u8 rss_key_size;
 	u16 rss_indir_table_size;
 	u32 rss_hash_types_supported;
+	u32 rss_hash_types_saved;
 
 	/* Has control virtqueue */
 	bool has_cvq;
@@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
 	int i = 0;
 
 	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+	vi->rss_hash_types_saved = vi->rss_hash_types_supported;
 	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
 	vi->ctrl->rss.table_info.unclassified_queue = 0;
 
@@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
 	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
 }
 
+static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+	info->data = 0;
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case TCP_V6_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case UDP_V4_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case UDP_V6_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST |
+						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
+		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+		}
+		break;
+	case IPV4_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+
+		break;
+	case IPV6_FLOW:
+		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+			info->data = RXH_IP_SRC | RXH_IP_DST;
+
+		break;
+	default:
+		info->data = 0;
+		break;
+	}
+}
+
+static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+	u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
+	u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
+	u32 new_hashtypes = vi->rss_hash_types_saved;
+
+	if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
+	    (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
+		return false;
+	}
+
+	if (!is_iphash && is_porthash)
+		return false;
+
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+	case UDP_V4_FLOW:
+	case IPV4_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+		if (is_iphash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+
+		break;
+	case TCP_V6_FLOW:
+	case UDP_V6_FLOW:
+	case IPV6_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+		if (is_iphash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+
+		break;
+	default:
+		break;
+	}
+
+	switch (info->flow_type) {
+	case TCP_V4_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+
+		break;
+	case UDP_V4_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+
+		break;
+	case TCP_V6_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+
+		break;
+	case UDP_V6_FLOW:
+		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+		if (is_porthash)
+			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+
+		break;
+	default:
+		break;
+	}
+
+	if (new_hashtypes != vi->rss_hash_types_saved) {
+		vi->rss_hash_types_saved = new_hashtypes;
+		vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+		if (vi->dev->features & NETIF_F_RXHASH)
+			return virtnet_commit_rss_command(vi);
+	}
+
+	return true;
+}
 
 static void virtnet_get_drvinfo(struct net_device *dev,
 				struct ethtool_drvinfo *info)
@@ -2574,6 +2701,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
 	switch (info->cmd) {
 	case ETHTOOL_GRXRINGS:
 		info->data = vi->curr_queue_pairs;
+		break;
+	case ETHTOOL_GRXFH:
+		virtnet_get_hashflow(vi, info);
+		break;
+	default:
+		rc = -EOPNOTSUPP;
+	}
+
+	return rc;
+}
+
+static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
+{
+	struct virtnet_info *vi = netdev_priv(dev);
+	int rc = 0;
+
+	switch (info->cmd) {
+	case ETHTOOL_SRXFH:
+		if (!virtnet_set_hashflow(vi, info))
+			rc = -EINVAL;
+
 		break;
 	default:
 		rc = -EOPNOTSUPP;
@@ -2602,6 +2750,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
 	.get_rxfh = virtnet_get_rxfh,
 	.set_rxfh = virtnet_set_rxfh,
 	.get_rxnfc = virtnet_get_rxnfc,
+	.set_rxnfc = virtnet_set_rxnfc,
 };
 
 static void virtnet_freeze_down(struct virtio_device *vdev)
@@ -2854,6 +3003,16 @@ static int virtnet_set_features(struct net_device *dev,
 		vi->guest_offloads = offloads;
 	}
 
+	if ((dev->features ^ features) & NETIF_F_RXHASH) {
+		if (features & NETIF_F_RXHASH)
+			vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+		else
+			vi->ctrl->rss.table_info.hash_types = 0;
+
+		if (!virtnet_commit_rss_command(vi))
+			return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.34.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
  2022-01-09 21:06   ` Andrew Melnychenko
  (?)
@ 2022-01-09 22:21   ` Jakub Kicinski
  -1 siblings, 0 replies; 25+ messages in thread
From: Jakub Kicinski @ 2022-01-09 22:21 UTC (permalink / raw)
  To: Andrew Melnychenko
  Cc: netdev, virtualization, linux-kernel, davem, jasowang, mst, yan,
	yuri.benditovich

On Sun,  9 Jan 2022 23:06:56 +0200 Andrew Melnychenko wrote:
> The header v1 provides additional info about RSS.
> Added changes to computing proper header length.
> In the next patches, the header may contain RSS hash info
> for the hash population.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>

You can't break build in between patches:

drivers/net/virtio_net.c: In function ‘page_to_skb’:
drivers/net/virtio_net.c:398:15: error: ‘struct virtnet_info’ has no member named ‘has_rss_hash_report’
  398 |         if (vi->has_rss_hash_report)
      |               ^~

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

* Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
  2022-01-09 21:06   ` Andrew Melnychenko
@ 2022-01-11  3:44     ` Jason Wang
  -1 siblings, 0 replies; 25+ messages in thread
From: Jason Wang @ 2022-01-11  3:44 UTC (permalink / raw)
  To: Andrew Melnychenko, netdev, virtualization, linux-kernel, davem,
	kuba, mst
  Cc: yan, yuri.benditovich


在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> Added features for RSS.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Virtio RSS "IPv6 extensions" hashes disabled.
> Added ethtools ops to set key and indirection table.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 184 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 66439ca488f4..21794731fc75 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,28 @@ struct receive_queue {
>   	struct xdp_rxq_info xdp_rxq;
>   };
>   
> +/* This structure can contain rss message with maximum settings for indirection table and keysize
> + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> +struct virtio_net_ctrl_rss {
> +	struct {
> +		__le32 hash_types;
> +		__le16 indirection_table_mask;
> +		__le16 unclassified_queue;
> +	} __packed table_info;
> +	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> +	struct {
> +		u16 max_tx_vq; /* queues */
> +		u8 hash_key_length;
> +	} __packed key_info;
> +	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};


We need to consider to tweak and use uAPI in the future, e.g split the 
above into four parts:

1) first embed structure
2) indirection table
3) second embed structure
4) key array

1) and 3) could be uAPI.


> +
>   /* Control VQ buffers: protected by the rtnl lock */
>   struct control_buf {
>   	struct virtio_net_ctrl_hdr hdr;
> @@ -178,6 +200,7 @@ struct control_buf {
>   	u8 allmulti;
>   	__virtio16 vid;
>   	__virtio64 offloads;
> +	struct virtio_net_ctrl_rss rss;
>   };
>   
>   struct virtnet_info {
> @@ -206,6 +229,12 @@ struct virtnet_info {
>   	/* Host will merge rx buffers for big packets (shake it! shake it!) */
>   	bool mergeable_rx_bufs;
>   
> +	/* Host supports rss and/or hash report */
> +	bool has_rss;
> +	u8 rss_key_size;
> +	u16 rss_indir_table_size;
> +	u32 rss_hash_types_supported;
> +
>   	/* Has control virtqueue */
>   	bool has_cvq;
>   
> @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>   	hdr_p = p;
>   
>   	hdr_len = vi->hdr_len;
> -	if (vi->has_rss_hash_report)
> -		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> -	else if (vi->mergeable_rx_bufs)
> +	if (vi->mergeable_rx_bufs)
>   		hdr_padded_len = sizeof(*hdr);


Is this correct if both mergeable_rx_bufs and hash_report are set?


>   	else
>   		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
>   	ring->tx_pending = ring->tx_max_pending;
>   }
>   
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> +	struct net_device *dev = vi->dev;
> +	struct scatterlist sgs[4];
> +	unsigned int sg_buf_size;
> +
> +	/* prepare sgs */
> +	sg_init_table(sgs, 4);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> +	sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> +
> +	sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> +	sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> +	sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> +
> +	sg_buf_size = vi->rss_key_size;
> +	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> +
> +	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> +				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> +		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> +	u32 indir_val = 0;
> +	int i = 0;
> +
> +	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> +	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> +	vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> +	for (; i < vi->rss_indir_table_size; ++i) {
> +		indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> +		vi->ctrl->rss.indirection_table[i] = indir_val;
> +	}
> +
> +	vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> +	vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> +
> +	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> +}
> +
>   
>   static void virtnet_get_drvinfo(struct net_device *dev,
>   				struct ethtool_drvinfo *info)
> @@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
>   		vi->duplex = duplex;
>   }
>   
> +static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
> +}
> +
> +static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
> +}
> +
> +static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			indir[i] = vi->ctrl->rss.indirection_table[i];
> +	}
> +
> +	if (key)
> +		memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
> +
> +	if (hfunc)
> +		*hfunc = ETH_RSS_HASH_TOP;
> +
> +	return 0;
> +}
> +
> +static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> +		return -EOPNOTSUPP;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			vi->ctrl->rss.indirection_table[i] = indir[i];
> +	}
> +	if (key)
> +		memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
> +
> +	virtnet_commit_rss_command(vi);
> +
> +	return 0;
> +}
> +
> +static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int rc = 0;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		info->data = vi->curr_queue_pairs;
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +	}
> +
> +	return rc;
> +}
> +
>   static const struct ethtool_ops virtnet_ethtool_ops = {
>   	.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
>   	.get_drvinfo = virtnet_get_drvinfo,
> @@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>   	.set_link_ksettings = virtnet_set_link_ksettings,
>   	.set_coalesce = virtnet_set_coalesce,
>   	.get_coalesce = virtnet_get_coalesce,
> +	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
> +	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
> +	.get_rxfh = virtnet_get_rxfh,
> +	.set_rxfh = virtnet_set_rxfh,
> +	.get_rxnfc = virtnet_get_rxnfc,
>   };
>   
>   static void virtnet_freeze_down(struct virtio_device *vdev)
> @@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>   			     "VIRTIO_NET_F_CTRL_VQ") ||
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> -			     "VIRTIO_NET_F_CTRL_VQ"))) {
> +			     "VIRTIO_NET_F_CTRL_VQ") ||
> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
>   		return false;
>   	}
>   
> @@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	u16 max_queue_pairs;
>   	int mtu;
>   
> -	/* Find if host supports multiqueue virtio_net device */
> -	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> -				   struct virtio_net_config,
> -				   max_virtqueue_pairs, &max_queue_pairs);
> +	/* Find if host supports multiqueue/rss virtio_net device */
> +	max_queue_pairs = 0;
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> +		max_queue_pairs =
> +		     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
>   


Can we simply do virtio_cread_feature(vdev, VIRTIO_NET_F_MQ | 
VIRTIO_NET_F_RSS, ...) ?


>   	/* We need at least 2 queue's */
> -	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> +	if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
>   	    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
>   	    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>   		max_queue_pairs = 1;
> @@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
>   		vi->mergeable_rx_bufs = true;
>   
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> +		vi->has_rss = true;
> +		vi->rss_indir_table_size =
> +			virtio_cread16(vdev, offsetof(struct virtio_net_config,
> +						      rss_max_indirection_table_length));
> +		vi->rss_key_size =
> +			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> +	}
> +
> +	if (vi->has_rss) {
> +		vi->rss_hash_types_supported =
> +		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> +		vi->rss_hash_types_supported &=
> +				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> +
> +		dev->hw_features |= NETIF_F_RXHASH;
> +	}
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
>   	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>   		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> @@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>   		}
>   	}
>   
> +	if (vi->has_rss) {
> +		rtnl_lock();


Is rtnl_lock() really needed here consider we haven't even register netdev?

Thanks


> +		virtnet_init_default_rss(vi);
> +		rtnl_unlock();
> +	}
> +
>   	err = register_netdev(dev);
>   	if (err) {
>   		pr_debug("virtio_net: registering device failed\n");
> @@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
>   	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>   	VIRTIO_NET_F_CTRL_MAC_ADDR, \
>   	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> -	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> +	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> +	VIRTIO_NET_F_RSS
>   
>   static unsigned int features[] = {
>   	VIRTNET_FEATURES,


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

* Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-01-11  3:44     ` Jason Wang
  0 siblings, 0 replies; 25+ messages in thread
From: Jason Wang @ 2022-01-11  3:44 UTC (permalink / raw)
  To: Andrew Melnychenko, netdev, virtualization, linux-kernel, davem,
	kuba, mst
  Cc: yan, yuri.benditovich


在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> Added features for RSS.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Virtio RSS "IPv6 extensions" hashes disabled.
> Added ethtools ops to set key and indirection table.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 184 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 66439ca488f4..21794731fc75 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,28 @@ struct receive_queue {
>   	struct xdp_rxq_info xdp_rxq;
>   };
>   
> +/* This structure can contain rss message with maximum settings for indirection table and keysize
> + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> +struct virtio_net_ctrl_rss {
> +	struct {
> +		__le32 hash_types;
> +		__le16 indirection_table_mask;
> +		__le16 unclassified_queue;
> +	} __packed table_info;
> +	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> +	struct {
> +		u16 max_tx_vq; /* queues */
> +		u8 hash_key_length;
> +	} __packed key_info;
> +	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};


We need to consider to tweak and use uAPI in the future, e.g split the 
above into four parts:

1) first embed structure
2) indirection table
3) second embed structure
4) key array

1) and 3) could be uAPI.


> +
>   /* Control VQ buffers: protected by the rtnl lock */
>   struct control_buf {
>   	struct virtio_net_ctrl_hdr hdr;
> @@ -178,6 +200,7 @@ struct control_buf {
>   	u8 allmulti;
>   	__virtio16 vid;
>   	__virtio64 offloads;
> +	struct virtio_net_ctrl_rss rss;
>   };
>   
>   struct virtnet_info {
> @@ -206,6 +229,12 @@ struct virtnet_info {
>   	/* Host will merge rx buffers for big packets (shake it! shake it!) */
>   	bool mergeable_rx_bufs;
>   
> +	/* Host supports rss and/or hash report */
> +	bool has_rss;
> +	u8 rss_key_size;
> +	u16 rss_indir_table_size;
> +	u32 rss_hash_types_supported;
> +
>   	/* Has control virtqueue */
>   	bool has_cvq;
>   
> @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>   	hdr_p = p;
>   
>   	hdr_len = vi->hdr_len;
> -	if (vi->has_rss_hash_report)
> -		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> -	else if (vi->mergeable_rx_bufs)
> +	if (vi->mergeable_rx_bufs)
>   		hdr_padded_len = sizeof(*hdr);


Is this correct if both mergeable_rx_bufs and hash_report are set?


>   	else
>   		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
>   	ring->tx_pending = ring->tx_max_pending;
>   }
>   
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> +	struct net_device *dev = vi->dev;
> +	struct scatterlist sgs[4];
> +	unsigned int sg_buf_size;
> +
> +	/* prepare sgs */
> +	sg_init_table(sgs, 4);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> +	sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> +
> +	sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> +	sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> +	sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> +
> +	sg_buf_size = vi->rss_key_size;
> +	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> +
> +	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> +				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> +		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> +	u32 indir_val = 0;
> +	int i = 0;
> +
> +	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> +	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> +	vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> +	for (; i < vi->rss_indir_table_size; ++i) {
> +		indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> +		vi->ctrl->rss.indirection_table[i] = indir_val;
> +	}
> +
> +	vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> +	vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> +
> +	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> +}
> +
>   
>   static void virtnet_get_drvinfo(struct net_device *dev,
>   				struct ethtool_drvinfo *info)
> @@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
>   		vi->duplex = duplex;
>   }
>   
> +static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
> +}
> +
> +static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
> +}
> +
> +static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			indir[i] = vi->ctrl->rss.indirection_table[i];
> +	}
> +
> +	if (key)
> +		memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
> +
> +	if (hfunc)
> +		*hfunc = ETH_RSS_HASH_TOP;
> +
> +	return 0;
> +}
> +
> +static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> +		return -EOPNOTSUPP;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			vi->ctrl->rss.indirection_table[i] = indir[i];
> +	}
> +	if (key)
> +		memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
> +
> +	virtnet_commit_rss_command(vi);
> +
> +	return 0;
> +}
> +
> +static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int rc = 0;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		info->data = vi->curr_queue_pairs;
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +	}
> +
> +	return rc;
> +}
> +
>   static const struct ethtool_ops virtnet_ethtool_ops = {
>   	.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
>   	.get_drvinfo = virtnet_get_drvinfo,
> @@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>   	.set_link_ksettings = virtnet_set_link_ksettings,
>   	.set_coalesce = virtnet_set_coalesce,
>   	.get_coalesce = virtnet_get_coalesce,
> +	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
> +	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
> +	.get_rxfh = virtnet_get_rxfh,
> +	.set_rxfh = virtnet_set_rxfh,
> +	.get_rxnfc = virtnet_get_rxnfc,
>   };
>   
>   static void virtnet_freeze_down(struct virtio_device *vdev)
> @@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>   			     "VIRTIO_NET_F_CTRL_VQ") ||
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> -			     "VIRTIO_NET_F_CTRL_VQ"))) {
> +			     "VIRTIO_NET_F_CTRL_VQ") ||
> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
>   		return false;
>   	}
>   
> @@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	u16 max_queue_pairs;
>   	int mtu;
>   
> -	/* Find if host supports multiqueue virtio_net device */
> -	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> -				   struct virtio_net_config,
> -				   max_virtqueue_pairs, &max_queue_pairs);
> +	/* Find if host supports multiqueue/rss virtio_net device */
> +	max_queue_pairs = 0;
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> +		max_queue_pairs =
> +		     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
>   


Can we simply do virtio_cread_feature(vdev, VIRTIO_NET_F_MQ | 
VIRTIO_NET_F_RSS, ...) ?


>   	/* We need at least 2 queue's */
> -	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> +	if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
>   	    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
>   	    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>   		max_queue_pairs = 1;
> @@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
>   		vi->mergeable_rx_bufs = true;
>   
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> +		vi->has_rss = true;
> +		vi->rss_indir_table_size =
> +			virtio_cread16(vdev, offsetof(struct virtio_net_config,
> +						      rss_max_indirection_table_length));
> +		vi->rss_key_size =
> +			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> +	}
> +
> +	if (vi->has_rss) {
> +		vi->rss_hash_types_supported =
> +		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> +		vi->rss_hash_types_supported &=
> +				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> +
> +		dev->hw_features |= NETIF_F_RXHASH;
> +	}
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
>   	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>   		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> @@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>   		}
>   	}
>   
> +	if (vi->has_rss) {
> +		rtnl_lock();


Is rtnl_lock() really needed here consider we haven't even register netdev?

Thanks


> +		virtnet_init_default_rss(vi);
> +		rtnl_unlock();
> +	}
> +
>   	err = register_netdev(dev);
>   	if (err) {
>   		pr_debug("virtio_net: registering device failed\n");
> @@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
>   	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>   	VIRTIO_NET_F_CTRL_MAC_ADDR, \
>   	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> -	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> +	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> +	VIRTIO_NET_F_RSS
>   
>   static unsigned int features[] = {
>   	VIRTNET_FEATURES,

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
  2022-01-09 21:06   ` Andrew Melnychenko
@ 2022-01-11  4:05     ` Jason Wang
  -1 siblings, 0 replies; 25+ messages in thread
From: Jason Wang @ 2022-01-11  4:05 UTC (permalink / raw)
  To: Andrew Melnychenko, netdev, virtualization, linux-kernel, davem,
	kuba, mst
  Cc: yan, yuri.benditovich


在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> Added features for RSS hash report.
> If hash is provided - it sets to skb.
> Added checks if rss and/or hash are enabled together.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++------
>   1 file changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 21794731fc75..6e7461b01f87 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -231,6 +231,7 @@ struct virtnet_info {
>   
>   	/* Host supports rss and/or hash report */
>   	bool has_rss;
> +	bool has_rss_hash_report;
>   	u8 rss_key_size;
>   	u16 rss_indir_table_size;
>   	u32 rss_hash_types_supported;
> @@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>   	hdr_p = p;
>   
>   	hdr_len = vi->hdr_len;
> -	if (vi->mergeable_rx_bufs)
> +	if (vi->has_rss_hash_report)
> +		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> +	else if (vi->mergeable_rx_bufs)
>   		hdr_padded_len = sizeof(*hdr);
>   	else
>   		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>   	struct net_device *dev = vi->dev;
>   	struct sk_buff *skb;
>   	struct virtio_net_hdr_mrg_rxbuf *hdr;
> +	struct virtio_net_hdr_v1_hash *hdr_hash;
> +	enum pkt_hash_types rss_hash_type;
>   
>   	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
>   		pr_debug("%s: short packet %i\n", dev->name, len);
> @@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>   		return;
>   
>   	hdr = skb_vnet_hdr(skb);
> +	if (dev->features & NETIF_F_RXHASH) {
> +		hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> +
> +		switch (hdr_hash->hash_report) {
> +		case VIRTIO_NET_HASH_REPORT_TCPv4:
> +		case VIRTIO_NET_HASH_REPORT_UDPv4:
> +		case VIRTIO_NET_HASH_REPORT_TCPv6:
> +		case VIRTIO_NET_HASH_REPORT_UDPv6:
> +		case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> +		case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> +			rss_hash_type = PKT_HASH_TYPE_L4;
> +			break;
> +		case VIRTIO_NET_HASH_REPORT_IPv4:
> +		case VIRTIO_NET_HASH_REPORT_IPv6:
> +		case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> +			rss_hash_type = PKT_HASH_TYPE_L3;
> +			break;
> +		case VIRTIO_NET_HASH_REPORT_NONE:
> +		default:
> +			rss_hash_type = PKT_HASH_TYPE_NONE;
> +		}
> +		skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
> +	}
>   
>   	if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
>   		skb->ip_summed = CHECKSUM_UNNECESSARY;
> @@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
>   	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
>   
>   	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> -				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> +				  vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> +				  : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
>   		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
>   		return false;
>   	}
> @@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
>   			     "VIRTIO_NET_F_CTRL_VQ") ||
> -	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||


I think we should make RSS depend on CTRL_VQ.


> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> +			     "VIRTIO_NET_F_HASH_REPORT"))) {


Need to depend on CTRL_VQ here.


>   		return false;
>   	}
>   
> @@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
>   		vi->mergeable_rx_bufs = true;
>   
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> +		vi->has_rss_hash_report = true;
> +		vi->rss_indir_table_size = 1;
> +		vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;


Any reason to initialize RSS feature here not the init_default_rss()?

Thanks


> +	}
> +
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
>   		vi->has_rss = true;
>   		vi->rss_indir_table_size =
> @@ -3364,7 +3401,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>   			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
>   	}
>   
> -	if (vi->has_rss) {
> +	if (vi->has_rss || vi->has_rss_hash_report) {
>   		vi->rss_hash_types_supported =
>   		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
>   		vi->rss_hash_types_supported &=
> @@ -3374,8 +3411,11 @@ static int virtnet_probe(struct virtio_device *vdev)
>   
>   		dev->hw_features |= NETIF_F_RXHASH;
>   	}
> -	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> -	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> +
> +	if (vi->has_rss_hash_report)
> +		vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
> +	else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> +		 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>   		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
>   	else
>   		vi->hdr_len = sizeof(struct virtio_net_hdr);
> @@ -3442,7 +3482,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>   		}
>   	}
>   
> -	if (vi->has_rss) {
> +	if (vi->has_rss || vi->has_rss_hash_report) {
>   		rtnl_lock();
>   		virtnet_init_default_rss(vi);
>   		rtnl_unlock();
> @@ -3580,7 +3620,7 @@ static struct virtio_device_id id_table[] = {
>   	VIRTIO_NET_F_CTRL_MAC_ADDR, \
>   	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>   	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> -	VIRTIO_NET_F_RSS
> +	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
>   
>   static unsigned int features[] = {
>   	VIRTNET_FEATURES,


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

* Re: [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
@ 2022-01-11  4:05     ` Jason Wang
  0 siblings, 0 replies; 25+ messages in thread
From: Jason Wang @ 2022-01-11  4:05 UTC (permalink / raw)
  To: Andrew Melnychenko, netdev, virtualization, linux-kernel, davem,
	kuba, mst
  Cc: yan, yuri.benditovich


在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> Added features for RSS hash report.
> If hash is provided - it sets to skb.
> Added checks if rss and/or hash are enabled together.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++------
>   1 file changed, 48 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 21794731fc75..6e7461b01f87 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -231,6 +231,7 @@ struct virtnet_info {
>   
>   	/* Host supports rss and/or hash report */
>   	bool has_rss;
> +	bool has_rss_hash_report;
>   	u8 rss_key_size;
>   	u16 rss_indir_table_size;
>   	u32 rss_hash_types_supported;
> @@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>   	hdr_p = p;
>   
>   	hdr_len = vi->hdr_len;
> -	if (vi->mergeable_rx_bufs)
> +	if (vi->has_rss_hash_report)
> +		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> +	else if (vi->mergeable_rx_bufs)
>   		hdr_padded_len = sizeof(*hdr);
>   	else
>   		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>   	struct net_device *dev = vi->dev;
>   	struct sk_buff *skb;
>   	struct virtio_net_hdr_mrg_rxbuf *hdr;
> +	struct virtio_net_hdr_v1_hash *hdr_hash;
> +	enum pkt_hash_types rss_hash_type;
>   
>   	if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
>   		pr_debug("%s: short packet %i\n", dev->name, len);
> @@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
>   		return;
>   
>   	hdr = skb_vnet_hdr(skb);
> +	if (dev->features & NETIF_F_RXHASH) {
> +		hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> +
> +		switch (hdr_hash->hash_report) {
> +		case VIRTIO_NET_HASH_REPORT_TCPv4:
> +		case VIRTIO_NET_HASH_REPORT_UDPv4:
> +		case VIRTIO_NET_HASH_REPORT_TCPv6:
> +		case VIRTIO_NET_HASH_REPORT_UDPv6:
> +		case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> +		case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> +			rss_hash_type = PKT_HASH_TYPE_L4;
> +			break;
> +		case VIRTIO_NET_HASH_REPORT_IPv4:
> +		case VIRTIO_NET_HASH_REPORT_IPv6:
> +		case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> +			rss_hash_type = PKT_HASH_TYPE_L3;
> +			break;
> +		case VIRTIO_NET_HASH_REPORT_NONE:
> +		default:
> +			rss_hash_type = PKT_HASH_TYPE_NONE;
> +		}
> +		skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
> +	}
>   
>   	if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
>   		skb->ip_summed = CHECKSUM_UNNECESSARY;
> @@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
>   	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
>   
>   	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> -				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> +				  vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> +				  : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
>   		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
>   		return false;
>   	}
> @@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
>   	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
>   			     "VIRTIO_NET_F_CTRL_VQ") ||
> -	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||


I think we should make RSS depend on CTRL_VQ.


> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> +			     "VIRTIO_NET_F_HASH_REPORT"))) {


Need to depend on CTRL_VQ here.


>   		return false;
>   	}
>   
> @@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
>   		vi->mergeable_rx_bufs = true;
>   
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> +		vi->has_rss_hash_report = true;
> +		vi->rss_indir_table_size = 1;
> +		vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;


Any reason to initialize RSS feature here not the init_default_rss()?

Thanks


> +	}
> +
>   	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
>   		vi->has_rss = true;
>   		vi->rss_indir_table_size =
> @@ -3364,7 +3401,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>   			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
>   	}
>   
> -	if (vi->has_rss) {
> +	if (vi->has_rss || vi->has_rss_hash_report) {
>   		vi->rss_hash_types_supported =
>   		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
>   		vi->rss_hash_types_supported &=
> @@ -3374,8 +3411,11 @@ static int virtnet_probe(struct virtio_device *vdev)
>   
>   		dev->hw_features |= NETIF_F_RXHASH;
>   	}
> -	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> -	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> +
> +	if (vi->has_rss_hash_report)
> +		vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
> +	else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> +		 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>   		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
>   	else
>   		vi->hdr_len = sizeof(struct virtio_net_hdr);
> @@ -3442,7 +3482,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>   		}
>   	}
>   
> -	if (vi->has_rss) {
> +	if (vi->has_rss || vi->has_rss_hash_report) {
>   		rtnl_lock();
>   		virtnet_init_default_rss(vi);
>   		rtnl_unlock();
> @@ -3580,7 +3620,7 @@ static struct virtio_device_id id_table[] = {
>   	VIRTIO_NET_F_CTRL_MAC_ADDR, \
>   	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
>   	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> -	VIRTIO_NET_F_RSS
> +	VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
>   
>   static unsigned int features[] = {
>   	VIRTNET_FEATURES,

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
  2022-01-09 21:06   ` Andrew Melnychenko
@ 2022-01-11  4:32     ` Jason Wang
  -1 siblings, 0 replies; 25+ messages in thread
From: Jason Wang @ 2022-01-11  4:32 UTC (permalink / raw)
  To: Andrew Melnychenko, netdev, virtualization, linux-kernel, davem,
	kuba, mst
  Cc: yan, yuri.benditovich


在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> Now it's possible to control supported hashflows.
> Also added hashflow set/get callbacks.
> Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
> TCP and UDP supports only:
> ethtool -U eth0 rx-flow-hash tcp4 sd
>      RXH_IP_SRC + RXH_IP_DST
> ethtool -U eth0 rx-flow-hash tcp4 sdfn
>      RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 159 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 6e7461b01f87..1b8dd384483c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -235,6 +235,7 @@ struct virtnet_info {
>   	u8 rss_key_size;
>   	u16 rss_indir_table_size;
>   	u32 rss_hash_types_supported;
> +	u32 rss_hash_types_saved;
>   
>   	/* Has control virtqueue */
>   	bool has_cvq;
> @@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
>   	int i = 0;
>   
>   	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> +	vi->rss_hash_types_saved = vi->rss_hash_types_supported;
>   	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
>   	vi->ctrl->rss.table_info.unclassified_queue = 0;
>   
> @@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
>   	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
>   }
>   
> +static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> +	info->data = 0;
> +	switch (info->flow_type) {
> +	case TCP_V4_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case TCP_V6_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case UDP_V4_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case UDP_V6_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case IPV4_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +
> +		break;
> +	case IPV6_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +
> +		break;
> +	default:
> +		info->data = 0;
> +		break;
> +	}
> +}
> +
> +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> +	u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
> +	u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
> +	u32 new_hashtypes = vi->rss_hash_types_saved;
> +
> +	if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
> +	    (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
> +		return false;
> +	}
> +
> +	if (!is_iphash && is_porthash)
> +		return false;


This seems not filter out all the combinations:

e.g RXH_VLAN with port hash?


> +
> +	switch (info->flow_type) {
> +	case TCP_V4_FLOW:
> +	case UDP_V4_FLOW:
> +	case IPV4_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> +		if (is_iphash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> +
> +		break;
> +	case TCP_V6_FLOW:
> +	case UDP_V6_FLOW:
> +	case IPV6_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +		if (is_iphash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	switch (info->flow_type) {
> +	case TCP_V4_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;


Any way to merge the two switch? The code is hard to be reviewed anyhow.


> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
> +
> +		break;
> +	case UDP_V4_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> +
> +		break;
> +	case TCP_V6_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> +
> +		break;
> +	case UDP_V6_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> +
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (new_hashtypes != vi->rss_hash_types_saved) {
> +		vi->rss_hash_types_saved = new_hashtypes;
> +		vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> +		if (vi->dev->features & NETIF_F_RXHASH)
> +			return virtnet_commit_rss_command(vi);
> +	}
> +
> +	return true;
> +}
>   
>   static void virtnet_get_drvinfo(struct net_device *dev,
>   				struct ethtool_drvinfo *info)
> @@ -2574,6 +2701,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
>   	switch (info->cmd) {
>   	case ETHTOOL_GRXRINGS:
>   		info->data = vi->curr_queue_pairs;
> +		break;
> +	case ETHTOOL_GRXFH:
> +		virtnet_get_hashflow(vi, info);
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +	}
> +
> +	return rc;
> +}
> +
> +static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int rc = 0;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_SRXFH:
> +		if (!virtnet_set_hashflow(vi, info))
> +			rc = -EINVAL;
> +
>   		break;
>   	default:
>   		rc = -EOPNOTSUPP;
> @@ -2602,6 +2750,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>   	.get_rxfh = virtnet_get_rxfh,
>   	.set_rxfh = virtnet_set_rxfh,
>   	.get_rxnfc = virtnet_get_rxnfc,
> +	.set_rxnfc = virtnet_set_rxnfc,
>   };
>   
>   static void virtnet_freeze_down(struct virtio_device *vdev)
> @@ -2854,6 +3003,16 @@ static int virtnet_set_features(struct net_device *dev,
>   		vi->guest_offloads = offloads;
>   	}
>   
> +	if ((dev->features ^ features) & NETIF_F_RXHASH) {
> +		if (features & NETIF_F_RXHASH)
> +			vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> +		else
> +			vi->ctrl->rss.table_info.hash_types = 0;


I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.

Thanks


> +
> +		if (!virtnet_commit_rss_command(vi))
> +			return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   


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

* Re: [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2022-01-11  4:32     ` Jason Wang
  0 siblings, 0 replies; 25+ messages in thread
From: Jason Wang @ 2022-01-11  4:32 UTC (permalink / raw)
  To: Andrew Melnychenko, netdev, virtualization, linux-kernel, davem,
	kuba, mst
  Cc: yan, yuri.benditovich


在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> Now it's possible to control supported hashflows.
> Also added hashflow set/get callbacks.
> Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
> TCP and UDP supports only:
> ethtool -U eth0 rx-flow-hash tcp4 sd
>      RXH_IP_SRC + RXH_IP_DST
> ethtool -U eth0 rx-flow-hash tcp4 sdfn
>      RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 159 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 6e7461b01f87..1b8dd384483c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -235,6 +235,7 @@ struct virtnet_info {
>   	u8 rss_key_size;
>   	u16 rss_indir_table_size;
>   	u32 rss_hash_types_supported;
> +	u32 rss_hash_types_saved;
>   
>   	/* Has control virtqueue */
>   	bool has_cvq;
> @@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
>   	int i = 0;
>   
>   	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> +	vi->rss_hash_types_saved = vi->rss_hash_types_supported;
>   	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
>   	vi->ctrl->rss.table_info.unclassified_queue = 0;
>   
> @@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
>   	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
>   }
>   
> +static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> +	info->data = 0;
> +	switch (info->flow_type) {
> +	case TCP_V4_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case TCP_V6_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case UDP_V4_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case UDP_V6_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST |
> +						 RXH_L4_B_0_1 | RXH_L4_B_2_3;
> +		} else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +		}
> +		break;
> +	case IPV4_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +
> +		break;
> +	case IPV6_FLOW:
> +		if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> +			info->data = RXH_IP_SRC | RXH_IP_DST;
> +
> +		break;
> +	default:
> +		info->data = 0;
> +		break;
> +	}
> +}
> +
> +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> +	u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
> +	u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
> +	u32 new_hashtypes = vi->rss_hash_types_saved;
> +
> +	if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
> +	    (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
> +		return false;
> +	}
> +
> +	if (!is_iphash && is_porthash)
> +		return false;


This seems not filter out all the combinations:

e.g RXH_VLAN with port hash?


> +
> +	switch (info->flow_type) {
> +	case TCP_V4_FLOW:
> +	case UDP_V4_FLOW:
> +	case IPV4_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> +		if (is_iphash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> +
> +		break;
> +	case TCP_V6_FLOW:
> +	case UDP_V6_FLOW:
> +	case IPV6_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +		if (is_iphash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> +
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	switch (info->flow_type) {
> +	case TCP_V4_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;


Any way to merge the two switch? The code is hard to be reviewed anyhow.


> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
> +
> +		break;
> +	case UDP_V4_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> +
> +		break;
> +	case TCP_V6_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> +
> +		break;
> +	case UDP_V6_FLOW:
> +		new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> +		if (is_porthash)
> +			new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> +
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	if (new_hashtypes != vi->rss_hash_types_saved) {
> +		vi->rss_hash_types_saved = new_hashtypes;
> +		vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> +		if (vi->dev->features & NETIF_F_RXHASH)
> +			return virtnet_commit_rss_command(vi);
> +	}
> +
> +	return true;
> +}
>   
>   static void virtnet_get_drvinfo(struct net_device *dev,
>   				struct ethtool_drvinfo *info)
> @@ -2574,6 +2701,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
>   	switch (info->cmd) {
>   	case ETHTOOL_GRXRINGS:
>   		info->data = vi->curr_queue_pairs;
> +		break;
> +	case ETHTOOL_GRXFH:
> +		virtnet_get_hashflow(vi, info);
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +	}
> +
> +	return rc;
> +}
> +
> +static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int rc = 0;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_SRXFH:
> +		if (!virtnet_set_hashflow(vi, info))
> +			rc = -EINVAL;
> +
>   		break;
>   	default:
>   		rc = -EOPNOTSUPP;
> @@ -2602,6 +2750,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>   	.get_rxfh = virtnet_get_rxfh,
>   	.set_rxfh = virtnet_set_rxfh,
>   	.get_rxnfc = virtnet_get_rxnfc,
> +	.set_rxnfc = virtnet_set_rxnfc,
>   };
>   
>   static void virtnet_freeze_down(struct virtio_device *vdev)
> @@ -2854,6 +3003,16 @@ static int virtnet_set_features(struct net_device *dev,
>   		vi->guest_offloads = offloads;
>   	}
>   
> +	if ((dev->features ^ features) & NETIF_F_RXHASH) {
> +		if (features & NETIF_F_RXHASH)
> +			vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> +		else
> +			vi->ctrl->rss.table_info.hash_types = 0;


I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.

Thanks


> +
> +		if (!virtnet_commit_rss_command(vi))
> +			return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
  2022-01-09 21:06   ` Andrew Melnychenko
@ 2022-01-11 12:00     ` Michael S. Tsirkin
  -1 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2022-01-11 12:00 UTC (permalink / raw)
  To: Andrew Melnychenko
  Cc: netdev, virtualization, linux-kernel, davem, kuba, jasowang, yan,
	yuri.benditovich

On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> Added features for RSS.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Virtio RSS "IPv6 extensions" hashes disabled.
> Added ethtools ops to set key and indirection table.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>  drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 184 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 66439ca488f4..21794731fc75 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,28 @@ struct receive_queue {
>  	struct xdp_rxq_info xdp_rxq;
>  };
>  
> +/* This structure can contain rss message with maximum settings for indirection table and keysize
> + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> +struct virtio_net_ctrl_rss {
> +	struct {
> +		__le32 hash_types;
> +		__le16 indirection_table_mask;
> +		__le16 unclassified_queue;
> +	} __packed table_info;
> +	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> +	struct {
> +		u16 max_tx_vq; /* queues */
> +		u8 hash_key_length;
> +	} __packed key_info;
> +	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};
> +

Generally best to avoid __packed.
I think it's not a bad idea to just follow the spec when
you lay out the structures. Makes it easier to follow
that it matches. Spec has just a single struct:

struct virtio_net_rss_config {
    le32 hash_types;
    le16 indirection_table_mask;
    le16 unclassified_queue;
    le16 indirection_table[indirection_table_length];
    le16 max_tx_vq;
    u8 hash_key_length;
    u8 hash_key_data[hash_key_length];
};

and with this layout you don't need __packed.



>  /* Control VQ buffers: protected by the rtnl lock */
>  struct control_buf {
>  	struct virtio_net_ctrl_hdr hdr;
> @@ -178,6 +200,7 @@ struct control_buf {
>  	u8 allmulti;
>  	__virtio16 vid;
>  	__virtio64 offloads;
> +	struct virtio_net_ctrl_rss rss;
>  };
>  
>  struct virtnet_info {
> @@ -206,6 +229,12 @@ struct virtnet_info {
>  	/* Host will merge rx buffers for big packets (shake it! shake it!) */
>  	bool mergeable_rx_bufs;
>  
> +	/* Host supports rss and/or hash report */
> +	bool has_rss;
> +	u8 rss_key_size;
> +	u16 rss_indir_table_size;
> +	u32 rss_hash_types_supported;
> +
>  	/* Has control virtqueue */
>  	bool has_cvq;
>  
> @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>  	hdr_p = p;
>  
>  	hdr_len = vi->hdr_len;
> -	if (vi->has_rss_hash_report)
> -		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> -	else if (vi->mergeable_rx_bufs)
> +	if (vi->mergeable_rx_bufs)
>  		hdr_padded_len = sizeof(*hdr);
>  	else
>  		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
>  	ring->tx_pending = ring->tx_max_pending;
>  }
>  
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> +	struct net_device *dev = vi->dev;
> +	struct scatterlist sgs[4];
> +	unsigned int sg_buf_size;
> +
> +	/* prepare sgs */
> +	sg_init_table(sgs, 4);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> +	sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> +
> +	sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> +	sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> +	sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> +
> +	sg_buf_size = vi->rss_key_size;
> +	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> +
> +	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> +				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> +		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> +	u32 indir_val = 0;
> +	int i = 0;
> +
> +	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> +	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> +	vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> +	for (; i < vi->rss_indir_table_size; ++i) {
> +		indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> +		vi->ctrl->rss.indirection_table[i] = indir_val;
> +	}
> +
> +	vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> +	vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> +
> +	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> +}
> +
>  
>  static void virtnet_get_drvinfo(struct net_device *dev,
>  				struct ethtool_drvinfo *info)
> @@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
>  		vi->duplex = duplex;
>  }
>  
> +static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
> +}
> +
> +static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
> +}
> +
> +static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			indir[i] = vi->ctrl->rss.indirection_table[i];
> +	}
> +
> +	if (key)
> +		memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
> +
> +	if (hfunc)
> +		*hfunc = ETH_RSS_HASH_TOP;
> +
> +	return 0;
> +}
> +
> +static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> +		return -EOPNOTSUPP;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			vi->ctrl->rss.indirection_table[i] = indir[i];
> +	}
> +	if (key)
> +		memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
> +
> +	virtnet_commit_rss_command(vi);
> +
> +	return 0;
> +}
> +
> +static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int rc = 0;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		info->data = vi->curr_queue_pairs;
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +	}
> +
> +	return rc;
> +}
> +
>  static const struct ethtool_ops virtnet_ethtool_ops = {
>  	.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
>  	.get_drvinfo = virtnet_get_drvinfo,
> @@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>  	.set_link_ksettings = virtnet_set_link_ksettings,
>  	.set_coalesce = virtnet_set_coalesce,
>  	.get_coalesce = virtnet_get_coalesce,
> +	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
> +	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
> +	.get_rxfh = virtnet_get_rxfh,
> +	.set_rxfh = virtnet_set_rxfh,
> +	.get_rxnfc = virtnet_get_rxnfc,
>  };
>  
>  static void virtnet_freeze_down(struct virtio_device *vdev)
> @@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>  			     "VIRTIO_NET_F_CTRL_VQ") ||
>  	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
>  	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> -			     "VIRTIO_NET_F_CTRL_VQ"))) {
> +			     "VIRTIO_NET_F_CTRL_VQ") ||
> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
>  		return false;
>  	}
>  
> @@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	u16 max_queue_pairs;
>  	int mtu;
>  
> -	/* Find if host supports multiqueue virtio_net device */
> -	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> -				   struct virtio_net_config,
> -				   max_virtqueue_pairs, &max_queue_pairs);
> +	/* Find if host supports multiqueue/rss virtio_net device */
> +	max_queue_pairs = 0;
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> +		max_queue_pairs =
> +		     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
>  
>  	/* We need at least 2 queue's */
> -	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> +	if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
>  	    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
>  	    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>  		max_queue_pairs = 1;
> @@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
>  		vi->mergeable_rx_bufs = true;
>  
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> +		vi->has_rss = true;
> +		vi->rss_indir_table_size =
> +			virtio_cread16(vdev, offsetof(struct virtio_net_config,
> +						      rss_max_indirection_table_length));
> +		vi->rss_key_size =
> +			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> +	}
> +
> +	if (vi->has_rss) {
> +		vi->rss_hash_types_supported =
> +		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> +		vi->rss_hash_types_supported &=
> +				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> +
> +		dev->hw_features |= NETIF_F_RXHASH;
> +	}
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
>  	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>  		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> @@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>  		}
>  	}
>  
> +	if (vi->has_rss) {
> +		rtnl_lock();
> +		virtnet_init_default_rss(vi);
> +		rtnl_unlock();
> +	}
> +
>  	err = register_netdev(dev);
>  	if (err) {
>  		pr_debug("virtio_net: registering device failed\n");
> @@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>  	VIRTIO_NET_F_CTRL_MAC_ADDR, \
>  	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> -	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> +	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> +	VIRTIO_NET_F_RSS
>  
>  static unsigned int features[] = {
>  	VIRTNET_FEATURES,
> -- 
> 2.34.1


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

* Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-01-11 12:00     ` Michael S. Tsirkin
  0 siblings, 0 replies; 25+ messages in thread
From: Michael S. Tsirkin @ 2022-01-11 12:00 UTC (permalink / raw)
  To: Andrew Melnychenko
  Cc: netdev, linux-kernel, virtualization, yuri.benditovich, yan, kuba, davem

On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> Added features for RSS.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Virtio RSS "IPv6 extensions" hashes disabled.
> Added ethtools ops to set key and indirection table.
> 
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>  drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 184 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 66439ca488f4..21794731fc75 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,28 @@ struct receive_queue {
>  	struct xdp_rxq_info xdp_rxq;
>  };
>  
> +/* This structure can contain rss message with maximum settings for indirection table and keysize
> + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> +struct virtio_net_ctrl_rss {
> +	struct {
> +		__le32 hash_types;
> +		__le16 indirection_table_mask;
> +		__le16 unclassified_queue;
> +	} __packed table_info;
> +	u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> +	struct {
> +		u16 max_tx_vq; /* queues */
> +		u8 hash_key_length;
> +	} __packed key_info;
> +	u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};
> +

Generally best to avoid __packed.
I think it's not a bad idea to just follow the spec when
you lay out the structures. Makes it easier to follow
that it matches. Spec has just a single struct:

struct virtio_net_rss_config {
    le32 hash_types;
    le16 indirection_table_mask;
    le16 unclassified_queue;
    le16 indirection_table[indirection_table_length];
    le16 max_tx_vq;
    u8 hash_key_length;
    u8 hash_key_data[hash_key_length];
};

and with this layout you don't need __packed.



>  /* Control VQ buffers: protected by the rtnl lock */
>  struct control_buf {
>  	struct virtio_net_ctrl_hdr hdr;
> @@ -178,6 +200,7 @@ struct control_buf {
>  	u8 allmulti;
>  	__virtio16 vid;
>  	__virtio64 offloads;
> +	struct virtio_net_ctrl_rss rss;
>  };
>  
>  struct virtnet_info {
> @@ -206,6 +229,12 @@ struct virtnet_info {
>  	/* Host will merge rx buffers for big packets (shake it! shake it!) */
>  	bool mergeable_rx_bufs;
>  
> +	/* Host supports rss and/or hash report */
> +	bool has_rss;
> +	u8 rss_key_size;
> +	u16 rss_indir_table_size;
> +	u32 rss_hash_types_supported;
> +
>  	/* Has control virtqueue */
>  	bool has_cvq;
>  
> @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>  	hdr_p = p;
>  
>  	hdr_len = vi->hdr_len;
> -	if (vi->has_rss_hash_report)
> -		hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> -	else if (vi->mergeable_rx_bufs)
> +	if (vi->mergeable_rx_bufs)
>  		hdr_padded_len = sizeof(*hdr);
>  	else
>  		hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
>  	ring->tx_pending = ring->tx_max_pending;
>  }
>  
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> +	struct net_device *dev = vi->dev;
> +	struct scatterlist sgs[4];
> +	unsigned int sg_buf_size;
> +
> +	/* prepare sgs */
> +	sg_init_table(sgs, 4);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> +	sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> +
> +	sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> +	sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> +	sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> +	sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> +
> +	sg_buf_size = vi->rss_key_size;
> +	sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> +
> +	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> +				  VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> +		dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> +		return false;
> +	}
> +	return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> +	u32 indir_val = 0;
> +	int i = 0;
> +
> +	vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> +	vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> +	vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> +	for (; i < vi->rss_indir_table_size; ++i) {
> +		indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> +		vi->ctrl->rss.indirection_table[i] = indir_val;
> +	}
> +
> +	vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> +	vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> +
> +	netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> +}
> +
>  
>  static void virtnet_get_drvinfo(struct net_device *dev,
>  				struct ethtool_drvinfo *info)
> @@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
>  		vi->duplex = duplex;
>  }
>  
> +static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
> +}
> +
> +static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
> +{
> +	return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
> +}
> +
> +static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			indir[i] = vi->ctrl->rss.indirection_table[i];
> +	}
> +
> +	if (key)
> +		memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
> +
> +	if (hfunc)
> +		*hfunc = ETH_RSS_HASH_TOP;
> +
> +	return 0;
> +}
> +
> +static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int i;
> +
> +	if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> +		return -EOPNOTSUPP;
> +
> +	if (indir) {
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			vi->ctrl->rss.indirection_table[i] = indir[i];
> +	}
> +	if (key)
> +		memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
> +
> +	virtnet_commit_rss_command(vi);
> +
> +	return 0;
> +}
> +
> +static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
> +{
> +	struct virtnet_info *vi = netdev_priv(dev);
> +	int rc = 0;
> +
> +	switch (info->cmd) {
> +	case ETHTOOL_GRXRINGS:
> +		info->data = vi->curr_queue_pairs;
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +	}
> +
> +	return rc;
> +}
> +
>  static const struct ethtool_ops virtnet_ethtool_ops = {
>  	.supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
>  	.get_drvinfo = virtnet_get_drvinfo,
> @@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>  	.set_link_ksettings = virtnet_set_link_ksettings,
>  	.set_coalesce = virtnet_set_coalesce,
>  	.get_coalesce = virtnet_get_coalesce,
> +	.get_rxfh_key_size = virtnet_get_rxfh_key_size,
> +	.get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
> +	.get_rxfh = virtnet_get_rxfh,
> +	.set_rxfh = virtnet_set_rxfh,
> +	.get_rxnfc = virtnet_get_rxnfc,
>  };
>  
>  static void virtnet_freeze_down(struct virtio_device *vdev)
> @@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
>  			     "VIRTIO_NET_F_CTRL_VQ") ||
>  	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
>  	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> -			     "VIRTIO_NET_F_CTRL_VQ"))) {
> +			     "VIRTIO_NET_F_CTRL_VQ") ||
> +	     VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
>  		return false;
>  	}
>  
> @@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	u16 max_queue_pairs;
>  	int mtu;
>  
> -	/* Find if host supports multiqueue virtio_net device */
> -	err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> -				   struct virtio_net_config,
> -				   max_virtqueue_pairs, &max_queue_pairs);
> +	/* Find if host supports multiqueue/rss virtio_net device */
> +	max_queue_pairs = 0;
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> +		max_queue_pairs =
> +		     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
>  
>  	/* We need at least 2 queue's */
> -	if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> +	if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
>  	    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
>  	    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>  		max_queue_pairs = 1;
> @@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
>  		vi->mergeable_rx_bufs = true;
>  
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> +		vi->has_rss = true;
> +		vi->rss_indir_table_size =
> +			virtio_cread16(vdev, offsetof(struct virtio_net_config,
> +						      rss_max_indirection_table_length));
> +		vi->rss_key_size =
> +			virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> +	}
> +
> +	if (vi->has_rss) {
> +		vi->rss_hash_types_supported =
> +		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> +		vi->rss_hash_types_supported &=
> +				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> +				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> +
> +		dev->hw_features |= NETIF_F_RXHASH;
> +	}
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
>  	    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
>  		vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> @@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>  		}
>  	}
>  
> +	if (vi->has_rss) {
> +		rtnl_lock();
> +		virtnet_init_default_rss(vi);
> +		rtnl_unlock();
> +	}
> +
>  	err = register_netdev(dev);
>  	if (err) {
>  		pr_debug("virtio_net: registering device failed\n");
> @@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
>  	VIRTIO_NET_F_CTRL_MAC_ADDR, \
>  	VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> -	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> +	VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> +	VIRTIO_NET_F_RSS
>  
>  static unsigned int features[] = {
>  	VIRTNET_FEATURES,
> -- 
> 2.34.1

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
  2022-01-11 12:00     ` Michael S. Tsirkin
@ 2022-01-17  7:49       ` Andrew Melnichenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnichenko @ 2022-01-17  7:49 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, virtualization, linux-kernel, davem, kuba, Jason Wang,
	Yan Vugenfirer, Yuri Benditovich

Hi all

> Is this correct if both mergeable_rx_bufs and hash_report are set?
Yes, there is a similar code in qemu.

> Can we simply do virtio_cread_feature(vdev, VIRTIO_NET_F_MQ |
> VIRTIO_NET_F_RSS, ...) ?
No, VIRTIO_NET_F_* is bit offset - so in the end "1 <<
(VIRTIO_NET_F_MQ | VIRTIO_NET_F_RSS)" is not valid.

> Is rtnl_lock() really needed here consider we haven't even register netdev?
I'll remove rtnl lock.

> Generally best to avoid __packed.
I'll refactor the structure.

On Tue, Jan 11, 2022 at 2:00 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> > Added features for RSS.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Virtio RSS "IPv6 extensions" hashes disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >  drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 184 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 66439ca488f4..21794731fc75 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,28 @@ struct receive_queue {
> >       struct xdp_rxq_info xdp_rxq;
> >  };
> >
> > +/* This structure can contain rss message with maximum settings for indirection table and keysize
> > + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> > +struct virtio_net_ctrl_rss {
> > +     struct {
> > +             __le32 hash_types;
> > +             __le16 indirection_table_mask;
> > +             __le16 unclassified_queue;
> > +     } __packed table_info;
> > +     u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > +     struct {
> > +             u16 max_tx_vq; /* queues */
> > +             u8 hash_key_length;
> > +     } __packed key_info;
> > +     u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
>
> Generally best to avoid __packed.
> I think it's not a bad idea to just follow the spec when
> you lay out the structures. Makes it easier to follow
> that it matches. Spec has just a single struct:
>
> struct virtio_net_rss_config {
>     le32 hash_types;
>     le16 indirection_table_mask;
>     le16 unclassified_queue;
>     le16 indirection_table[indirection_table_length];
>     le16 max_tx_vq;
>     u8 hash_key_length;
>     u8 hash_key_data[hash_key_length];
> };
>
> and with this layout you don't need __packed.
>
>
>
> >  /* Control VQ buffers: protected by the rtnl lock */
> >  struct control_buf {
> >       struct virtio_net_ctrl_hdr hdr;
> > @@ -178,6 +200,7 @@ struct control_buf {
> >       u8 allmulti;
> >       __virtio16 vid;
> >       __virtio64 offloads;
> > +     struct virtio_net_ctrl_rss rss;
> >  };
> >
> >  struct virtnet_info {
> > @@ -206,6 +229,12 @@ struct virtnet_info {
> >       /* Host will merge rx buffers for big packets (shake it! shake it!) */
> >       bool mergeable_rx_bufs;
> >
> > +     /* Host supports rss and/or hash report */
> > +     bool has_rss;
> > +     u8 rss_key_size;
> > +     u16 rss_indir_table_size;
> > +     u32 rss_hash_types_supported;
> > +
> >       /* Has control virtqueue */
> >       bool has_cvq;
> >
> > @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >       hdr_p = p;
> >
> >       hdr_len = vi->hdr_len;
> > -     if (vi->has_rss_hash_report)
> > -             hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > -     else if (vi->mergeable_rx_bufs)
> > +     if (vi->mergeable_rx_bufs)
> >               hdr_padded_len = sizeof(*hdr);
> >       else
> >               hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
> >       ring->tx_pending = ring->tx_max_pending;
> >  }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > +     struct net_device *dev = vi->dev;
> > +     struct scatterlist sgs[4];
> > +     unsigned int sg_buf_size;
> > +
> > +     /* prepare sgs */
> > +     sg_init_table(sgs, 4);
> > +
> > +     sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> > +     sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> > +
> > +     sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > +     sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > +     sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> > +     sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> > +
> > +     sg_buf_size = vi->rss_key_size;
> > +     sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> > +
> > +     if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > +                               VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > +             dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> > +             return false;
> > +     }
> > +     return true;
> > +}
> > +
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > +     u32 indir_val = 0;
> > +     int i = 0;
> > +
> > +     vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> > +     vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> > +     vi->ctrl->rss.table_info.unclassified_queue = 0;
> > +
> > +     for (; i < vi->rss_indir_table_size; ++i) {
> > +             indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> > +             vi->ctrl->rss.indirection_table[i] = indir_val;
> > +     }
> > +
> > +     vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> > +     vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> > +
> > +     netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> > +}
> > +
> >
> >  static void virtnet_get_drvinfo(struct net_device *dev,
> >                               struct ethtool_drvinfo *info)
> > @@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
> >               vi->duplex = duplex;
> >  }
> >
> > +static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> > +{
> > +     return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
> > +}
> > +
> > +static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
> > +{
> > +     return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
> > +}
> > +
> > +static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int i;
> > +
> > +     if (indir) {
> > +             for (i = 0; i < vi->rss_indir_table_size; ++i)
> > +                     indir[i] = vi->ctrl->rss.indirection_table[i];
> > +     }
> > +
> > +     if (key)
> > +             memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
> > +
> > +     if (hfunc)
> > +             *hfunc = ETH_RSS_HASH_TOP;
> > +
> > +     return 0;
> > +}
> > +
> > +static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int i;
> > +
> > +     if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> > +             return -EOPNOTSUPP;
> > +
> > +     if (indir) {
> > +             for (i = 0; i < vi->rss_indir_table_size; ++i)
> > +                     vi->ctrl->rss.indirection_table[i] = indir[i];
> > +     }
> > +     if (key)
> > +             memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
> > +
> > +     virtnet_commit_rss_command(vi);
> > +
> > +     return 0;
> > +}
> > +
> > +static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int rc = 0;
> > +
> > +     switch (info->cmd) {
> > +     case ETHTOOL_GRXRINGS:
> > +             info->data = vi->curr_queue_pairs;
> > +             break;
> > +     default:
> > +             rc = -EOPNOTSUPP;
> > +     }
> > +
> > +     return rc;
> > +}
> > +
> >  static const struct ethtool_ops virtnet_ethtool_ops = {
> >       .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
> >       .get_drvinfo = virtnet_get_drvinfo,
> > @@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> >       .set_link_ksettings = virtnet_set_link_ksettings,
> >       .set_coalesce = virtnet_set_coalesce,
> >       .get_coalesce = virtnet_get_coalesce,
> > +     .get_rxfh_key_size = virtnet_get_rxfh_key_size,
> > +     .get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
> > +     .get_rxfh = virtnet_get_rxfh,
> > +     .set_rxfh = virtnet_set_rxfh,
> > +     .get_rxnfc = virtnet_get_rxnfc,
> >  };
> >
> >  static void virtnet_freeze_down(struct virtio_device *vdev)
> > @@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> >                            "VIRTIO_NET_F_CTRL_VQ") ||
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> > -                          "VIRTIO_NET_F_CTRL_VQ"))) {
> > +                          "VIRTIO_NET_F_CTRL_VQ") ||
> > +          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> >               return false;
> >       }
> >
> > @@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
> >       u16 max_queue_pairs;
> >       int mtu;
> >
> > -     /* Find if host supports multiqueue virtio_net device */
> > -     err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> > -                                struct virtio_net_config,
> > -                                max_virtqueue_pairs, &max_queue_pairs);
> > +     /* Find if host supports multiqueue/rss virtio_net device */
> > +     max_queue_pairs = 0;
> > +     if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> > +             max_queue_pairs =
> > +                  virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
> >
> >       /* We need at least 2 queue's */
> > -     if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> > +     if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> >           max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
> >           !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
> >               max_queue_pairs = 1;
> > @@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> >               vi->mergeable_rx_bufs = true;
> >
> > +     if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > +             vi->has_rss = true;
> > +             vi->rss_indir_table_size =
> > +                     virtio_cread16(vdev, offsetof(struct virtio_net_config,
> > +                                                   rss_max_indirection_table_length));
> > +             vi->rss_key_size =
> > +                     virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> > +     }
> > +
> > +     if (vi->has_rss) {
> > +             vi->rss_hash_types_supported =
> > +                 virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> > +             vi->rss_hash_types_supported &=
> > +                             ~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> > +                               VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> > +                               VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> > +
> > +             dev->hw_features |= NETIF_F_RXHASH;
> > +     }
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> >           virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> >               vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> > @@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
> >               }
> >       }
> >
> > +     if (vi->has_rss) {
> > +             rtnl_lock();
> > +             virtnet_init_default_rss(vi);
> > +             rtnl_unlock();
> > +     }
> > +
> >       err = register_netdev(dev);
> >       if (err) {
> >               pr_debug("virtio_net: registering device failed\n");
> > @@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
> >       VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> >       VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >       VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > -     VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> > +     VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > +     VIRTIO_NET_F_RSS
> >
> >  static unsigned int features[] = {
> >       VIRTNET_FEATURES,
> > --
> > 2.34.1
>

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

* Re: [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-01-17  7:49       ` Andrew Melnichenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnichenko @ 2022-01-17  7:49 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, linux-kernel, virtualization, Yuri Benditovich,
	Yan Vugenfirer, kuba, davem

Hi all

> Is this correct if both mergeable_rx_bufs and hash_report are set?
Yes, there is a similar code in qemu.

> Can we simply do virtio_cread_feature(vdev, VIRTIO_NET_F_MQ |
> VIRTIO_NET_F_RSS, ...) ?
No, VIRTIO_NET_F_* is bit offset - so in the end "1 <<
(VIRTIO_NET_F_MQ | VIRTIO_NET_F_RSS)" is not valid.

> Is rtnl_lock() really needed here consider we haven't even register netdev?
I'll remove rtnl lock.

> Generally best to avoid __packed.
I'll refactor the structure.

On Tue, Jan 11, 2022 at 2:00 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Jan 09, 2022 at 11:06:57PM +0200, Andrew Melnychenko wrote:
> > Added features for RSS.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Virtio RSS "IPv6 extensions" hashes disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >  drivers/net/virtio_net.c | 194 +++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 184 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 66439ca488f4..21794731fc75 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,28 @@ struct receive_queue {
> >       struct xdp_rxq_info xdp_rxq;
> >  };
> >
> > +/* This structure can contain rss message with maximum settings for indirection table and keysize
> > + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN    128
> > +struct virtio_net_ctrl_rss {
> > +     struct {
> > +             __le32 hash_types;
> > +             __le16 indirection_table_mask;
> > +             __le16 unclassified_queue;
> > +     } __packed table_info;
> > +     u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > +     struct {
> > +             u16 max_tx_vq; /* queues */
> > +             u8 hash_key_length;
> > +     } __packed key_info;
> > +     u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
>
> Generally best to avoid __packed.
> I think it's not a bad idea to just follow the spec when
> you lay out the structures. Makes it easier to follow
> that it matches. Spec has just a single struct:
>
> struct virtio_net_rss_config {
>     le32 hash_types;
>     le16 indirection_table_mask;
>     le16 unclassified_queue;
>     le16 indirection_table[indirection_table_length];
>     le16 max_tx_vq;
>     u8 hash_key_length;
>     u8 hash_key_data[hash_key_length];
> };
>
> and with this layout you don't need __packed.
>
>
>
> >  /* Control VQ buffers: protected by the rtnl lock */
> >  struct control_buf {
> >       struct virtio_net_ctrl_hdr hdr;
> > @@ -178,6 +200,7 @@ struct control_buf {
> >       u8 allmulti;
> >       __virtio16 vid;
> >       __virtio64 offloads;
> > +     struct virtio_net_ctrl_rss rss;
> >  };
> >
> >  struct virtnet_info {
> > @@ -206,6 +229,12 @@ struct virtnet_info {
> >       /* Host will merge rx buffers for big packets (shake it! shake it!) */
> >       bool mergeable_rx_bufs;
> >
> > +     /* Host supports rss and/or hash report */
> > +     bool has_rss;
> > +     u8 rss_key_size;
> > +     u16 rss_indir_table_size;
> > +     u32 rss_hash_types_supported;
> > +
> >       /* Has control virtqueue */
> >       bool has_cvq;
> >
> > @@ -395,9 +424,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >       hdr_p = p;
> >
> >       hdr_len = vi->hdr_len;
> > -     if (vi->has_rss_hash_report)
> > -             hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > -     else if (vi->mergeable_rx_bufs)
> > +     if (vi->mergeable_rx_bufs)
> >               hdr_padded_len = sizeof(*hdr);
> >       else
> >               hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -2184,6 +2211,55 @@ static void virtnet_get_ringparam(struct net_device *dev,
> >       ring->tx_pending = ring->tx_max_pending;
> >  }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > +     struct net_device *dev = vi->dev;
> > +     struct scatterlist sgs[4];
> > +     unsigned int sg_buf_size;
> > +
> > +     /* prepare sgs */
> > +     sg_init_table(sgs, 4);
> > +
> > +     sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> > +     sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> > +
> > +     sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > +     sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > +     sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> > +     sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> > +
> > +     sg_buf_size = vi->rss_key_size;
> > +     sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> > +
> > +     if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > +                               VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > +             dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> > +             return false;
> > +     }
> > +     return true;
> > +}
> > +
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > +     u32 indir_val = 0;
> > +     int i = 0;
> > +
> > +     vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> > +     vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> > +     vi->ctrl->rss.table_info.unclassified_queue = 0;
> > +
> > +     for (; i < vi->rss_indir_table_size; ++i) {
> > +             indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> > +             vi->ctrl->rss.indirection_table[i] = indir_val;
> > +     }
> > +
> > +     vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> > +     vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> > +
> > +     netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> > +}
> > +
> >
> >  static void virtnet_get_drvinfo(struct net_device *dev,
> >                               struct ethtool_drvinfo *info)
> > @@ -2412,6 +2488,71 @@ static void virtnet_update_settings(struct virtnet_info *vi)
> >               vi->duplex = duplex;
> >  }
> >
> > +static u32 virtnet_get_rxfh_key_size(struct net_device *dev)
> > +{
> > +     return ((struct virtnet_info *)netdev_priv(dev))->rss_key_size;
> > +}
> > +
> > +static u32 virtnet_get_rxfh_indir_size(struct net_device *dev)
> > +{
> > +     return ((struct virtnet_info *)netdev_priv(dev))->rss_indir_table_size;
> > +}
> > +
> > +static int virtnet_get_rxfh(struct net_device *dev, u32 *indir, u8 *key, u8 *hfunc)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int i;
> > +
> > +     if (indir) {
> > +             for (i = 0; i < vi->rss_indir_table_size; ++i)
> > +                     indir[i] = vi->ctrl->rss.indirection_table[i];
> > +     }
> > +
> > +     if (key)
> > +             memcpy(key, vi->ctrl->rss.key, vi->rss_key_size);
> > +
> > +     if (hfunc)
> > +             *hfunc = ETH_RSS_HASH_TOP;
> > +
> > +     return 0;
> > +}
> > +
> > +static int virtnet_set_rxfh(struct net_device *dev, const u32 *indir, const u8 *key, const u8 hfunc)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int i;
> > +
> > +     if (hfunc != ETH_RSS_HASH_NO_CHANGE && hfunc != ETH_RSS_HASH_TOP)
> > +             return -EOPNOTSUPP;
> > +
> > +     if (indir) {
> > +             for (i = 0; i < vi->rss_indir_table_size; ++i)
> > +                     vi->ctrl->rss.indirection_table[i] = indir[i];
> > +     }
> > +     if (key)
> > +             memcpy(vi->ctrl->rss.key, key, vi->rss_key_size);
> > +
> > +     virtnet_commit_rss_command(vi);
> > +
> > +     return 0;
> > +}
> > +
> > +static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info, u32 *rule_locs)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int rc = 0;
> > +
> > +     switch (info->cmd) {
> > +     case ETHTOOL_GRXRINGS:
> > +             info->data = vi->curr_queue_pairs;
> > +             break;
> > +     default:
> > +             rc = -EOPNOTSUPP;
> > +     }
> > +
> > +     return rc;
> > +}
> > +
> >  static const struct ethtool_ops virtnet_ethtool_ops = {
> >       .supported_coalesce_params = ETHTOOL_COALESCE_MAX_FRAMES,
> >       .get_drvinfo = virtnet_get_drvinfo,
> > @@ -2427,6 +2568,11 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> >       .set_link_ksettings = virtnet_set_link_ksettings,
> >       .set_coalesce = virtnet_set_coalesce,
> >       .get_coalesce = virtnet_get_coalesce,
> > +     .get_rxfh_key_size = virtnet_get_rxfh_key_size,
> > +     .get_rxfh_indir_size = virtnet_get_rxfh_indir_size,
> > +     .get_rxfh = virtnet_get_rxfh,
> > +     .set_rxfh = virtnet_set_rxfh,
> > +     .get_rxnfc = virtnet_get_rxnfc,
> >  };
> >
> >  static void virtnet_freeze_down(struct virtio_device *vdev)
> > @@ -3073,7 +3219,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> >                            "VIRTIO_NET_F_CTRL_VQ") ||
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> > -                          "VIRTIO_NET_F_CTRL_VQ"))) {
> > +                          "VIRTIO_NET_F_CTRL_VQ") ||
> > +          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> >               return false;
> >       }
> >
> > @@ -3113,13 +3260,14 @@ static int virtnet_probe(struct virtio_device *vdev)
> >       u16 max_queue_pairs;
> >       int mtu;
> >
> > -     /* Find if host supports multiqueue virtio_net device */
> > -     err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> > -                                struct virtio_net_config,
> > -                                max_virtqueue_pairs, &max_queue_pairs);
> > +     /* Find if host supports multiqueue/rss virtio_net device */
> > +     max_queue_pairs = 0;
> > +     if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> > +             max_queue_pairs =
> > +                  virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
> >
> >       /* We need at least 2 queue's */
> > -     if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> > +     if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
> >           max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
> >           !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
> >               max_queue_pairs = 1;
> > @@ -3207,6 +3355,25 @@ static int virtnet_probe(struct virtio_device *vdev)
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> >               vi->mergeable_rx_bufs = true;
> >
> > +     if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > +             vi->has_rss = true;
> > +             vi->rss_indir_table_size =
> > +                     virtio_cread16(vdev, offsetof(struct virtio_net_config,
> > +                                                   rss_max_indirection_table_length));
> > +             vi->rss_key_size =
> > +                     virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> > +     }
> > +
> > +     if (vi->has_rss) {
> > +             vi->rss_hash_types_supported =
> > +                 virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> > +             vi->rss_hash_types_supported &=
> > +                             ~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> > +                               VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> > +                               VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> > +
> > +             dev->hw_features |= NETIF_F_RXHASH;
> > +     }
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> >           virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> >               vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> > @@ -3275,6 +3442,12 @@ static int virtnet_probe(struct virtio_device *vdev)
> >               }
> >       }
> >
> > +     if (vi->has_rss) {
> > +             rtnl_lock();
> > +             virtnet_init_default_rss(vi);
> > +             rtnl_unlock();
> > +     }
> > +
> >       err = register_netdev(dev);
> >       if (err) {
> >               pr_debug("virtio_net: registering device failed\n");
> > @@ -3406,7 +3579,8 @@ static struct virtio_device_id id_table[] = {
> >       VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> >       VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >       VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> > -     VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY
> > +     VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > +     VIRTIO_NET_F_RSS
> >
> >  static unsigned int features[] = {
> >       VIRTNET_FEATURES,
> > --
> > 2.34.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
  2022-01-11  4:05     ` Jason Wang
@ 2022-01-17  7:57       ` Andrew Melnichenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnichenko @ 2022-01-17  7:57 UTC (permalink / raw)
  To: Jason Wang
  Cc: netdev, virtualization, linux-kernel, davem, kuba,
	Michael S. Tsirkin, Yan Vugenfirer, Yuri Benditovich

Hi all.

> I think we should make RSS depend on CTRL_VQ.
> Need to depend on CTRL_VQ here.
I'll fix that.

> Any reason to initialize RSS feature here not the init_default_rss()?
init_default_rss() initializes virtio_net_ctrl_rss structure only, I
think it's a good idea not to mix field initializations.

On Tue, Jan 11, 2022 at 6:06 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> > Added features for RSS hash report.
> > If hash is provided - it sets to skb.
> > Added checks if rss and/or hash are enabled together.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >   drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++------
> >   1 file changed, 48 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 21794731fc75..6e7461b01f87 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -231,6 +231,7 @@ struct virtnet_info {
> >
> >       /* Host supports rss and/or hash report */
> >       bool has_rss;
> > +     bool has_rss_hash_report;
> >       u8 rss_key_size;
> >       u16 rss_indir_table_size;
> >       u32 rss_hash_types_supported;
> > @@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >       hdr_p = p;
> >
> >       hdr_len = vi->hdr_len;
> > -     if (vi->mergeable_rx_bufs)
> > +     if (vi->has_rss_hash_report)
> > +             hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > +     else if (vi->mergeable_rx_bufs)
> >               hdr_padded_len = sizeof(*hdr);
> >       else
> >               hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> >       struct net_device *dev = vi->dev;
> >       struct sk_buff *skb;
> >       struct virtio_net_hdr_mrg_rxbuf *hdr;
> > +     struct virtio_net_hdr_v1_hash *hdr_hash;
> > +     enum pkt_hash_types rss_hash_type;
> >
> >       if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> >               pr_debug("%s: short packet %i\n", dev->name, len);
> > @@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> >               return;
> >
> >       hdr = skb_vnet_hdr(skb);
> > +     if (dev->features & NETIF_F_RXHASH) {
> > +             hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> > +
> > +             switch (hdr_hash->hash_report) {
> > +             case VIRTIO_NET_HASH_REPORT_TCPv4:
> > +             case VIRTIO_NET_HASH_REPORT_UDPv4:
> > +             case VIRTIO_NET_HASH_REPORT_TCPv6:
> > +             case VIRTIO_NET_HASH_REPORT_UDPv6:
> > +             case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > +             case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > +                     rss_hash_type = PKT_HASH_TYPE_L4;
> > +                     break;
> > +             case VIRTIO_NET_HASH_REPORT_IPv4:
> > +             case VIRTIO_NET_HASH_REPORT_IPv6:
> > +             case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > +                     rss_hash_type = PKT_HASH_TYPE_L3;
> > +                     break;
> > +             case VIRTIO_NET_HASH_REPORT_NONE:
> > +             default:
> > +                     rss_hash_type = PKT_HASH_TYPE_NONE;
> > +             }
> > +             skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
> > +     }
> >
> >       if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> >               skb->ip_summed = CHECKSUM_UNNECESSARY;
> > @@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> >       sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> >
> >       if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > -                               VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > +                               vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> > +                               : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
> >               dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> >               return false;
> >       }
> > @@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> >                            "VIRTIO_NET_F_CTRL_VQ") ||
> > -          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> > +          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
>
>
> I think we should make RSS depend on CTRL_VQ.
>
>
> > +          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > +                          "VIRTIO_NET_F_HASH_REPORT"))) {
>
>
> Need to depend on CTRL_VQ here.
>
>
> >               return false;
> >       }
> >
> > @@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> >               vi->mergeable_rx_bufs = true;
> >
> > +     if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> > +             vi->has_rss_hash_report = true;
> > +             vi->rss_indir_table_size = 1;
> > +             vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
>
>
> Any reason to initialize RSS feature here not the init_default_rss()?
>
> Thanks
>
>
> > +     }
> > +
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> >               vi->has_rss = true;
> >               vi->rss_indir_table_size =
> > @@ -3364,7 +3401,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >                       virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> >       }
> >
> > -     if (vi->has_rss) {
> > +     if (vi->has_rss || vi->has_rss_hash_report) {
> >               vi->rss_hash_types_supported =
> >                   virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> >               vi->rss_hash_types_supported &=
> > @@ -3374,8 +3411,11 @@ static int virtnet_probe(struct virtio_device *vdev)
> >
> >               dev->hw_features |= NETIF_F_RXHASH;
> >       }
> > -     if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> > -         virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> > +
> > +     if (vi->has_rss_hash_report)
> > +             vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
> > +     else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> > +              virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> >               vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> >       else
> >               vi->hdr_len = sizeof(struct virtio_net_hdr);
> > @@ -3442,7 +3482,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >               }
> >       }
> >
> > -     if (vi->has_rss) {
> > +     if (vi->has_rss || vi->has_rss_hash_report) {
> >               rtnl_lock();
> >               virtnet_init_default_rss(vi);
> >               rtnl_unlock();
> > @@ -3580,7 +3620,7 @@ static struct virtio_device_id id_table[] = {
> >       VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >       VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> >       VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > -     VIRTIO_NET_F_RSS
> > +     VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
> >
> >   static unsigned int features[] = {
> >       VIRTNET_FEATURES,
>

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

* Re: [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report.
@ 2022-01-17  7:57       ` Andrew Melnichenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnichenko @ 2022-01-17  7:57 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Yuri Benditovich, Yan Vugenfirer, kuba, davem

Hi all.

> I think we should make RSS depend on CTRL_VQ.
> Need to depend on CTRL_VQ here.
I'll fix that.

> Any reason to initialize RSS feature here not the init_default_rss()?
init_default_rss() initializes virtio_net_ctrl_rss structure only, I
think it's a good idea not to mix field initializations.

On Tue, Jan 11, 2022 at 6:06 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> > Added features for RSS hash report.
> > If hash is provided - it sets to skb.
> > Added checks if rss and/or hash are enabled together.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >   drivers/net/virtio_net.c | 56 ++++++++++++++++++++++++++++++++++------
> >   1 file changed, 48 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 21794731fc75..6e7461b01f87 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -231,6 +231,7 @@ struct virtnet_info {
> >
> >       /* Host supports rss and/or hash report */
> >       bool has_rss;
> > +     bool has_rss_hash_report;
> >       u8 rss_key_size;
> >       u16 rss_indir_table_size;
> >       u32 rss_hash_types_supported;
> > @@ -424,7 +425,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >       hdr_p = p;
> >
> >       hdr_len = vi->hdr_len;
> > -     if (vi->mergeable_rx_bufs)
> > +     if (vi->has_rss_hash_report)
> > +             hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > +     else if (vi->mergeable_rx_bufs)
> >               hdr_padded_len = sizeof(*hdr);
> >       else
> >               hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -1160,6 +1163,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> >       struct net_device *dev = vi->dev;
> >       struct sk_buff *skb;
> >       struct virtio_net_hdr_mrg_rxbuf *hdr;
> > +     struct virtio_net_hdr_v1_hash *hdr_hash;
> > +     enum pkt_hash_types rss_hash_type;
> >
> >       if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
> >               pr_debug("%s: short packet %i\n", dev->name, len);
> > @@ -1186,6 +1191,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> >               return;
> >
> >       hdr = skb_vnet_hdr(skb);
> > +     if (dev->features & NETIF_F_RXHASH) {
> > +             hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
> > +
> > +             switch (hdr_hash->hash_report) {
> > +             case VIRTIO_NET_HASH_REPORT_TCPv4:
> > +             case VIRTIO_NET_HASH_REPORT_UDPv4:
> > +             case VIRTIO_NET_HASH_REPORT_TCPv6:
> > +             case VIRTIO_NET_HASH_REPORT_UDPv6:
> > +             case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
> > +             case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
> > +                     rss_hash_type = PKT_HASH_TYPE_L4;
> > +                     break;
> > +             case VIRTIO_NET_HASH_REPORT_IPv4:
> > +             case VIRTIO_NET_HASH_REPORT_IPv6:
> > +             case VIRTIO_NET_HASH_REPORT_IPv6_EX:
> > +                     rss_hash_type = PKT_HASH_TYPE_L3;
> > +                     break;
> > +             case VIRTIO_NET_HASH_REPORT_NONE:
> > +             default:
> > +                     rss_hash_type = PKT_HASH_TYPE_NONE;
> > +             }
> > +             skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
> > +     }
> >
> >       if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> >               skb->ip_summed = CHECKSUM_UNNECESSARY;
> > @@ -2233,7 +2261,8 @@ static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> >       sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> >
> >       if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > -                               VIRTIO_NET_CTRL_MQ_RSS_CONFIG, sgs)) {
> > +                               vi->has_rss ? VIRTIO_NET_CTRL_MQ_RSS_CONFIG
> > +                               : VIRTIO_NET_CTRL_MQ_HASH_CONFIG, sgs)) {
> >               dev_warn(&dev->dev, "VIRTIONET issue with committing RSS sgs\n");
> >               return false;
> >       }
> > @@ -3220,7 +3249,9 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> >            VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> >                            "VIRTIO_NET_F_CTRL_VQ") ||
> > -          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS"))) {
> > +          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
>
>
> I think we should make RSS depend on CTRL_VQ.
>
>
> > +          VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > +                          "VIRTIO_NET_F_HASH_REPORT"))) {
>
>
> Need to depend on CTRL_VQ here.
>
>
> >               return false;
> >       }
> >
> > @@ -3355,6 +3386,12 @@ static int virtnet_probe(struct virtio_device *vdev)
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> >               vi->mergeable_rx_bufs = true;
> >
> > +     if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> > +             vi->has_rss_hash_report = true;
> > +             vi->rss_indir_table_size = 1;
> > +             vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
>
>
> Any reason to initialize RSS feature here not the init_default_rss()?
>
> Thanks
>
>
> > +     }
> > +
> >       if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> >               vi->has_rss = true;
> >               vi->rss_indir_table_size =
> > @@ -3364,7 +3401,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >                       virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
> >       }
> >
> > -     if (vi->has_rss) {
> > +     if (vi->has_rss || vi->has_rss_hash_report) {
> >               vi->rss_hash_types_supported =
> >                   virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
> >               vi->rss_hash_types_supported &=
> > @@ -3374,8 +3411,11 @@ static int virtnet_probe(struct virtio_device *vdev)
> >
> >               dev->hw_features |= NETIF_F_RXHASH;
> >       }
> > -     if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> > -         virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> > +
> > +     if (vi->has_rss_hash_report)
> > +             vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
> > +     else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
> > +              virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> >               vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> >       else
> >               vi->hdr_len = sizeof(struct virtio_net_hdr);
> > @@ -3442,7 +3482,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >               }
> >       }
> >
> > -     if (vi->has_rss) {
> > +     if (vi->has_rss || vi->has_rss_hash_report) {
> >               rtnl_lock();
> >               virtnet_init_default_rss(vi);
> >               rtnl_unlock();
> > @@ -3580,7 +3620,7 @@ static struct virtio_device_id id_table[] = {
> >       VIRTIO_NET_F_CTRL_MAC_ADDR, \
> >       VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> >       VIRTIO_NET_F_SPEED_DUPLEX, VIRTIO_NET_F_STANDBY, \
> > -     VIRTIO_NET_F_RSS
> > +     VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT
> >
> >   static unsigned int features[] = {
> >       VIRTNET_FEATURES,
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
  2022-01-11  4:32     ` Jason Wang
@ 2022-01-17  7:58       ` Andrew Melnichenko
  -1 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnichenko @ 2022-01-17  7:58 UTC (permalink / raw)
  To: Jason Wang
  Cc: netdev, virtualization, linux-kernel, davem, kuba,
	Michael S. Tsirkin, Yan Vugenfirer, Yuri Benditovich

Hi all,

> e.g RXH_VLAN with port hash?
> Any way to merge the two switch? The code is hard to be reviewed anyhow.
I'll refactor virtnet_set_hashflow.

> I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.
Yes, I'll fix that.

On Tue, Jan 11, 2022 at 6:33 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> > Now it's possible to control supported hashflows.
> > Also added hashflow set/get callbacks.
> > Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
> > TCP and UDP supports only:
> > ethtool -U eth0 rx-flow-hash tcp4 sd
> >      RXH_IP_SRC + RXH_IP_DST
> > ethtool -U eth0 rx-flow-hash tcp4 sdfn
> >      RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >   drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 159 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 6e7461b01f87..1b8dd384483c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -235,6 +235,7 @@ struct virtnet_info {
> >       u8 rss_key_size;
> >       u16 rss_indir_table_size;
> >       u32 rss_hash_types_supported;
> > +     u32 rss_hash_types_saved;
> >
> >       /* Has control virtqueue */
> >       bool has_cvq;
> > @@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
> >       int i = 0;
> >
> >       vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> > +     vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> >       vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> >       vi->ctrl->rss.table_info.unclassified_queue = 0;
> >
> > @@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
> >       netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> >   }
> >
> > +static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
> > +{
> > +     info->data = 0;
> > +     switch (info->flow_type) {
> > +     case TCP_V4_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case TCP_V6_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case UDP_V4_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case UDP_V6_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case IPV4_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +
> > +             break;
> > +     case IPV6_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +
> > +             break;
> > +     default:
> > +             info->data = 0;
> > +             break;
> > +     }
> > +}
> > +
> > +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> > +{
> > +     u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
> > +     u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
> > +     u32 new_hashtypes = vi->rss_hash_types_saved;
> > +
> > +     if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
> > +         (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
> > +             return false;
> > +     }
> > +
> > +     if (!is_iphash && is_porthash)
> > +             return false;
>
>
> This seems not filter out all the combinations:
>
> e.g RXH_VLAN with port hash?
>
>
> > +
> > +     switch (info->flow_type) {
> > +     case TCP_V4_FLOW:
> > +     case UDP_V4_FLOW:
> > +     case IPV4_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > +             if (is_iphash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > +
> > +             break;
> > +     case TCP_V6_FLOW:
> > +     case UDP_V6_FLOW:
> > +     case IPV6_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > +             if (is_iphash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > +
> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     switch (info->flow_type) {
> > +     case TCP_V4_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
>
>
> Any way to merge the two switch? The code is hard to be reviewed anyhow.
>
>
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
> > +
> > +             break;
> > +     case UDP_V4_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> > +
> > +             break;
> > +     case TCP_V6_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> > +
> > +             break;
> > +     case UDP_V6_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> > +
> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     if (new_hashtypes != vi->rss_hash_types_saved) {
> > +             vi->rss_hash_types_saved = new_hashtypes;
> > +             vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> > +             if (vi->dev->features & NETIF_F_RXHASH)
> > +                     return virtnet_commit_rss_command(vi);
> > +     }
> > +
> > +     return true;
> > +}
> >
> >   static void virtnet_get_drvinfo(struct net_device *dev,
> >                               struct ethtool_drvinfo *info)
> > @@ -2574,6 +2701,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
> >       switch (info->cmd) {
> >       case ETHTOOL_GRXRINGS:
> >               info->data = vi->curr_queue_pairs;
> > +             break;
> > +     case ETHTOOL_GRXFH:
> > +             virtnet_get_hashflow(vi, info);
> > +             break;
> > +     default:
> > +             rc = -EOPNOTSUPP;
> > +     }
> > +
> > +     return rc;
> > +}
> > +
> > +static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int rc = 0;
> > +
> > +     switch (info->cmd) {
> > +     case ETHTOOL_SRXFH:
> > +             if (!virtnet_set_hashflow(vi, info))
> > +                     rc = -EINVAL;
> > +
> >               break;
> >       default:
> >               rc = -EOPNOTSUPP;
> > @@ -2602,6 +2750,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> >       .get_rxfh = virtnet_get_rxfh,
> >       .set_rxfh = virtnet_set_rxfh,
> >       .get_rxnfc = virtnet_get_rxnfc,
> > +     .set_rxnfc = virtnet_set_rxnfc,
> >   };
> >
> >   static void virtnet_freeze_down(struct virtio_device *vdev)
> > @@ -2854,6 +3003,16 @@ static int virtnet_set_features(struct net_device *dev,
> >               vi->guest_offloads = offloads;
> >       }
> >
> > +     if ((dev->features ^ features) & NETIF_F_RXHASH) {
> > +             if (features & NETIF_F_RXHASH)
> > +                     vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> > +             else
> > +                     vi->ctrl->rss.table_info.hash_types = 0;
>
>
> I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.
>
> Thanks
>
>
> > +
> > +             if (!virtnet_commit_rss_command(vi))
> > +                     return -EINVAL;
> > +     }
> > +
> >       return 0;
> >   }
> >
>

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

* Re: [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2022-01-17  7:58       ` Andrew Melnichenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andrew Melnichenko @ 2022-01-17  7:58 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Yuri Benditovich, Yan Vugenfirer, kuba, davem

Hi all,

> e.g RXH_VLAN with port hash?
> Any way to merge the two switch? The code is hard to be reviewed anyhow.
I'll refactor virtnet_set_hashflow.

> I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.
Yes, I'll fix that.

On Tue, Jan 11, 2022 at 6:33 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/1/10 上午5:06, Andrew Melnychenko 写道:
> > Now it's possible to control supported hashflows.
> > Also added hashflow set/get callbacks.
> > Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
> > TCP and UDP supports only:
> > ethtool -U eth0 rx-flow-hash tcp4 sd
> >      RXH_IP_SRC + RXH_IP_DST
> > ethtool -U eth0 rx-flow-hash tcp4 sdfn
> >      RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> >   drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 159 insertions(+)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 6e7461b01f87..1b8dd384483c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -235,6 +235,7 @@ struct virtnet_info {
> >       u8 rss_key_size;
> >       u16 rss_indir_table_size;
> >       u32 rss_hash_types_supported;
> > +     u32 rss_hash_types_saved;
> >
> >       /* Has control virtqueue */
> >       bool has_cvq;
> > @@ -2275,6 +2276,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
> >       int i = 0;
> >
> >       vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> > +     vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> >       vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> >       vi->ctrl->rss.table_info.unclassified_queue = 0;
> >
> > @@ -2289,6 +2291,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
> >       netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> >   }
> >
> > +static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
> > +{
> > +     info->data = 0;
> > +     switch (info->flow_type) {
> > +     case TCP_V4_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case TCP_V6_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case UDP_V4_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case UDP_V6_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST |
> > +                                              RXH_L4_B_0_1 | RXH_L4_B_2_3;
> > +             } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +             }
> > +             break;
> > +     case IPV4_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +
> > +             break;
> > +     case IPV6_FLOW:
> > +             if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
> > +                     info->data = RXH_IP_SRC | RXH_IP_DST;
> > +
> > +             break;
> > +     default:
> > +             info->data = 0;
> > +             break;
> > +     }
> > +}
> > +
> > +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> > +{
> > +     u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
> > +     u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
> > +     u32 new_hashtypes = vi->rss_hash_types_saved;
> > +
> > +     if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
> > +         (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
> > +             return false;
> > +     }
> > +
> > +     if (!is_iphash && is_porthash)
> > +             return false;
>
>
> This seems not filter out all the combinations:
>
> e.g RXH_VLAN with port hash?
>
>
> > +
> > +     switch (info->flow_type) {
> > +     case TCP_V4_FLOW:
> > +     case UDP_V4_FLOW:
> > +     case IPV4_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > +             if (is_iphash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > +
> > +             break;
> > +     case TCP_V6_FLOW:
> > +     case UDP_V6_FLOW:
> > +     case IPV6_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > +             if (is_iphash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > +
> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     switch (info->flow_type) {
> > +     case TCP_V4_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
>
>
> Any way to merge the two switch? The code is hard to be reviewed anyhow.
>
>
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
> > +
> > +             break;
> > +     case UDP_V4_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
> > +
> > +             break;
> > +     case TCP_V6_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
> > +
> > +             break;
> > +     case UDP_V6_FLOW:
> > +             new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> > +             if (is_porthash)
> > +                     new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
> > +
> > +             break;
> > +     default:
> > +             break;
> > +     }
> > +
> > +     if (new_hashtypes != vi->rss_hash_types_saved) {
> > +             vi->rss_hash_types_saved = new_hashtypes;
> > +             vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> > +             if (vi->dev->features & NETIF_F_RXHASH)
> > +                     return virtnet_commit_rss_command(vi);
> > +     }
> > +
> > +     return true;
> > +}
> >
> >   static void virtnet_get_drvinfo(struct net_device *dev,
> >                               struct ethtool_drvinfo *info)
> > @@ -2574,6 +2701,27 @@ static int virtnet_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
> >       switch (info->cmd) {
> >       case ETHTOOL_GRXRINGS:
> >               info->data = vi->curr_queue_pairs;
> > +             break;
> > +     case ETHTOOL_GRXFH:
> > +             virtnet_get_hashflow(vi, info);
> > +             break;
> > +     default:
> > +             rc = -EOPNOTSUPP;
> > +     }
> > +
> > +     return rc;
> > +}
> > +
> > +static int virtnet_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info)
> > +{
> > +     struct virtnet_info *vi = netdev_priv(dev);
> > +     int rc = 0;
> > +
> > +     switch (info->cmd) {
> > +     case ETHTOOL_SRXFH:
> > +             if (!virtnet_set_hashflow(vi, info))
> > +                     rc = -EINVAL;
> > +
> >               break;
> >       default:
> >               rc = -EOPNOTSUPP;
> > @@ -2602,6 +2750,7 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
> >       .get_rxfh = virtnet_get_rxfh,
> >       .set_rxfh = virtnet_set_rxfh,
> >       .get_rxnfc = virtnet_get_rxnfc,
> > +     .set_rxnfc = virtnet_set_rxnfc,
> >   };
> >
> >   static void virtnet_freeze_down(struct virtio_device *vdev)
> > @@ -2854,6 +3003,16 @@ static int virtnet_set_features(struct net_device *dev,
> >               vi->guest_offloads = offloads;
> >       }
> >
> > +     if ((dev->features ^ features) & NETIF_F_RXHASH) {
> > +             if (features & NETIF_F_RXHASH)
> > +                     vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
> > +             else
> > +                     vi->ctrl->rss.table_info.hash_types = 0;
>
>
> I think it's better to use VIRTIO_NET_HASH_REPORT_NONE here.
>
> Thanks
>
>
> > +
> > +             if (!virtnet_commit_rss_command(vi))
> > +                     return -EINVAL;
> > +     }
> > +
> >       return 0;
> >   }
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

end of thread, other threads:[~2022-01-17  7:59 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-09 21:06 [PATCH 0/4] RSS support for VirtioNet Andrew Melnychenko
2022-01-09 21:06 ` Andrew Melnychenko
2022-01-09 21:06 ` [PATCH 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash Andrew Melnychenko
2022-01-09 21:06   ` Andrew Melnychenko
2022-01-09 22:21   ` Jakub Kicinski
2022-01-09 21:06 ` [PATCH 2/4] drivers/net/virtio_net: Added basic RSS support Andrew Melnychenko
2022-01-09 21:06   ` Andrew Melnychenko
2022-01-11  3:44   ` Jason Wang
2022-01-11  3:44     ` Jason Wang
2022-01-11 12:00   ` Michael S. Tsirkin
2022-01-11 12:00     ` Michael S. Tsirkin
2022-01-17  7:49     ` Andrew Melnichenko
2022-01-17  7:49       ` Andrew Melnichenko
2022-01-09 21:06 ` [PATCH 3/4] drivers/net/virtio_net: Added RSS hash report Andrew Melnychenko
2022-01-09 21:06   ` Andrew Melnychenko
2022-01-11  4:05   ` Jason Wang
2022-01-11  4:05     ` Jason Wang
2022-01-17  7:57     ` Andrew Melnichenko
2022-01-17  7:57       ` Andrew Melnichenko
2022-01-09 21:06 ` [PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control Andrew Melnychenko
2022-01-09 21:06   ` Andrew Melnychenko
2022-01-11  4:32   ` Jason Wang
2022-01-11  4:32     ` Jason Wang
2022-01-17  7:58     ` Andrew Melnichenko
2022-01-17  7:58       ` Andrew Melnichenko

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