* [PATCH v3 0/4] RSS support for VirtioNet.
@ 2022-02-08 18:15 ` Andrew Melnychenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 v2:
* Fixed issue with calculating padded header length.
During review/tests, there was found an issue that
will crash the kernel if VIRTIO_NET_F_MRG_RXBUF
was not set. (thx to Jason Wang <jasowang@redhat.com>)
* Refactored the code according to review.
Changes since v1:
* Refactored virtnet_set_hashflow.
* Refactored virtio_net_ctrl_rss.
* Moved hunks between patches a bit.
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 | 382 +++++++++++++++++++++++++++++++++++++--
1 file changed, 369 insertions(+), 13 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 0/4] RSS support for VirtioNet.
@ 2022-02-08 18:15 ` Andrew Melnychenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 v2:
* Fixed issue with calculating padded header length.
During review/tests, there was found an issue that
will crash the kernel if VIRTIO_NET_F_MRG_RXBUF
was not set. (thx to Jason Wang <jasowang@redhat.com>)
* Refactored the code according to review.
Changes since v1:
* Refactored virtnet_set_hashflow.
* Refactored virtio_net_ctrl_rss.
* Moved hunks between patches a bit.
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 | 382 +++++++++++++++++++++++++++++++++++++--
1 file changed, 369 insertions(+), 13 deletions(-)
--
2.34.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 18:15 ` Andrew Melnychenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a801ea40908f..1404e683a2fd 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)
@@ -1266,7 +1266,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)
@@ -2851,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] 31+ messages in thread
* [PATCH v3 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash.
@ 2022-02-08 18:15 ` Andrew Melnychenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a801ea40908f..1404e683a2fd 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)
@@ -1266,7 +1266,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)
@@ -2851,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] 31+ messages in thread
* [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 18:15 ` Andrew Melnychenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 | 191 +++++++++++++++++++++++++++++++++++++--
1 file changed, 185 insertions(+), 6 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1404e683a2fd..495aed524e33 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -169,6 +169,24 @@ 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 {
+ u32 hash_types;
+ u16 indirection_table_mask;
+ u16 unclassified_queue;
+ u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+ u16 max_tx_vq;
+ u8 hash_key_length;
+ 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 +196,7 @@ struct control_buf {
u8 allmulti;
__virtio16 vid;
__virtio64 offloads;
+ struct virtio_net_ctrl_rss rss;
};
struct virtnet_info {
@@ -206,6 +225,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;
@@ -2184,6 +2209,56 @@ 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 = offsetof(struct virtio_net_ctrl_rss, indirection_table);
+ sg_set_buf(&sgs[0], &vi->ctrl->rss, 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 = offsetof(struct virtio_net_ctrl_rss, key)
+ - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
+ sg_set_buf(&sgs[2], &vi->ctrl->rss.max_tx_vq, 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.hash_types = vi->rss_hash_types_supported;
+ vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
+ vi->ctrl->rss.unclassified_queue = 0;
+
+ for (; i < vi->rss_indir_table_size; ++i) {
+ indir_val = ethtool_rxfh_indir_default(i, vi->curr_queue_pairs);
+ vi->ctrl->rss.indirection_table[i] = indir_val;
+ }
+
+ vi->ctrl->rss.max_tx_vq = vi->curr_queue_pairs;
+ vi->ctrl->rss.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 +2487,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 +2567,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)
@@ -2679,6 +2824,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.hash_types = vi->rss_hash_types_supported;
+ else
+ vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
+
+ if (!virtnet_commit_rss_command(vi))
+ return -EINVAL;
+ }
+
return 0;
}
@@ -3073,6 +3228,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") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
"VIRTIO_NET_F_CTRL_VQ"))) {
return false;
}
@@ -3113,13 +3270,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 = 1;
+ 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 +3365,23 @@ 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));
+
+ 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 +3450,9 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
+ if (vi->has_rss)
+ virtnet_init_default_rss(vi);
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
@@ -3406,7 +3584,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] 31+ messages in thread
* [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-08 18:15 ` Andrew Melnychenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 | 191 +++++++++++++++++++++++++++++++++++++--
1 file changed, 185 insertions(+), 6 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1404e683a2fd..495aed524e33 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -169,6 +169,24 @@ 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 {
+ u32 hash_types;
+ u16 indirection_table_mask;
+ u16 unclassified_queue;
+ u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+ u16 max_tx_vq;
+ u8 hash_key_length;
+ 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 +196,7 @@ struct control_buf {
u8 allmulti;
__virtio16 vid;
__virtio64 offloads;
+ struct virtio_net_ctrl_rss rss;
};
struct virtnet_info {
@@ -206,6 +225,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;
@@ -2184,6 +2209,56 @@ 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 = offsetof(struct virtio_net_ctrl_rss, indirection_table);
+ sg_set_buf(&sgs[0], &vi->ctrl->rss, 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 = offsetof(struct virtio_net_ctrl_rss, key)
+ - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
+ sg_set_buf(&sgs[2], &vi->ctrl->rss.max_tx_vq, 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.hash_types = vi->rss_hash_types_supported;
+ vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
+ vi->ctrl->rss.unclassified_queue = 0;
+
+ for (; i < vi->rss_indir_table_size; ++i) {
+ indir_val = ethtool_rxfh_indir_default(i, vi->curr_queue_pairs);
+ vi->ctrl->rss.indirection_table[i] = indir_val;
+ }
+
+ vi->ctrl->rss.max_tx_vq = vi->curr_queue_pairs;
+ vi->ctrl->rss.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 +2487,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 +2567,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)
@@ -2679,6 +2824,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.hash_types = vi->rss_hash_types_supported;
+ else
+ vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
+
+ if (!virtnet_commit_rss_command(vi))
+ return -EINVAL;
+ }
+
return 0;
}
@@ -3073,6 +3228,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") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
"VIRTIO_NET_F_CTRL_VQ"))) {
return false;
}
@@ -3113,13 +3270,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 = 1;
+ 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 +3365,23 @@ 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));
+
+ 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 +3450,9 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
+ if (vi->has_rss)
+ virtnet_init_default_rss(vi);
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
@@ -3406,7 +3584,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] 31+ messages in thread
* [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 18:15 ` Andrew Melnychenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 | 51 ++++++++++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 495aed524e33..543da2fbdd2d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -227,6 +227,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;
@@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_len = vi->hdr_len;
if (vi->mergeable_rx_bufs)
- hdr_padded_len = sizeof(*hdr);
+ hdr_padded_len = hdr_len;
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1156,6 +1157,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);
@@ -1182,6 +1185,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 && vi->has_rss_hash_report) {
+ 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;
@@ -2232,7 +2258,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;
}
@@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
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_CTRL_VQ") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
"VIRTIO_NET_F_CTRL_VQ"))) {
return false;
}
@@ -3365,8 +3394,13 @@ 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)) {
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
+ vi->has_rss_hash_report = true;
+
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
vi->has_rss = true;
+
+ if (vi->has_rss || vi->has_rss_hash_report) {
vi->rss_indir_table_size =
virtio_cread16(vdev, offsetof(struct virtio_net_config,
rss_max_indirection_table_length));
@@ -3382,8 +3416,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);
@@ -3450,7 +3487,7 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
- if (vi->has_rss)
+ if (vi->has_rss || vi->has_rss_hash_report)
virtnet_init_default_rss(vi);
err = register_netdev(dev);
@@ -3585,7 +3622,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] 31+ messages in thread
* [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
@ 2022-02-08 18:15 ` Andrew Melnychenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 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 | 51 ++++++++++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 495aed524e33..543da2fbdd2d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -227,6 +227,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;
@@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_len = vi->hdr_len;
if (vi->mergeable_rx_bufs)
- hdr_padded_len = sizeof(*hdr);
+ hdr_padded_len = hdr_len;
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1156,6 +1157,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);
@@ -1182,6 +1185,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 && vi->has_rss_hash_report) {
+ 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;
@@ -2232,7 +2258,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;
}
@@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
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_CTRL_VQ") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
"VIRTIO_NET_F_CTRL_VQ"))) {
return false;
}
@@ -3365,8 +3394,13 @@ 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)) {
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
+ vi->has_rss_hash_report = true;
+
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
vi->has_rss = true;
+
+ if (vi->has_rss || vi->has_rss_hash_report) {
vi->rss_indir_table_size =
virtio_cread16(vdev, offsetof(struct virtio_net_config,
rss_max_indirection_table_length));
@@ -3382,8 +3416,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);
@@ -3450,7 +3487,7 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
- if (vi->has_rss)
+ if (vi->has_rss || vi->has_rss_hash_report)
virtnet_init_default_rss(vi);
err = register_netdev(dev);
@@ -3585,7 +3622,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] 31+ messages in thread
* [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 18:15 ` Andrew Melnychenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 UTC (permalink / raw)
To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
Cc: yan, yuri.benditovich
Now it's possible to control supported hashflows.
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 | 141 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 543da2fbdd2d..88759d5e693c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -231,6 +231,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;
@@ -2272,6 +2273,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
int i = 0;
vi->ctrl->rss.hash_types = vi->rss_hash_types_supported;
+ vi->rss_hash_types_saved = vi->rss_hash_types_supported;
vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
vi->ctrl->rss.unclassified_queue = 0;
@@ -2286,6 +2288,121 @@ 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_IPv6)
+ 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)
+{
+ u32 new_hashtypes = vi->rss_hash_types_saved;
+ bool is_disable = info->data & RXH_DISCARD;
+ bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
+
+ /* supports only 'sd', 'sdfn' and 'r' */
+ if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))
+ return false;
+
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
+ break;
+ case UDP_V4_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
+ break;
+ case IPV4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+ if (!is_disable)
+ new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+ break;
+ case TCP_V6_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
+ break;
+ case UDP_V6_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
+ break;
+ case IPV6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+ if (!is_disable)
+ new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+ break;
+ default:
+ /* unsupported flow */
+ return false;
+ }
+
+ /* if unsupported hashtype was set */
+ if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
+ return false;
+
+ if (new_hashtypes != vi->rss_hash_types_saved) {
+ vi->rss_hash_types_saved = new_hashtypes;
+ vi->ctrl->rss.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)
@@ -2571,6 +2688,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;
@@ -2599,6 +2737,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)
@@ -2853,7 +2992,7 @@ static int virtnet_set_features(struct net_device *dev,
if ((dev->features ^ features) & NETIF_F_RXHASH) {
if (features & NETIF_F_RXHASH)
- vi->ctrl->rss.hash_types = vi->rss_hash_types_supported;
+ vi->ctrl->rss.hash_types = vi->rss_hash_types_saved;
else
vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
--
2.34.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2022-02-08 18:15 ` Andrew Melnychenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnychenko @ 2022-02-08 18:15 UTC (permalink / raw)
To: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst
Cc: yan, yuri.benditovich
Now it's possible to control supported hashflows.
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 | 141 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 543da2fbdd2d..88759d5e693c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -231,6 +231,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;
@@ -2272,6 +2273,7 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
int i = 0;
vi->ctrl->rss.hash_types = vi->rss_hash_types_supported;
+ vi->rss_hash_types_saved = vi->rss_hash_types_supported;
vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
vi->ctrl->rss.unclassified_queue = 0;
@@ -2286,6 +2288,121 @@ 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_IPv6)
+ 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)
+{
+ u32 new_hashtypes = vi->rss_hash_types_saved;
+ bool is_disable = info->data & RXH_DISCARD;
+ bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
+
+ /* supports only 'sd', 'sdfn' and 'r' */
+ if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))
+ return false;
+
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
+ break;
+ case UDP_V4_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
+ break;
+ case IPV4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+ if (!is_disable)
+ new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+ break;
+ case TCP_V6_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
+ break;
+ case UDP_V6_FLOW:
+ new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
+ if (!is_disable)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
+ | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
+ break;
+ case IPV6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+ if (!is_disable)
+ new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+ break;
+ default:
+ /* unsupported flow */
+ return false;
+ }
+
+ /* if unsupported hashtype was set */
+ if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
+ return false;
+
+ if (new_hashtypes != vi->rss_hash_types_saved) {
+ vi->rss_hash_types_saved = new_hashtypes;
+ vi->ctrl->rss.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)
@@ -2571,6 +2688,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;
@@ -2599,6 +2737,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)
@@ -2853,7 +2992,7 @@ static int virtnet_set_features(struct net_device *dev,
if ((dev->features ^ features) & NETIF_F_RXHASH) {
if (features & NETIF_F_RXHASH)
- vi->ctrl->rss.hash_types = vi->rss_hash_types_supported;
+ vi->ctrl->rss.hash_types = vi->rss_hash_types_saved;
else
vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
--
2.34.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 20:36 ` Willem de Bruijn
-1 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-08 20:36 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: mst, netdev, linux-kernel, virtualization, yuri.benditovich, yan,
kuba, davem
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> 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 | 191 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 185 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1404e683a2fd..495aed524e33 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,24 @@ 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
Future proof, may want to support larger sizes.
netdevice.h defines NETDEV_RSS_KEY_LEN at 52.
tools/testing/selftests/net/toeplitz.c supports up to 60
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> +struct virtio_net_ctrl_rss {
> + u32 hash_types;
conversely, u32 is a bit extreme?
> + u16 indirection_table_mask;
> + u16 unclassified_queue;
> + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> + u16 max_tx_vq;
> + u8 hash_key_length;
> + 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 +196,7 @@ struct control_buf {
> u8 allmulti;
> __virtio16 vid;
> __virtio64 offloads;
> + struct virtio_net_ctrl_rss rss;
> };
>
> struct virtnet_info {
> @@ -206,6 +225,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;
>
> @@ -2184,6 +2209,56 @@ 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 = offsetof(struct virtio_net_ctrl_rss, indirection_table);
> + sg_set_buf(&sgs[0], &vi->ctrl->rss, 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 = offsetof(struct virtio_net_ctrl_rss, key)
> + - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
> + sg_set_buf(&sgs[2], &vi->ctrl->rss.max_tx_vq, 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.hash_types = vi->rss_hash_types_supported;
> + vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
Is table size always a power of two?
> + vi->ctrl->rss.unclassified_queue = 0;
> +
> + for (; i < vi->rss_indir_table_size; ++i) {
> + indir_val = ethtool_rxfh_indir_default(i, vi->curr_queue_pairs);
> + vi->ctrl->rss.indirection_table[i] = indir_val;
> + }
> +
> + vi->ctrl->rss.max_tx_vq = vi->curr_queue_pairs;
> + vi->ctrl->rss.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 +2487,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 +2567,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)
> @@ -2679,6 +2824,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.hash_types = vi->rss_hash_types_supported;
> + else
> + vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
> +
> + if (!virtnet_commit_rss_command(vi))
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -3073,6 +3228,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") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> "VIRTIO_NET_F_CTRL_VQ"))) {
> return false;
> }
> @@ -3113,13 +3270,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 = 1;
> + 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));
Instead of testing either feature and treating them as somewhat equal,
shouldn't RSS be dependent on MQ?
>
> /* 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 +3365,23 @@ 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));
> +
> + 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;
Only make the feature visible when the hash is actually reported in
the skb, patch 3.
Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
rxhash config.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-08 20:36 ` Willem de Bruijn
0 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-08 20:36 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst,
yan, yuri.benditovich
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> 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 | 191 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 185 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 1404e683a2fd..495aed524e33 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -169,6 +169,24 @@ 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
Future proof, may want to support larger sizes.
netdevice.h defines NETDEV_RSS_KEY_LEN at 52.
tools/testing/selftests/net/toeplitz.c supports up to 60
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> +struct virtio_net_ctrl_rss {
> + u32 hash_types;
conversely, u32 is a bit extreme?
> + u16 indirection_table_mask;
> + u16 unclassified_queue;
> + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> + u16 max_tx_vq;
> + u8 hash_key_length;
> + 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 +196,7 @@ struct control_buf {
> u8 allmulti;
> __virtio16 vid;
> __virtio64 offloads;
> + struct virtio_net_ctrl_rss rss;
> };
>
> struct virtnet_info {
> @@ -206,6 +225,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;
>
> @@ -2184,6 +2209,56 @@ 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 = offsetof(struct virtio_net_ctrl_rss, indirection_table);
> + sg_set_buf(&sgs[0], &vi->ctrl->rss, 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 = offsetof(struct virtio_net_ctrl_rss, key)
> + - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
> + sg_set_buf(&sgs[2], &vi->ctrl->rss.max_tx_vq, 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.hash_types = vi->rss_hash_types_supported;
> + vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
Is table size always a power of two?
> + vi->ctrl->rss.unclassified_queue = 0;
> +
> + for (; i < vi->rss_indir_table_size; ++i) {
> + indir_val = ethtool_rxfh_indir_default(i, vi->curr_queue_pairs);
> + vi->ctrl->rss.indirection_table[i] = indir_val;
> + }
> +
> + vi->ctrl->rss.max_tx_vq = vi->curr_queue_pairs;
> + vi->ctrl->rss.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 +2487,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 +2567,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)
> @@ -2679,6 +2824,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.hash_types = vi->rss_hash_types_supported;
> + else
> + vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
> +
> + if (!virtnet_commit_rss_command(vi))
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> @@ -3073,6 +3228,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") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> "VIRTIO_NET_F_CTRL_VQ"))) {
> return false;
> }
> @@ -3113,13 +3270,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 = 1;
> + 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));
Instead of testing either feature and treating them as somewhat equal,
shouldn't RSS be dependent on MQ?
>
> /* 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 +3365,23 @@ 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));
> +
> + 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;
Only make the feature visible when the hash is actually reported in
the skb, patch 3.
Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
rxhash config.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 20:54 ` Willem de Bruijn
-1 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-08 20:54 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: mst, netdev, linux-kernel, virtualization, yuri.benditovich, yan,
kuba, davem
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
>
> 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 | 51 ++++++++++++++++++++++++++++++++++------
> 1 file changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 495aed524e33..543da2fbdd2d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -227,6 +227,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;
> @@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>
> hdr_len = vi->hdr_len;
> if (vi->mergeable_rx_bufs)
> - hdr_padded_len = sizeof(*hdr);
> + hdr_padded_len = hdr_len;
Belongs in patch 1?
> else
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
>
> @@ -1156,6 +1157,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);
> @@ -1182,6 +1185,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 && vi->has_rss_hash_report) {
Can the first be true if the second is not?
> + 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);
> + }
so many lines, perhaps deserves a helper function
>
> if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> @@ -2232,7 +2258,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;
> }
> @@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> 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_CTRL_VQ") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> "VIRTIO_NET_F_CTRL_VQ"))) {
> return false;
> }
> @@ -3365,8 +3394,13 @@ 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)) {
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> + vi->has_rss_hash_report = true;
> +
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> vi->has_rss = true;
> +
> + if (vi->has_rss || vi->has_rss_hash_report) {
> vi->rss_indir_table_size =
> virtio_cread16(vdev, offsetof(struct virtio_net_config,
should indir table size be zero if only hash report is enabled?
> rss_max_indirection_table_length));
> @@ -3382,8 +3416,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);
> @@ -3450,7 +3487,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> }
> }
>
> - if (vi->has_rss)
> + if (vi->has_rss || vi->has_rss_hash_report)
> virtnet_init_default_rss(vi);
>
> err = register_netdev(dev);
> @@ -3585,7 +3622,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 [flat|nested] 31+ messages in thread
* Re: [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
@ 2022-02-08 20:54 ` Willem de Bruijn
0 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-08 20:54 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst,
yan, yuri.benditovich
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
>
> 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 | 51 ++++++++++++++++++++++++++++++++++------
> 1 file changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 495aed524e33..543da2fbdd2d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -227,6 +227,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;
> @@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>
> hdr_len = vi->hdr_len;
> if (vi->mergeable_rx_bufs)
> - hdr_padded_len = sizeof(*hdr);
> + hdr_padded_len = hdr_len;
Belongs in patch 1?
> else
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
>
> @@ -1156,6 +1157,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);
> @@ -1182,6 +1185,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 && vi->has_rss_hash_report) {
Can the first be true if the second is not?
> + 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);
> + }
so many lines, perhaps deserves a helper function
>
> if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> @@ -2232,7 +2258,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;
> }
> @@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> 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_CTRL_VQ") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> "VIRTIO_NET_F_CTRL_VQ"))) {
> return false;
> }
> @@ -3365,8 +3394,13 @@ 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)) {
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> + vi->has_rss_hash_report = true;
> +
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> vi->has_rss = true;
> +
> + if (vi->has_rss || vi->has_rss_hash_report) {
> vi->rss_indir_table_size =
> virtio_cread16(vdev, offsetof(struct virtio_net_config,
should indir table size be zero if only hash report is enabled?
> rss_max_indirection_table_length));
> @@ -3382,8 +3416,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);
> @@ -3450,7 +3487,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> }
> }
>
> - if (vi->has_rss)
> + if (vi->has_rss || vi->has_rss_hash_report)
> virtnet_init_default_rss(vi);
>
> err = register_netdev(dev);
> @@ -3585,7 +3622,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 [flat|nested] 31+ messages in thread
* Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
2022-02-08 18:15 ` Andrew Melnychenko
@ 2022-02-08 20:58 ` Willem de Bruijn
-1 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-08 20:58 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: mst, netdev, linux-kernel, virtualization, yuri.benditovich, yan,
kuba, davem
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
>
> Now it's possible to control supported hashflows.
> Added hashflow set/get callbacks.
> Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
I don't follow this comment. Can you elaborate?
> 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 | 141 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 140 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 543da2fbdd2d..88759d5e693c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -231,6 +231,7 @@ struct virtnet_info {
> u8 rss_key_size;
> u16 rss_indir_table_size;
> u32 rss_hash_types_supported;
> + u32 rss_hash_types_saved;
hash_types_active?
> +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> + u32 new_hashtypes = vi->rss_hash_types_saved;
> + bool is_disable = info->data & RXH_DISCARD;
> + bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
> +
> + /* supports only 'sd', 'sdfn' and 'r' */
> + if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))
maybe add an is_l3
> + return false;
> +
> + switch (info->flow_type) {
> + case TCP_V4_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
> + break;
> + case UDP_V4_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
> + break;
> + case IPV4_FLOW:
> + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> + if (!is_disable)
> + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> + break;
> + case TCP_V6_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
> + break;
> + case UDP_V6_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
> + break;
> + case IPV6_FLOW:
> + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> + if (!is_disable)
> + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> + break;
> + default:
> + /* unsupported flow */
> + return false;
> + }
> +
> + /* if unsupported hashtype was set */
> + if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
> + return false;
> +
> + if (new_hashtypes != vi->rss_hash_types_saved) {
> + vi->rss_hash_types_saved = new_hashtypes;
should only be updated if the commit function returned success?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2022-02-08 20:58 ` Willem de Bruijn
0 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-08 20:58 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: netdev, virtualization, linux-kernel, davem, kuba, jasowang, mst,
yan, yuri.benditovich
On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
>
> Now it's possible to control supported hashflows.
> Added hashflow set/get callbacks.
> Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
I don't follow this comment. Can you elaborate?
> 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 | 141 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 140 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 543da2fbdd2d..88759d5e693c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -231,6 +231,7 @@ struct virtnet_info {
> u8 rss_key_size;
> u16 rss_indir_table_size;
> u32 rss_hash_types_supported;
> + u32 rss_hash_types_saved;
hash_types_active?
> +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> +{
> + u32 new_hashtypes = vi->rss_hash_types_saved;
> + bool is_disable = info->data & RXH_DISCARD;
> + bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
> +
> + /* supports only 'sd', 'sdfn' and 'r' */
> + if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))
maybe add an is_l3
> + return false;
> +
> + switch (info->flow_type) {
> + case TCP_V4_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
> + break;
> + case UDP_V4_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
> + break;
> + case IPV4_FLOW:
> + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> + if (!is_disable)
> + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> + break;
> + case TCP_V6_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
> + break;
> + case UDP_V6_FLOW:
> + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
> + if (!is_disable)
> + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
> + break;
> + case IPV6_FLOW:
> + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> + if (!is_disable)
> + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> + break;
> + default:
> + /* unsupported flow */
> + return false;
> + }
> +
> + /* if unsupported hashtype was set */
> + if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
> + return false;
> +
> + if (new_hashtypes != vi->rss_hash_types_saved) {
> + vi->rss_hash_types_saved = new_hashtypes;
should only be updated if the commit function returned success?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
2022-02-08 18:15 ` Andrew Melnychenko
(?)
(?)
@ 2022-02-09 15:09 ` kernel test robot
-1 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2022-02-09 15:09 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6331 bytes --]
Hi Andrew,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on net/master horms-ipvs/master net-next/master linus/master v5.17-rc3 next-20220209]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Andrew-Melnychenko/RSS-support-for-VirtioNet/20220209-021715
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: arm64-randconfig-s032-20220208 (https://download.01.org/0day-ci/archive/20220209/202202092243.OLKO83i2-lkp(a)intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/92ce7fd51cc1e4c8bd576ec43086416533212c26
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Melnychenko/RSS-support-for-VirtioNet/20220209-021715
git checkout 92ce7fd51cc1e4c8bd576ec43086416533212c26
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/net/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1191:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1209:43: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned int [usertype] hash @@ got restricted __le32 [usertype] hash_value @@
drivers/net/virtio_net.c:1209:43: sparse: expected unsigned int [usertype] hash
drivers/net/virtio_net.c:1209:43: sparse: got restricted __le32 [usertype] hash_value
drivers/net/virtio_net.c:1568:13: sparse: sparse: context imbalance in 'virtnet_poll_cleantx' - different lock contexts for basic block
vim +1191 drivers/net/virtio_net.c
1151
1152 static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
1153 void *buf, unsigned int len, void **ctx,
1154 unsigned int *xdp_xmit,
1155 struct virtnet_rq_stats *stats)
1156 {
1157 struct net_device *dev = vi->dev;
1158 struct sk_buff *skb;
1159 struct virtio_net_hdr_mrg_rxbuf *hdr;
1160 struct virtio_net_hdr_v1_hash *hdr_hash;
1161 enum pkt_hash_types rss_hash_type;
1162
1163 if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
1164 pr_debug("%s: short packet %i\n", dev->name, len);
1165 dev->stats.rx_length_errors++;
1166 if (vi->mergeable_rx_bufs) {
1167 put_page(virt_to_head_page(buf));
1168 } else if (vi->big_packets) {
1169 give_pages(rq, buf);
1170 } else {
1171 put_page(virt_to_head_page(buf));
1172 }
1173 return;
1174 }
1175
1176 if (vi->mergeable_rx_bufs)
1177 skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
1178 stats);
1179 else if (vi->big_packets)
1180 skb = receive_big(dev, vi, rq, buf, len, stats);
1181 else
1182 skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
1183
1184 if (unlikely(!skb))
1185 return;
1186
1187 hdr = skb_vnet_hdr(skb);
1188 if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) {
1189 hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
1190
> 1191 switch (hdr_hash->hash_report) {
1192 case VIRTIO_NET_HASH_REPORT_TCPv4:
1193 case VIRTIO_NET_HASH_REPORT_UDPv4:
1194 case VIRTIO_NET_HASH_REPORT_TCPv6:
1195 case VIRTIO_NET_HASH_REPORT_UDPv6:
1196 case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
1197 case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
1198 rss_hash_type = PKT_HASH_TYPE_L4;
1199 break;
1200 case VIRTIO_NET_HASH_REPORT_IPv4:
1201 case VIRTIO_NET_HASH_REPORT_IPv6:
1202 case VIRTIO_NET_HASH_REPORT_IPv6_EX:
1203 rss_hash_type = PKT_HASH_TYPE_L3;
1204 break;
1205 case VIRTIO_NET_HASH_REPORT_NONE:
1206 default:
1207 rss_hash_type = PKT_HASH_TYPE_NONE;
1208 }
> 1209 skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
1210 }
1211
1212 if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
1213 skb->ip_summed = CHECKSUM_UNNECESSARY;
1214
1215 if (virtio_net_hdr_to_skb(skb, &hdr->hdr,
1216 virtio_is_little_endian(vi->vdev))) {
1217 net_warn_ratelimited("%s: bad gso: type: %u, size: %u\n",
1218 dev->name, hdr->hdr.gso_type,
1219 hdr->hdr.gso_size);
1220 goto frame_err;
1221 }
1222
1223 skb_record_rx_queue(skb, vq2rxq(rq->vq));
1224 skb->protocol = eth_type_trans(skb, dev);
1225 pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
1226 ntohs(skb->protocol), skb->len, skb->pkt_type);
1227
1228 napi_gro_receive(&rq->napi, skb);
1229 return;
1230
1231 frame_err:
1232 dev->stats.rx_frame_errors++;
1233 dev_kfree_skb(skb);
1234 }
1235
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-08 20:36 ` Willem de Bruijn
@ 2022-02-13 17:01 ` Andrew Melnichenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-13 17:01 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, virtualization, LKML, David S. Miller,
Jakub Kicinski, Jason Wang, Michael S. Tsirkin, Yan Vugenfirer,
Yuri Benditovich
Hi all,
On Tue, Feb 8, 2022 at 10:37 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> 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 | 191 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 185 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 1404e683a2fd..495aed524e33 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,24 @@ 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
>
> Future proof, may want to support larger sizes.
>
> netdevice.h defines NETDEV_RSS_KEY_LEN at 52.
>
> tools/testing/selftests/net/toeplitz.c supports up to 60
According to virtio specification, the length of the key is
40bytes(and an indirection table is 128 entries max).
So for now, we support a maximum of the spec regardless of what the
kernel is capable of.
>
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> > +struct virtio_net_ctrl_rss {
> > + u32 hash_types;
>
> conversely, u32 is a bit extreme?
No, the structure virtio_net_ctrl_rss is specified by the specification.
>
> > + u16 indirection_table_mask;
> > + u16 unclassified_queue;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + u16 max_tx_vq;
> > + u8 hash_key_length;
> > + 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 +196,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -206,6 +225,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;
> >
> > @@ -2184,6 +2209,56 @@ 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 = offsetof(struct virtio_net_ctrl_rss, indirection_table);
> > + sg_set_buf(&sgs[0], &vi->ctrl->rss, 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 = offsetof(struct virtio_net_ctrl_rss, key)
> > + - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
> > + sg_set_buf(&sgs[2], &vi->ctrl->rss.max_tx_vq, 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.hash_types = vi->rss_hash_types_supported;
> > + vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
>
> Is table size always a power of two?
Yes, it should be.
>
> > + vi->ctrl->rss.unclassified_queue = 0;
> > +
> > + for (; i < vi->rss_indir_table_size; ++i) {
> > + indir_val = ethtool_rxfh_indir_default(i, vi->curr_queue_pairs);
> > + vi->ctrl->rss.indirection_table[i] = indir_val;
> > + }
> > +
> > + vi->ctrl->rss.max_tx_vq = vi->curr_queue_pairs;
> > + vi->ctrl->rss.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 +2487,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 +2567,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)
> > @@ -2679,6 +2824,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.hash_types = vi->rss_hash_types_supported;
> > + else
> > + vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
> > +
> > + if (!virtnet_commit_rss_command(vi))
> > + return -EINVAL;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -3073,6 +3228,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") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> > "VIRTIO_NET_F_CTRL_VQ"))) {
> > return false;
> > }
> > @@ -3113,13 +3270,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 = 1;
> > + 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));
>
> Instead of testing either feature and treating them as somewhat equal,
> shouldn't RSS be dependent on MQ?
No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
>
> >
> > /* 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 +3365,23 @@ 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));
> > +
> > + 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;
>
> Only make the feature visible when the hash is actually reported in
> the skb, patch 3.
VirtioNET has two features: RSS(steering only) and hash(hash report in
vnet header)
Both features may be enabled/disabled separately:
1. rss on and hash off - packets steered to the corresponding vqs
2. rss off and hash on - packets steered by tap(like mq) but headers
have properly calculated hash.
3. rss on and hash on - packets steered to corresponding vqs and hash
is present in the header.
RXHASH feature allows the user to enable/disable the rss/hash(any combination).
I think it's a good idea to leave RXHASH in patch 2/4 to give the user
ability to manipulate the rss only feature.
But, if you think that it requires to move it to the 3/4, I'll do it.
>
> Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> rxhash config.
Currently:
Patch 2/4 - adds VirtioNet rss feature.
Patch 3/4 - adds VirtioNet hash report feature.
Patch 4/4 - adds the ability to manipulate supported hash types.
Can you provide more detailed suggestions on how to move hunks?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-13 17:01 ` Andrew Melnichenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-13 17:01 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Network Development, LKML, virtualization,
Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
Hi all,
On Tue, Feb 8, 2022 at 10:37 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> 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 | 191 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 185 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 1404e683a2fd..495aed524e33 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -169,6 +169,24 @@ 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
>
> Future proof, may want to support larger sizes.
>
> netdevice.h defines NETDEV_RSS_KEY_LEN at 52.
>
> tools/testing/selftests/net/toeplitz.c supports up to 60
According to virtio specification, the length of the key is
40bytes(and an indirection table is 128 entries max).
So for now, we support a maximum of the spec regardless of what the
kernel is capable of.
>
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> > +struct virtio_net_ctrl_rss {
> > + u32 hash_types;
>
> conversely, u32 is a bit extreme?
No, the structure virtio_net_ctrl_rss is specified by the specification.
>
> > + u16 indirection_table_mask;
> > + u16 unclassified_queue;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + u16 max_tx_vq;
> > + u8 hash_key_length;
> > + 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 +196,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -206,6 +225,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;
> >
> > @@ -2184,6 +2209,56 @@ 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 = offsetof(struct virtio_net_ctrl_rss, indirection_table);
> > + sg_set_buf(&sgs[0], &vi->ctrl->rss, 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 = offsetof(struct virtio_net_ctrl_rss, key)
> > + - offsetof(struct virtio_net_ctrl_rss, max_tx_vq);
> > + sg_set_buf(&sgs[2], &vi->ctrl->rss.max_tx_vq, 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.hash_types = vi->rss_hash_types_supported;
> > + vi->ctrl->rss.indirection_table_mask = vi->rss_indir_table_size - 1;
>
> Is table size always a power of two?
Yes, it should be.
>
> > + vi->ctrl->rss.unclassified_queue = 0;
> > +
> > + for (; i < vi->rss_indir_table_size; ++i) {
> > + indir_val = ethtool_rxfh_indir_default(i, vi->curr_queue_pairs);
> > + vi->ctrl->rss.indirection_table[i] = indir_val;
> > + }
> > +
> > + vi->ctrl->rss.max_tx_vq = vi->curr_queue_pairs;
> > + vi->ctrl->rss.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 +2487,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 +2567,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)
> > @@ -2679,6 +2824,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.hash_types = vi->rss_hash_types_supported;
> > + else
> > + vi->ctrl->rss.hash_types = VIRTIO_NET_HASH_REPORT_NONE;
> > +
> > + if (!virtnet_commit_rss_command(vi))
> > + return -EINVAL;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -3073,6 +3228,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") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS,
> > "VIRTIO_NET_F_CTRL_VQ"))) {
> > return false;
> > }
> > @@ -3113,13 +3270,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 = 1;
> > + 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));
>
> Instead of testing either feature and treating them as somewhat equal,
> shouldn't RSS be dependent on MQ?
No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
>
> >
> > /* 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 +3365,23 @@ 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));
> > +
> > + 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;
>
> Only make the feature visible when the hash is actually reported in
> the skb, patch 3.
VirtioNET has two features: RSS(steering only) and hash(hash report in
vnet header)
Both features may be enabled/disabled separately:
1. rss on and hash off - packets steered to the corresponding vqs
2. rss off and hash on - packets steered by tap(like mq) but headers
have properly calculated hash.
3. rss on and hash on - packets steered to corresponding vqs and hash
is present in the header.
RXHASH feature allows the user to enable/disable the rss/hash(any combination).
I think it's a good idea to leave RXHASH in patch 2/4 to give the user
ability to manipulate the rss only feature.
But, if you think that it requires to move it to the 3/4, I'll do it.
>
> Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> rxhash config.
Currently:
Patch 2/4 - adds VirtioNet rss feature.
Patch 3/4 - adds VirtioNet hash report feature.
Patch 4/4 - adds the ability to manipulate supported hash types.
Can you provide more detailed suggestions on how to move hunks?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
2022-02-08 20:54 ` Willem de Bruijn
@ 2022-02-13 17:08 ` Andrew Melnichenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-13 17:08 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, virtualization, LKML, David S. Miller,
Jakub Kicinski, Jason Wang, Michael S. Tsirkin, Yan Vugenfirer,
Yuri Benditovich
Hi all,
On Tue, Feb 8, 2022 at 10:55 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
> >
> > 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 | 51 ++++++++++++++++++++++++++++++++++------
> > 1 file changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 495aed524e33..543da2fbdd2d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -227,6 +227,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;
> > @@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >
> > hdr_len = vi->hdr_len;
> > if (vi->mergeable_rx_bufs)
> > - hdr_padded_len = sizeof(*hdr);
> > + hdr_padded_len = hdr_len;
>
> Belongs in patch 1?
Yeah, I'll move it.
>
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> >
> > @@ -1156,6 +1157,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);
> > @@ -1182,6 +1185,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 && vi->has_rss_hash_report) {
>
> Can the first be true if the second is not?
Yes, RSS may be enabled, but the hash report feature is disabled.
For now, it's possible to enable/disable VirtioNet RSS by manipulating 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);
> > + }
>
> so many lines, perhaps deserves a helper function
Ok, I'll create the helper.
>
> >
> > if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> > skb->ip_summed = CHECKSUM_UNNECESSARY;
> > @@ -2232,7 +2258,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;
> > }
> > @@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> > 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_CTRL_VQ") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > "VIRTIO_NET_F_CTRL_VQ"))) {
> > return false;
> > }
> > @@ -3365,8 +3394,13 @@ 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)) {
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> > + vi->has_rss_hash_report = true;
> > +
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> > vi->has_rss = true;
> > +
> > + if (vi->has_rss || vi->has_rss_hash_report) {
> > vi->rss_indir_table_size =
> > virtio_cread16(vdev, offsetof(struct virtio_net_config,
>
> should indir table size be zero if only hash report is enabled?
Not really - but of course, for hash only, the table is not necessary.
(Qemu always provides the table with size 1, I'll add checks for zero sizes
in case of hardware implementation.)
>
> > rss_max_indirection_table_length));
> > @@ -3382,8 +3416,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);
> > @@ -3450,7 +3487,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > }
> > }
> >
> > - if (vi->has_rss)
> > + if (vi->has_rss || vi->has_rss_hash_report)
> > virtnet_init_default_rss(vi);
> >
> > err = register_netdev(dev);
> > @@ -3585,7 +3622,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 [flat|nested] 31+ messages in thread
* Re: [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report.
@ 2022-02-13 17:08 ` Andrew Melnichenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-13 17:08 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Network Development, LKML, virtualization,
Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
Hi all,
On Tue, Feb 8, 2022 at 10:55 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
> >
> > 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 | 51 ++++++++++++++++++++++++++++++++++------
> > 1 file changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 495aed524e33..543da2fbdd2d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -227,6 +227,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;
> > @@ -421,7 +422,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> >
> > hdr_len = vi->hdr_len;
> > if (vi->mergeable_rx_bufs)
> > - hdr_padded_len = sizeof(*hdr);
> > + hdr_padded_len = hdr_len;
>
> Belongs in patch 1?
Yeah, I'll move it.
>
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> >
> > @@ -1156,6 +1157,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);
> > @@ -1182,6 +1185,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 && vi->has_rss_hash_report) {
>
> Can the first be true if the second is not?
Yes, RSS may be enabled, but the hash report feature is disabled.
For now, it's possible to enable/disable VirtioNet RSS by manipulating 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);
> > + }
>
> so many lines, perhaps deserves a helper function
Ok, I'll create the helper.
>
> >
> > if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
> > skb->ip_summed = CHECKSUM_UNNECESSARY;
> > @@ -2232,7 +2258,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;
> > }
> > @@ -3230,6 +3257,8 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> > 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_CTRL_VQ") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > "VIRTIO_NET_F_CTRL_VQ"))) {
> > return false;
> > }
> > @@ -3365,8 +3394,13 @@ 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)) {
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> > + vi->has_rss_hash_report = true;
> > +
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> > vi->has_rss = true;
> > +
> > + if (vi->has_rss || vi->has_rss_hash_report) {
> > vi->rss_indir_table_size =
> > virtio_cread16(vdev, offsetof(struct virtio_net_config,
>
> should indir table size be zero if only hash report is enabled?
Not really - but of course, for hash only, the table is not necessary.
(Qemu always provides the table with size 1, I'll add checks for zero sizes
in case of hardware implementation.)
>
> > rss_max_indirection_table_length));
> > @@ -3382,8 +3416,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);
> > @@ -3450,7 +3487,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> > }
> > }
> >
> > - if (vi->has_rss)
> > + if (vi->has_rss || vi->has_rss_hash_report)
> > virtnet_init_default_rss(vi);
> >
> > err = register_netdev(dev);
> > @@ -3585,7 +3622,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 [flat|nested] 31+ messages in thread
* Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
2022-02-08 20:58 ` Willem de Bruijn
@ 2022-02-13 17:22 ` Andrew Melnichenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-13 17:22 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, virtualization, LKML, David S. Miller,
Jakub Kicinski, Jason Wang, Michael S. Tsirkin, Yan Vugenfirer,
Yuri Benditovich
Hi all,
On Tue, Feb 8, 2022 at 10:59 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
> >
> > Now it's possible to control supported hashflows.
> > Added hashflow set/get callbacks.
> > Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
>
> I don't follow this comment. Can you elaborate?
I'll rephrase it in next version of patches.
The idea is that VirtioNet RSS doesn't distinguish IP hashes between
TCP and UDP.
For TCP and UDP it's possible to set IP+PORT hashes.
But disabling IP hashes will disable them for TCP and UDP simultaneously.
It's possible to set IP+PORT for TCP and IP for everything else(UDP, ICMP etc.)
>
> > 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 | 141 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 140 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 543da2fbdd2d..88759d5e693c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -231,6 +231,7 @@ struct virtnet_info {
> > u8 rss_key_size;
> > u16 rss_indir_table_size;
> > u32 rss_hash_types_supported;
> > + u32 rss_hash_types_saved;
>
> hash_types_active?
I think "hash_types_saved" is more suitable for the current field.
Idea is that the user may disable RSS/HASH and we need to save
what hash type configurations previously were enabled.
So, we can restore it when the user will enable RSS/HASH back.
>
> > +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> > +{
> > + u32 new_hashtypes = vi->rss_hash_types_saved;
> > + bool is_disable = info->data & RXH_DISCARD;
> > + bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
> > +
> > + /* supports only 'sd', 'sdfn' and 'r' */
> > + if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))
>
> maybe add an is_l3
There used to be "is_l3", but that variable was used only in that
condition statement.
So I've decided to inplace it.
>
> > + return false;
> > +
> > + switch (info->flow_type) {
> > + case TCP_V4_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
> > + break;
> > + case UDP_V4_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
> > + break;
> > + case IPV4_FLOW:
> > + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > + if (!is_disable)
> > + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > + break;
> > + case TCP_V6_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
> > + break;
> > + case UDP_V6_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
> > + break;
> > + case IPV6_FLOW:
> > + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > + if (!is_disable)
> > + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > + break;
> > + default:
> > + /* unsupported flow */
> > + return false;
> > + }
> > +
> > + /* if unsupported hashtype was set */
> > + if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
> > + return false;
> > +
> > + if (new_hashtypes != vi->rss_hash_types_saved) {
> > + vi->rss_hash_types_saved = new_hashtypes;
>
> should only be updated if the commit function returned success?
Not really, we already made all checks against "supported" hash types.
Also, the commit function may not be called if RSS is disabled by the user.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2022-02-13 17:22 ` Andrew Melnichenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-13 17:22 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Network Development, LKML, virtualization,
Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
Hi all,
On Tue, Feb 8, 2022 at 10:59 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Tue, Feb 8, 2022 at 1:19 PM Andrew Melnychenko <andrew@daynix.com> wrote:
> >
> > Now it's possible to control supported hashflows.
> > Added hashflow set/get callbacks.
> > Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
>
> I don't follow this comment. Can you elaborate?
I'll rephrase it in next version of patches.
The idea is that VirtioNet RSS doesn't distinguish IP hashes between
TCP and UDP.
For TCP and UDP it's possible to set IP+PORT hashes.
But disabling IP hashes will disable them for TCP and UDP simultaneously.
It's possible to set IP+PORT for TCP and IP for everything else(UDP, ICMP etc.)
>
> > 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 | 141 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 140 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 543da2fbdd2d..88759d5e693c 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -231,6 +231,7 @@ struct virtnet_info {
> > u8 rss_key_size;
> > u16 rss_indir_table_size;
> > u32 rss_hash_types_supported;
> > + u32 rss_hash_types_saved;
>
> hash_types_active?
I think "hash_types_saved" is more suitable for the current field.
Idea is that the user may disable RSS/HASH and we need to save
what hash type configurations previously were enabled.
So, we can restore it when the user will enable RSS/HASH back.
>
> > +static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
> > +{
> > + u32 new_hashtypes = vi->rss_hash_types_saved;
> > + bool is_disable = info->data & RXH_DISCARD;
> > + bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
> > +
> > + /* supports only 'sd', 'sdfn' and 'r' */
> > + if (!((info->data == (RXH_IP_SRC | RXH_IP_DST)) | is_l4 | is_disable))
>
> maybe add an is_l3
There used to be "is_l3", but that variable was used only in that
condition statement.
So I've decided to inplace it.
>
> > + return false;
> > +
> > + switch (info->flow_type) {
> > + case TCP_V4_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_TCPv4);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv4 : 0);
> > + break;
> > + case UDP_V4_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv4 | VIRTIO_NET_RSS_HASH_TYPE_UDPv4);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv4 : 0);
> > + break;
> > + case IPV4_FLOW:
> > + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > + if (!is_disable)
> > + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv4;
> > + break;
> > + case TCP_V6_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_TCPv6);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_TCPv6 : 0);
> > + break;
> > + case UDP_V6_FLOW:
> > + new_hashtypes &= ~(VIRTIO_NET_RSS_HASH_TYPE_IPv6 | VIRTIO_NET_RSS_HASH_TYPE_UDPv6);
> > + if (!is_disable)
> > + new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6
> > + | (is_l4 ? VIRTIO_NET_RSS_HASH_TYPE_UDPv6 : 0);
> > + break;
> > + case IPV6_FLOW:
> > + new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > + if (!is_disable)
> > + new_hashtypes = VIRTIO_NET_RSS_HASH_TYPE_IPv6;
> > + break;
> > + default:
> > + /* unsupported flow */
> > + return false;
> > + }
> > +
> > + /* if unsupported hashtype was set */
> > + if (new_hashtypes != (new_hashtypes & vi->rss_hash_types_supported))
> > + return false;
> > +
> > + if (new_hashtypes != vi->rss_hash_types_saved) {
> > + vi->rss_hash_types_saved = new_hashtypes;
>
> should only be updated if the commit function returned success?
Not really, we already made all checks against "supported" hash types.
Also, the commit function may not be called if RSS is disabled by the user.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-13 17:01 ` Andrew Melnichenko
@ 2022-02-13 22:09 ` Willem de Bruijn
-1 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-13 22:09 UTC (permalink / raw)
To: Andrew Melnichenko
Cc: Willem de Bruijn, Network Development, virtualization, LKML,
David S. Miller, Jakub Kicinski, Jason Wang, Michael S. Tsirkin,
Yan Vugenfirer, Yuri Benditovich
> > > @@ -3113,13 +3270,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 = 1;
> > > + 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));
> >
> > Instead of testing either feature and treating them as somewhat equal,
> > shouldn't RSS be dependent on MQ?
>
> No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
RSS depends on having multiple queues.
What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
> >
> > >
> > > /* 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 +3365,23 @@ 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));
> > > +
> > > + 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;
> >
> > Only make the feature visible when the hash is actually reported in
> > the skb, patch 3.
>
> VirtioNET has two features: RSS(steering only) and hash(hash report in
> vnet header)
> Both features may be enabled/disabled separately:
> 1. rss on and hash off - packets steered to the corresponding vqs
> 2. rss off and hash on - packets steered by tap(like mq) but headers
> have properly calculated hash.
> 3. rss on and hash on - packets steered to corresponding vqs and hash
> is present in the header.
>
> RXHASH feature allows the user to enable/disable the rss/hash(any combination).
I find that confusing, but.. I see that there is prior art where some
drivers enable/disable entire RSS load balancing based on this flag.
So ok.
> I think it's a good idea to leave RXHASH in patch 2/4 to give the user
> ability to manipulate the rss only feature.
> But, if you think that it requires to move it to the 3/4, I'll do it.
>
> >
> > Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> > rxhash config.
>
> Currently:
> Patch 2/4 - adds VirtioNet rss feature.
> Patch 3/4 - adds VirtioNet hash report feature.
> Patch 4/4 - adds the ability to manipulate supported hash types.
>
> Can you provide more detailed suggestions on how to move hunks?
I gave one in the follow-on patch, to which you responded. That's probably it.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-13 22:09 ` Willem de Bruijn
0 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-13 22:09 UTC (permalink / raw)
To: Andrew Melnichenko
Cc: Willem de Bruijn, Michael S. Tsirkin, Network Development, LKML,
virtualization, Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
> > > @@ -3113,13 +3270,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 = 1;
> > > + 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));
> >
> > Instead of testing either feature and treating them as somewhat equal,
> > shouldn't RSS be dependent on MQ?
>
> No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
RSS depends on having multiple queues.
What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
> >
> > >
> > > /* 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 +3365,23 @@ 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));
> > > +
> > > + 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;
> >
> > Only make the feature visible when the hash is actually reported in
> > the skb, patch 3.
>
> VirtioNET has two features: RSS(steering only) and hash(hash report in
> vnet header)
> Both features may be enabled/disabled separately:
> 1. rss on and hash off - packets steered to the corresponding vqs
> 2. rss off and hash on - packets steered by tap(like mq) but headers
> have properly calculated hash.
> 3. rss on and hash on - packets steered to corresponding vqs and hash
> is present in the header.
>
> RXHASH feature allows the user to enable/disable the rss/hash(any combination).
I find that confusing, but.. I see that there is prior art where some
drivers enable/disable entire RSS load balancing based on this flag.
So ok.
> I think it's a good idea to leave RXHASH in patch 2/4 to give the user
> ability to manipulate the rss only feature.
> But, if you think that it requires to move it to the 3/4, I'll do it.
>
> >
> > Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> > rxhash config.
>
> Currently:
> Patch 2/4 - adds VirtioNet rss feature.
> Patch 3/4 - adds VirtioNet hash report feature.
> Patch 4/4 - adds the ability to manipulate supported hash types.
>
> Can you provide more detailed suggestions on how to move hunks?
I gave one in the follow-on patch, to which you responded. That's probably it.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-13 22:09 ` Willem de Bruijn
@ 2022-02-17 19:02 ` Andrew Melnichenko
-1 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-17 19:02 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Network Development, virtualization, LKML, David S. Miller,
Jakub Kicinski, Jason Wang, Michael S. Tsirkin, Yan Vugenfirer,
Yuri Benditovich
Hi all,
On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> > > > @@ -3113,13 +3270,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 = 1;
> > > > + 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));
> > >
> > > Instead of testing either feature and treating them as somewhat equal,
> > > shouldn't RSS be dependent on MQ?
> >
> > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
>
> RSS depends on having multiple queues.
>
> What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
RSS would work.
According to virtio spec article 5.1.6.5.5:
> A device MAY support one of these features or both. The driver MAY negotiate any set of these features
> that the device supports.
Also, in 5.1.3.1:
> VIRTIO_NET_F_RSS Requires VIRTIO_NET_F_CTRL_VQ.
>
> > >
> > > >
> > > > /* 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 +3365,23 @@ 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));
> > > > +
> > > > + 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;
> > >
> > > Only make the feature visible when the hash is actually reported in
> > > the skb, patch 3.
> >
> > VirtioNET has two features: RSS(steering only) and hash(hash report in
> > vnet header)
> > Both features may be enabled/disabled separately:
> > 1. rss on and hash off - packets steered to the corresponding vqs
> > 2. rss off and hash on - packets steered by tap(like mq) but headers
> > have properly calculated hash.
> > 3. rss on and hash on - packets steered to corresponding vqs and hash
> > is present in the header.
> >
> > RXHASH feature allows the user to enable/disable the rss/hash(any combination).
>
> I find that confusing, but.. I see that there is prior art where some
> drivers enable/disable entire RSS load balancing based on this flag.
> So ok.
>
> > I think it's a good idea to leave RXHASH in patch 2/4 to give the user
> > ability to manipulate the rss only feature.
> > But, if you think that it requires to move it to the 3/4, I'll do it.
> >
> > >
> > > Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> > > rxhash config.
> >
> > Currently:
> > Patch 2/4 - adds VirtioNet rss feature.
> > Patch 3/4 - adds VirtioNet hash report feature.
> > Patch 4/4 - adds the ability to manipulate supported hash types.
> >
> > Can you provide more detailed suggestions on how to move hunks?
>
> I gave one in the follow-on patch, to which you responded. That's probably it.
I'll add zero size table check and move hunk for padded header length
from 3/4 to 1/4.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-17 19:02 ` Andrew Melnichenko
0 siblings, 0 replies; 31+ messages in thread
From: Andrew Melnichenko @ 2022-02-17 19:02 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Network Development, LKML, virtualization,
Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
Hi all,
On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> > > > @@ -3113,13 +3270,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 = 1;
> > > > + 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));
> > >
> > > Instead of testing either feature and treating them as somewhat equal,
> > > shouldn't RSS be dependent on MQ?
> >
> > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
>
> RSS depends on having multiple queues.
>
> What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
RSS would work.
According to virtio spec article 5.1.6.5.5:
> A device MAY support one of these features or both. The driver MAY negotiate any set of these features
> that the device supports.
Also, in 5.1.3.1:
> VIRTIO_NET_F_RSS Requires VIRTIO_NET_F_CTRL_VQ.
>
> > >
> > > >
> > > > /* 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 +3365,23 @@ 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));
> > > > +
> > > > + 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;
> > >
> > > Only make the feature visible when the hash is actually reported in
> > > the skb, patch 3.
> >
> > VirtioNET has two features: RSS(steering only) and hash(hash report in
> > vnet header)
> > Both features may be enabled/disabled separately:
> > 1. rss on and hash off - packets steered to the corresponding vqs
> > 2. rss off and hash on - packets steered by tap(like mq) but headers
> > have properly calculated hash.
> > 3. rss on and hash on - packets steered to corresponding vqs and hash
> > is present in the header.
> >
> > RXHASH feature allows the user to enable/disable the rss/hash(any combination).
>
> I find that confusing, but.. I see that there is prior art where some
> drivers enable/disable entire RSS load balancing based on this flag.
> So ok.
>
> > I think it's a good idea to leave RXHASH in patch 2/4 to give the user
> > ability to manipulate the rss only feature.
> > But, if you think that it requires to move it to the 3/4, I'll do it.
> >
> > >
> > > Also, clearly separate the feature patches (2) rss, (3) rxhash, (4)
> > > rxhash config.
> >
> > Currently:
> > Patch 2/4 - adds VirtioNet rss feature.
> > Patch 3/4 - adds VirtioNet hash report feature.
> > Patch 4/4 - adds the ability to manipulate supported hash types.
> >
> > Can you provide more detailed suggestions on how to move hunks?
>
> I gave one in the follow-on patch, to which you responded. That's probably it.
I'll add zero size table check and move hunk for padded header length
from 3/4 to 1/4.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-17 19:02 ` Andrew Melnichenko
@ 2022-02-18 15:43 ` Willem de Bruijn
-1 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-18 15:43 UTC (permalink / raw)
To: Andrew Melnichenko
Cc: Willem de Bruijn, Network Development, virtualization, LKML,
David S. Miller, Jakub Kicinski, Jason Wang, Michael S. Tsirkin,
Yan Vugenfirer, Yuri Benditovich
On Thu, Feb 17, 2022 at 12:05 PM Andrew Melnichenko <andrew@daynix.com> wrote:
>
> Hi all,
>
> On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > > > > @@ -3113,13 +3270,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 = 1;
> > > > > + 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));
> > > >
> > > > Instead of testing either feature and treating them as somewhat equal,
> > > > shouldn't RSS be dependent on MQ?
> > >
> > > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
> >
> > RSS depends on having multiple queues.
> >
> > What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
>
> RSS would work.
What does that mean, exactly? RSS is load balancing, does that not
require multi-queue?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-18 15:43 ` Willem de Bruijn
0 siblings, 0 replies; 31+ messages in thread
From: Willem de Bruijn @ 2022-02-18 15:43 UTC (permalink / raw)
To: Andrew Melnichenko
Cc: Willem de Bruijn, Michael S. Tsirkin, Network Development, LKML,
virtualization, Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
On Thu, Feb 17, 2022 at 12:05 PM Andrew Melnichenko <andrew@daynix.com> wrote:
>
> Hi all,
>
> On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > > > > @@ -3113,13 +3270,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 = 1;
> > > > > + 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));
> > > >
> > > > Instead of testing either feature and treating them as somewhat equal,
> > > > shouldn't RSS be dependent on MQ?
> > >
> > > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
> >
> > RSS depends on having multiple queues.
> >
> > What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
>
> RSS would work.
What does that mean, exactly? RSS is load balancing, does that not
require multi-queue?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
2022-02-18 15:43 ` Willem de Bruijn
@ 2022-02-20 13:17 ` Michael S. Tsirkin
-1 siblings, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-02-20 13:17 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Andrew Melnichenko, Network Development, virtualization, LKML,
David S. Miller, Jakub Kicinski, Jason Wang, Yan Vugenfirer,
Yuri Benditovich
On Fri, Feb 18, 2022 at 08:43:20AM -0700, Willem de Bruijn wrote:
> On Thu, Feb 17, 2022 at 12:05 PM Andrew Melnichenko <andrew@daynix.com> wrote:
> >
> > Hi all,
> >
> > On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > > > > @@ -3113,13 +3270,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 = 1;
> > > > > > + 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));
> > > > >
> > > > > Instead of testing either feature and treating them as somewhat equal,
> > > > > shouldn't RSS be dependent on MQ?
> > > >
> > > > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
> > >
> > > RSS depends on having multiple queues.
> > >
> > > What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
> >
> > RSS would work.
>
> What does that mean, exactly? RSS is load balancing, does that not
> require multi-queue?
It does, but VIRTIO_NET_F_MQ is a misnomer.
\item[VIRTIO_NET_F_MQ(22)] Device supports multiqueue with automatic
receive steering.
VIRTIO_NET_F_RSS implies multi queue and does not depend on VIRTIO_NET_F_MQ.
--
MST
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support.
@ 2022-02-20 13:17 ` Michael S. Tsirkin
0 siblings, 0 replies; 31+ messages in thread
From: Michael S. Tsirkin @ 2022-02-20 13:17 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Andrew Melnichenko, Network Development, LKML, virtualization,
Yuri Benditovich, Yan Vugenfirer, Jakub Kicinski,
David S. Miller
On Fri, Feb 18, 2022 at 08:43:20AM -0700, Willem de Bruijn wrote:
> On Thu, Feb 17, 2022 at 12:05 PM Andrew Melnichenko <andrew@daynix.com> wrote:
> >
> > Hi all,
> >
> > On Mon, Feb 14, 2022 at 12:09 AM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > > > > @@ -3113,13 +3270,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 = 1;
> > > > > > + 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));
> > > > >
> > > > > Instead of testing either feature and treating them as somewhat equal,
> > > > > shouldn't RSS be dependent on MQ?
> > > >
> > > > No, RSS is dependent on CTRL_VQ. Technically RSS and MQ are similar features.
> > >
> > > RSS depends on having multiple queues.
> > >
> > > What would enabling VIRTIO_NET_F_RSS without VIRTIO_NET_F_MQ do?
> >
> > RSS would work.
>
> What does that mean, exactly? RSS is load balancing, does that not
> require multi-queue?
It does, but VIRTIO_NET_F_MQ is a misnomer.
\item[VIRTIO_NET_F_MQ(22)] Device supports multiqueue with automatic
receive steering.
VIRTIO_NET_F_RSS implies multi queue and does not depend on VIRTIO_NET_F_MQ.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2022-02-20 13:17 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 18:15 [PATCH v3 0/4] RSS support for VirtioNet Andrew Melnychenko
2022-02-08 18:15 ` Andrew Melnychenko
2022-02-08 18:15 ` [PATCH v3 1/4] drivers/net/virtio_net: Fixed padded vheader to use v1 with hash Andrew Melnychenko
2022-02-08 18:15 ` Andrew Melnychenko
2022-02-08 18:15 ` [PATCH v3 2/4] drivers/net/virtio_net: Added basic RSS support Andrew Melnychenko
2022-02-08 18:15 ` Andrew Melnychenko
2022-02-08 20:36 ` Willem de Bruijn
2022-02-08 20:36 ` Willem de Bruijn
2022-02-13 17:01 ` Andrew Melnichenko
2022-02-13 17:01 ` Andrew Melnichenko
2022-02-13 22:09 ` Willem de Bruijn
2022-02-13 22:09 ` Willem de Bruijn
2022-02-17 19:02 ` Andrew Melnichenko
2022-02-17 19:02 ` Andrew Melnichenko
2022-02-18 15:43 ` Willem de Bruijn
2022-02-18 15:43 ` Willem de Bruijn
2022-02-20 13:17 ` Michael S. Tsirkin
2022-02-20 13:17 ` Michael S. Tsirkin
2022-02-08 18:15 ` [PATCH v3 3/4] drivers/net/virtio_net: Added RSS hash report Andrew Melnychenko
2022-02-08 18:15 ` Andrew Melnychenko
2022-02-08 20:54 ` Willem de Bruijn
2022-02-08 20:54 ` Willem de Bruijn
2022-02-13 17:08 ` Andrew Melnichenko
2022-02-13 17:08 ` Andrew Melnichenko
2022-02-09 15:09 ` kernel test robot
2022-02-08 18:15 ` [PATCH v3 4/4] drivers/net/virtio_net: Added RSS hash report control Andrew Melnychenko
2022-02-08 18:15 ` Andrew Melnychenko
2022-02-08 20:58 ` Willem de Bruijn
2022-02-08 20:58 ` Willem de Bruijn
2022-02-13 17:22 ` Andrew Melnichenko
2022-02-13 17:22 ` 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.