* [RFC PATCH 0/4] Added RSS support.
@ 2021-10-31 4:59 ` Andrew Melnychenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: yan, netdev, yuri.benditovich, linux-kernel, virtualization
This series of RFC patches for comments and additional proposals.
Virtio-net supports "hardware" RSS with toeplitz key.
Also, it allows receiving calculated hash in vheader
that may be used with RPS.
Added ethtools callbacks to manipulate RSS.
Technically hash calculation may be set only for
SRC+DST and SRC+DST+PORTSRC+PORTDST hashflows.
The completely disabling hash calculation for TCP or UDP
would disable hash calculation for IP.
RSS/RXHASH is disabled by default.
Changes since rfc:
* code refactored
* patches reformatted
* added feature validation
Andrew Melnychenko (4):
drivers/net/virtio_net: Fixed vheader to use v1.
drivers/net/virtio_net: Changed mergeable buffer length calculation.
drivers/net/virtio_net: Added basic RSS support.
drivers/net/virtio_net: Added RSS hash report control.
drivers/net/virtio_net.c | 405 +++++++++++++++++++++++++++++++++++++--
1 file changed, 390 insertions(+), 15 deletions(-)
--
2.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFC PATCH 0/4] Added RSS support.
@ 2021-10-31 4:59 ` Andrew Melnychenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: virtualization, netdev, linux-kernel, yuri.benditovich, yan
This series of RFC patches for comments and additional proposals.
Virtio-net supports "hardware" RSS with toeplitz key.
Also, it allows receiving calculated hash in vheader
that may be used with RPS.
Added ethtools callbacks to manipulate RSS.
Technically hash calculation may be set only for
SRC+DST and SRC+DST+PORTSRC+PORTDST hashflows.
The completely disabling hash calculation for TCP or UDP
would disable hash calculation for IP.
RSS/RXHASH is disabled by default.
Changes since rfc:
* code refactored
* patches reformatted
* added feature validation
Andrew Melnychenko (4):
drivers/net/virtio_net: Fixed vheader to use v1.
drivers/net/virtio_net: Changed mergeable buffer length calculation.
drivers/net/virtio_net: Added basic RSS support.
drivers/net/virtio_net: Added RSS hash report control.
drivers/net/virtio_net.c | 405 +++++++++++++++++++++++++++++++++++++--
1 file changed, 390 insertions(+), 15 deletions(-)
--
2.33.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-10-31 4:59 ` Andrew Melnychenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: yan, netdev, yuri.benditovich, linux-kernel, virtualization
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4ad25a8b0870..b72b21ac8ebd 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -240,13 +240,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)
@@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
struct virtnet_info *vi = sq->vq->vdev->priv;
int num_sg;
- unsigned hdr_len = vi->hdr_len;
+ unsigned int hdr_len = vi->hdr_len;
bool can_push;
pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
--
2.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
@ 2021-10-31 4:59 ` Andrew Melnychenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: virtualization, netdev, linux-kernel, yuri.benditovich, yan
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4ad25a8b0870..b72b21ac8ebd 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -240,13 +240,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)
@@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
struct virtnet_info *vi = sq->vq->vdev->priv;
int num_sg;
- unsigned hdr_len = vi->hdr_len;
+ unsigned int hdr_len = vi->hdr_len;
bool can_push;
pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
--
2.33.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-10-31 4:59 ` Andrew Melnychenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: yan, netdev, yuri.benditovich, linux-kernel, virtualization
Now minimal virtual header length is may include the entire v1 header
if the hash report were populated.
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
drivers/net/virtio_net.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b72b21ac8ebd..abca2e93355d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_p = p;
hdr_len = vi->hdr_len;
- if (vi->mergeable_rx_bufs)
+ if (vi->has_rss_hash_report)
+ hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
+ else if (vi->mergeable_rx_bufs)
hdr_padded_len = sizeof(*hdr);
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1252,7 +1254,7 @@ 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);
+ const size_t hdr_len = ((struct virtnet_info *)(rq->vq->vdev->priv))->hdr_len;
unsigned int len;
if (room)
@@ -2817,7 +2819,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.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
@ 2021-10-31 4:59 ` Andrew Melnychenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: virtualization, netdev, linux-kernel, yuri.benditovich, yan
Now minimal virtual header length is may include the entire v1 header
if the hash report were populated.
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
drivers/net/virtio_net.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b72b21ac8ebd..abca2e93355d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
hdr_p = p;
hdr_len = vi->hdr_len;
- if (vi->mergeable_rx_bufs)
+ if (vi->has_rss_hash_report)
+ hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
+ else if (vi->mergeable_rx_bufs)
hdr_padded_len = sizeof(*hdr);
else
hdr_padded_len = sizeof(struct padded_vnet_hdr);
@@ -1252,7 +1254,7 @@ 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);
+ const size_t hdr_len = ((struct virtnet_info *)(rq->vq->vdev->priv))->hdr_len;
unsigned int len;
if (room)
@@ -2817,7 +2819,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.33.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-10-31 4:59 ` Andrew Melnychenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: yan, netdev, yuri.benditovich, linux-kernel, virtualization
Added features for RSS and RSS hash report.
Added initialization, RXHASH feature and ethtool ops.
By default RSS/RXHASH is disabled.
Added ethtools ops to set key and indirection table.
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
drivers/net/virtio_net.c | 232 +++++++++++++++++++++++++++++++++++++--
1 file changed, 223 insertions(+), 9 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index abca2e93355d..cff7340f40bb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -167,6 +167,28 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
};
+/* This structure can contain rss message with maximum settings for indirection table and keysize
+ * Note, that default structure that describes RSS configuration virtio_net_rss_config
+ * contains same info but can't handle table values.
+ * In any case, structure would be passed to virtio hw through sg_buf split by parts
+ * because table sizes may be differ according to the device configuration.
+ */
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
+struct virtio_net_ctrl_rss {
+ struct {
+ __le32 hash_types;
+ __le16 indirection_table_mask;
+ __le16 unclassified_queue;
+ } __packed table_info;
+ u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+ struct {
+ u16 max_tx_vq; /* queues */
+ u8 hash_key_length;
+ } __packed key_info;
+ u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+};
+
/* Control VQ buffers: protected by the rtnl lock */
struct control_buf {
struct virtio_net_ctrl_hdr hdr;
@@ -176,6 +198,7 @@ struct control_buf {
u8 allmulti;
__virtio16 vid;
__virtio64 offloads;
+ struct virtio_net_ctrl_rss rss;
};
struct virtnet_info {
@@ -204,6 +227,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;
+ bool has_rss_hash_report;
+ u8 rss_key_size;
+ u16 rss_indir_table_size;
+
/* Has control virtqueue */
bool has_cvq;
@@ -1119,6 +1148,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);
@@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
return;
hdr = skb_vnet_hdr(skb);
+ if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
+ hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
+
+ switch (hdr_hash->hash_report) {
+ case VIRTIO_NET_HASH_REPORT_TCPv4:
+ case VIRTIO_NET_HASH_REPORT_UDPv4:
+ case VIRTIO_NET_HASH_REPORT_TCPv6:
+ case VIRTIO_NET_HASH_REPORT_UDPv6:
+ case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
+ case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
+ rss_hash_type = PKT_HASH_TYPE_L4;
+ break;
+ case VIRTIO_NET_HASH_REPORT_IPv4:
+ case VIRTIO_NET_HASH_REPORT_IPv6:
+ case VIRTIO_NET_HASH_REPORT_IPv6_EX:
+ rss_hash_type = PKT_HASH_TYPE_L3;
+ break;
+ case VIRTIO_NET_HASH_REPORT_NONE:
+ default:
+ rss_hash_type = PKT_HASH_TYPE_NONE;
+ }
+ skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
+ }
if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -2167,6 +2221,57 @@ static void virtnet_get_ringparam(struct net_device *dev,
ring->tx_pending = ring->tx_max_pending;
}
+static bool virtnet_commit_rss_command(struct virtnet_info *vi)
+{
+ struct net_device *dev = vi->dev;
+ struct scatterlist sgs[4];
+ unsigned int sg_buf_size;
+
+ /* prepare sgs */
+ sg_init_table(sgs, 4);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.table_info);
+ sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
+
+ sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+ sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.key_info);
+ sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
+
+ sg_buf_size = vi->rss_key_size;
+ sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
+
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
+ 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;
+ }
+ return true;
+}
+
+static void virtnet_init_default_rss(struct virtnet_info *vi)
+{
+ u32 indir_val = 0;
+ int i = 0;
+
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+ vi->rss_hash_types_saved = vi->rss_hash_types_supported;
+ vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
+ vi->ctrl->rss.table_info.unclassified_queue = 0;
+
+ for (; i < vi->rss_indir_table_size; ++i) {
+ indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
+ vi->ctrl->rss.indirection_table[i] = indir_val;
+ }
+
+ vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
+ vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
+
+ netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
+}
+
static void virtnet_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
@@ -2395,6 +2500,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,
@@ -2410,6 +2580,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)
@@ -3040,7 +3215,10 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
"VIRTIO_NET_F_CTRL_VQ") ||
VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
- "VIRTIO_NET_F_CTRL_VQ"))) {
+ "VIRTIO_NET_F_CTRL_VQ") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
+ "VIRTIO_NET_F_HASH_REPORT"))) {
return false;
}
@@ -3080,13 +3258,14 @@ static int virtnet_probe(struct virtio_device *vdev)
u16 max_queue_pairs;
int mtu;
- /* Find if host supports multiqueue virtio_net device */
- err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
- struct virtio_net_config,
- max_virtqueue_pairs, &max_queue_pairs);
+ /* Find if host supports multiqueue/rss virtio_net device */
+ max_queue_pairs = 0;
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
+ max_queue_pairs =
+ virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
/* We need at least 2 queue's */
- if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
+ if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
max_queue_pairs = 1;
@@ -3170,8 +3349,36 @@ 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_MRG_RXBUF) ||
- virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
+ vi->has_rss_hash_report = true;
+ vi->rss_indir_table_size = 1;
+ vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
+ }
+
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
+ vi->has_rss = true;
+ vi->rss_indir_table_size =
+ virtio_cread16(vdev, offsetof(struct virtio_net_config,
+ rss_max_indirection_table_length));
+ vi->rss_key_size =
+ virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
+ }
+
+ if (vi->has_rss || vi->has_rss_hash_report) {
+ vi->rss_hash_types_supported =
+ virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
+ vi->rss_hash_types_supported &=
+ ~(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 (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);
@@ -3238,6 +3445,12 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
+ if (vi->has_rss || vi->has_rss_hash_report) {
+ rtnl_lock();
+ virtnet_init_default_rss(vi);
+ rtnl_unlock();
+ }
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
@@ -3369,7 +3582,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, VIRTIO_NET_F_HASH_REPORT
static unsigned int features[] = {
VIRTNET_FEATURES,
--
2.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
@ 2021-10-31 4:59 ` Andrew Melnychenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: virtualization, netdev, linux-kernel, yuri.benditovich, yan
Added features for RSS and RSS hash report.
Added initialization, RXHASH feature and ethtool ops.
By default RSS/RXHASH is disabled.
Added ethtools ops to set key and indirection table.
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
drivers/net/virtio_net.c | 232 +++++++++++++++++++++++++++++++++++++--
1 file changed, 223 insertions(+), 9 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index abca2e93355d..cff7340f40bb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -167,6 +167,28 @@ struct receive_queue {
struct xdp_rxq_info xdp_rxq;
};
+/* This structure can contain rss message with maximum settings for indirection table and keysize
+ * Note, that default structure that describes RSS configuration virtio_net_rss_config
+ * contains same info but can't handle table values.
+ * In any case, structure would be passed to virtio hw through sg_buf split by parts
+ * because table sizes may be differ according to the device configuration.
+ */
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
+#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
+struct virtio_net_ctrl_rss {
+ struct {
+ __le32 hash_types;
+ __le16 indirection_table_mask;
+ __le16 unclassified_queue;
+ } __packed table_info;
+ u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
+ struct {
+ u16 max_tx_vq; /* queues */
+ u8 hash_key_length;
+ } __packed key_info;
+ u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+};
+
/* Control VQ buffers: protected by the rtnl lock */
struct control_buf {
struct virtio_net_ctrl_hdr hdr;
@@ -176,6 +198,7 @@ struct control_buf {
u8 allmulti;
__virtio16 vid;
__virtio64 offloads;
+ struct virtio_net_ctrl_rss rss;
};
struct virtnet_info {
@@ -204,6 +227,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;
+ bool has_rss_hash_report;
+ u8 rss_key_size;
+ u16 rss_indir_table_size;
+
/* Has control virtqueue */
bool has_cvq;
@@ -1119,6 +1148,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);
@@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
return;
hdr = skb_vnet_hdr(skb);
+ if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
+ hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
+
+ switch (hdr_hash->hash_report) {
+ case VIRTIO_NET_HASH_REPORT_TCPv4:
+ case VIRTIO_NET_HASH_REPORT_UDPv4:
+ case VIRTIO_NET_HASH_REPORT_TCPv6:
+ case VIRTIO_NET_HASH_REPORT_UDPv6:
+ case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
+ case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
+ rss_hash_type = PKT_HASH_TYPE_L4;
+ break;
+ case VIRTIO_NET_HASH_REPORT_IPv4:
+ case VIRTIO_NET_HASH_REPORT_IPv6:
+ case VIRTIO_NET_HASH_REPORT_IPv6_EX:
+ rss_hash_type = PKT_HASH_TYPE_L3;
+ break;
+ case VIRTIO_NET_HASH_REPORT_NONE:
+ default:
+ rss_hash_type = PKT_HASH_TYPE_NONE;
+ }
+ skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
+ }
if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -2167,6 +2221,57 @@ static void virtnet_get_ringparam(struct net_device *dev,
ring->tx_pending = ring->tx_max_pending;
}
+static bool virtnet_commit_rss_command(struct virtnet_info *vi)
+{
+ struct net_device *dev = vi->dev;
+ struct scatterlist sgs[4];
+ unsigned int sg_buf_size;
+
+ /* prepare sgs */
+ sg_init_table(sgs, 4);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.table_info);
+ sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
+
+ sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
+ sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
+
+ sg_buf_size = sizeof(vi->ctrl->rss.key_info);
+ sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
+
+ sg_buf_size = vi->rss_key_size;
+ sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
+
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
+ 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;
+ }
+ return true;
+}
+
+static void virtnet_init_default_rss(struct virtnet_info *vi)
+{
+ u32 indir_val = 0;
+ int i = 0;
+
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
+ vi->rss_hash_types_saved = vi->rss_hash_types_supported;
+ vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
+ vi->ctrl->rss.table_info.unclassified_queue = 0;
+
+ for (; i < vi->rss_indir_table_size; ++i) {
+ indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
+ vi->ctrl->rss.indirection_table[i] = indir_val;
+ }
+
+ vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
+ vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
+
+ netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
+}
+
static void virtnet_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
@@ -2395,6 +2500,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,
@@ -2410,6 +2580,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)
@@ -3040,7 +3215,10 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
"VIRTIO_NET_F_CTRL_VQ") ||
VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
- "VIRTIO_NET_F_CTRL_VQ"))) {
+ "VIRTIO_NET_F_CTRL_VQ") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
+ VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
+ "VIRTIO_NET_F_HASH_REPORT"))) {
return false;
}
@@ -3080,13 +3258,14 @@ static int virtnet_probe(struct virtio_device *vdev)
u16 max_queue_pairs;
int mtu;
- /* Find if host supports multiqueue virtio_net device */
- err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
- struct virtio_net_config,
- max_virtqueue_pairs, &max_queue_pairs);
+ /* Find if host supports multiqueue/rss virtio_net device */
+ max_queue_pairs = 0;
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
+ max_queue_pairs =
+ virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
/* We need at least 2 queue's */
- if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
+ if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
max_queue_pairs = 1;
@@ -3170,8 +3349,36 @@ 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_MRG_RXBUF) ||
- virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
+ vi->has_rss_hash_report = true;
+ vi->rss_indir_table_size = 1;
+ vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
+ }
+
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
+ vi->has_rss = true;
+ vi->rss_indir_table_size =
+ virtio_cread16(vdev, offsetof(struct virtio_net_config,
+ rss_max_indirection_table_length));
+ vi->rss_key_size =
+ virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
+ }
+
+ if (vi->has_rss || vi->has_rss_hash_report) {
+ vi->rss_hash_types_supported =
+ virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
+ vi->rss_hash_types_supported &=
+ ~(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 (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);
@@ -3238,6 +3445,12 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
+ if (vi->has_rss || vi->has_rss_hash_report) {
+ rtnl_lock();
+ virtnet_init_default_rss(vi);
+ rtnl_unlock();
+ }
+
err = register_netdev(dev);
if (err) {
pr_debug("virtio_net: registering device failed\n");
@@ -3369,7 +3582,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, VIRTIO_NET_F_HASH_REPORT
static unsigned int features[] = {
VIRTNET_FEATURES,
--
2.33.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-10-31 4:59 ` Andrew Melnychenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: yan, netdev, yuri.benditovich, linux-kernel, virtualization
Added set_hash for skb.
Also added hashflow set/get callbacks.
Virtio RSS "IPv6 extensions" hashes disabled.
Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
TCP and UDP supports only:
ethtool -U eth0 rx-flow-hash tcp4 sd
RXH_IP_SRC + RXH_IP_DST
ethtool -U eth0 rx-flow-hash tcp4 sdfn
RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
1 file changed, 159 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cff7340f40bb..b1ed373d942b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -232,6 +232,8 @@ struct virtnet_info {
bool has_rss_hash_report;
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 +2274,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
}
+static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+ info->data = 0;
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case TCP_V6_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case UDP_V4_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case UDP_V6_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case IPV4_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+
+ break;
+ case IPV6_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+
+ break;
+ default:
+ info->data = 0;
+ break;
+ }
+}
+
+static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+ u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
+ u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
+ u32 new_hashtypes = vi->rss_hash_types_saved;
+
+ if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
+ (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
+ return false;
+ }
+
+ if (!is_iphash && is_porthash)
+ return false;
+
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ case UDP_V4_FLOW:
+ case IPV4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+ if (is_iphash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+
+ break;
+ case TCP_V6_FLOW:
+ case UDP_V6_FLOW:
+ case IPV6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+ if (is_iphash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+
+ break;
+ default:
+ break;
+ }
+
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+
+ break;
+ case UDP_V4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+
+ break;
+ case TCP_V6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+
+ break;
+ case UDP_V6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+
+ break;
+ default:
+ break;
+ }
+
+ if (new_hashtypes != vi->rss_hash_types_saved) {
+ vi->rss_hash_types_saved = new_hashtypes;
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+ if (vi->dev->features & NETIF_F_RXHASH)
+ return virtnet_commit_rss_command(vi);
+ }
+
+ return true;
+}
static void virtnet_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
@@ -2557,6 +2684,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;
@@ -2585,6 +2733,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)
@@ -2837,6 +2986,16 @@ static int virtnet_set_features(struct net_device *dev,
vi->guest_offloads = offloads;
}
+ if ((dev->features ^ features) & NETIF_F_RXHASH) {
+ if (features & NETIF_F_RXHASH)
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+ else
+ vi->ctrl->rss.table_info.hash_types = 0;
+
+ if (!virtnet_commit_rss_command(vi))
+ return -EINVAL;
+ }
+
return 0;
}
--
2.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [RFC PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
@ 2021-10-31 4:59 ` Andrew Melnychenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnychenko @ 2021-10-31 4:59 UTC (permalink / raw)
To: mst, jasowang, davem, kuba
Cc: virtualization, netdev, linux-kernel, yuri.benditovich, yan
Added set_hash for skb.
Also added hashflow set/get callbacks.
Virtio RSS "IPv6 extensions" hashes disabled.
Also, disabling RXH_IP_SRC/DST for TCP would disable then for UDP.
TCP and UDP supports only:
ethtool -U eth0 rx-flow-hash tcp4 sd
RXH_IP_SRC + RXH_IP_DST
ethtool -U eth0 rx-flow-hash tcp4 sdfn
RXH_IP_SRC + RXH_IP_DST + RXH_L4_B_0_1 + RXH_L4_B_2_3
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
---
drivers/net/virtio_net.c | 159 +++++++++++++++++++++++++++++++++++++++
1 file changed, 159 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index cff7340f40bb..b1ed373d942b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -232,6 +232,8 @@ struct virtnet_info {
bool has_rss_hash_report;
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 +2274,131 @@ static void virtnet_init_default_rss(struct virtnet_info *vi)
netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
}
+static void virtnet_get_hashflow(const struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+ info->data = 0;
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case TCP_V6_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_TCPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case UDP_V4_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case UDP_V6_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_UDPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST |
+ RXH_L4_B_0_1 | RXH_L4_B_2_3;
+ } else if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv6) {
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+ }
+ break;
+ case IPV4_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+
+ break;
+ case IPV6_FLOW:
+ if (vi->rss_hash_types_saved & VIRTIO_NET_RSS_HASH_TYPE_IPv4)
+ info->data = RXH_IP_SRC | RXH_IP_DST;
+
+ break;
+ default:
+ info->data = 0;
+ break;
+ }
+}
+
+static bool virtnet_set_hashflow(struct virtnet_info *vi, struct ethtool_rxnfc *info)
+{
+ u64 is_iphash = info->data & (RXH_IP_SRC | RXH_IP_DST);
+ u64 is_porthash = info->data & (RXH_L4_B_0_1 | RXH_L4_B_2_3);
+ u32 new_hashtypes = vi->rss_hash_types_saved;
+
+ if ((is_iphash && (is_iphash != (RXH_IP_SRC | RXH_IP_DST))) ||
+ (is_porthash && (is_porthash != (RXH_L4_B_0_1 | RXH_L4_B_2_3)))) {
+ return false;
+ }
+
+ if (!is_iphash && is_porthash)
+ return false;
+
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ case UDP_V4_FLOW:
+ case IPV4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+ if (is_iphash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv4;
+
+ break;
+ case TCP_V6_FLOW:
+ case UDP_V6_FLOW:
+ case IPV6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+ if (is_iphash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_IPv6;
+
+ break;
+ default:
+ break;
+ }
+
+ switch (info->flow_type) {
+ case TCP_V4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv4;
+
+ break;
+ case UDP_V4_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv4;
+
+ break;
+ case TCP_V6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_TCPv6;
+
+ break;
+ case UDP_V6_FLOW:
+ new_hashtypes &= ~VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+ if (is_porthash)
+ new_hashtypes |= VIRTIO_NET_RSS_HASH_TYPE_UDPv6;
+
+ break;
+ default:
+ break;
+ }
+
+ if (new_hashtypes != vi->rss_hash_types_saved) {
+ vi->rss_hash_types_saved = new_hashtypes;
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+ if (vi->dev->features & NETIF_F_RXHASH)
+ return virtnet_commit_rss_command(vi);
+ }
+
+ return true;
+}
static void virtnet_get_drvinfo(struct net_device *dev,
struct ethtool_drvinfo *info)
@@ -2557,6 +2684,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;
@@ -2585,6 +2733,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)
@@ -2837,6 +2986,16 @@ static int virtnet_set_features(struct net_device *dev,
vi->guest_offloads = offloads;
}
+ if ((dev->features ^ features) & NETIF_F_RXHASH) {
+ if (features & NETIF_F_RXHASH)
+ vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_saved;
+ else
+ vi->ctrl->rss.table_info.hash_types = 0;
+
+ if (!virtnet_commit_rss_command(vi))
+ return -EINVAL;
+ }
+
return 0;
}
--
2.33.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
2021-10-31 4:59 ` Andrew Melnychenko
(?)
@ 2021-10-31 15:30 ` kernel test robot
-1 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2021-10-31 15:30 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4008 bytes --]
Hi Andrew,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on mst-vhost/linux-next]
[also build test ERROR on net-next/master net/master linus/master v5.15-rc7 next-20211029]
[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/Added-RSS-support/20211031-130048
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/8bcc65dfaae7855d3748ce0a1b03466a8048540f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Melnychenko/Added-RSS-support/20211031-130048
git checkout 8bcc65dfaae7855d3748ce0a1b03466a8048540f
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Note: the linux-review/Andrew-Melnychenko/Added-RSS-support/20211031-130048 HEAD 385fe5b07bf0ded4667d57d48eacf27e3d4e3733 builds fine.
It only hurts bisectability.
All errors (new ones prefixed by >>):
drivers/net/virtio_net.c: In function 'virtnet_init_default_rss':
>> drivers/net/virtio_net.c:2260:49: error: 'struct virtnet_info' has no member named 'rss_hash_types_supported'
2260 | vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
| ^~
>> drivers/net/virtio_net.c:2261:11: error: 'struct virtnet_info' has no member named 'rss_hash_types_saved'
2261 | vi->rss_hash_types_saved = vi->rss_hash_types_supported;
| ^~
drivers/net/virtio_net.c:2261:38: error: 'struct virtnet_info' has no member named 'rss_hash_types_supported'
2261 | vi->rss_hash_types_saved = vi->rss_hash_types_supported;
| ^~
drivers/net/virtio_net.c: In function 'virtnet_probe':
drivers/net/virtio_net.c:3369:19: error: 'struct virtnet_info' has no member named 'rss_hash_types_supported'
3369 | vi->rss_hash_types_supported =
| ^~
drivers/net/virtio_net.c:3371:19: error: 'struct virtnet_info' has no member named 'rss_hash_types_supported'
3371 | vi->rss_hash_types_supported &=
| ^~
vim +2260 drivers/net/virtio_net.c
2254
2255 static void virtnet_init_default_rss(struct virtnet_info *vi)
2256 {
2257 u32 indir_val = 0;
2258 int i = 0;
2259
> 2260 vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
> 2261 vi->rss_hash_types_saved = vi->rss_hash_types_supported;
2262 vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
2263 vi->ctrl->rss.table_info.unclassified_queue = 0;
2264
2265 for (; i < vi->rss_indir_table_size; ++i) {
2266 indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
2267 vi->ctrl->rss.indirection_table[i] = indir_val;
2268 }
2269
2270 vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
2271 vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
2272
2273 netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
2274 }
2275
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 61048 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-10-31 15:32 ` Willem de Bruijn
-1 siblings, 0 replies; 27+ messages in thread
From: Willem de Bruijn @ 2021-10-31 15:32 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: mst, jasowang, davem, kuba, virtualization, netdev, linux-kernel,
yuri.benditovich, yan
On Sun, Oct 31, 2021 at 1:00 AM Andrew Melnychenko <andrew@daynix.com> wrote:
>
> Added features for RSS and RSS hash report.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Added ethtools ops to set key and indirection table.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
> drivers/net/virtio_net.c | 232 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 223 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index abca2e93355d..cff7340f40bb 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -167,6 +167,28 @@ struct receive_queue {
> struct xdp_rxq_info xdp_rxq;
> };
>
> +/* This structure can contain rss message with maximum settings for indirection table and keysize
> + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
Unless there is a technical reason, this probably should be no shorter
than TOEPLITZ_KEY_LEN
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> +struct virtio_net_ctrl_rss {
> + struct {
> + __le32 hash_types;
> + __le16 indirection_table_mask;
> + __le16 unclassified_queue;
Is this explicit variable needed?
> + } __packed table_info;
> + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> + struct {
> + u16 max_tx_vq; /* queues */
> + u8 hash_key_length;
> + } __packed key_info;
> + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};
> +
> /* Control VQ buffers: protected by the rtnl lock */
> struct control_buf {
> struct virtio_net_ctrl_hdr hdr;
> @@ -176,6 +198,7 @@ struct control_buf {
> u8 allmulti;
> __virtio16 vid;
> __virtio64 offloads;
> + struct virtio_net_ctrl_rss rss;
> };
>
> struct virtnet_info {
> @@ -204,6 +227,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;
Superfluous, can be derived form non-zero rss_key_size?
> + bool has_rss_hash_report;
> + u8 rss_key_size;
> + u16 rss_indir_table_size;
> +
> /* Has control virtqueue */
> bool has_cvq;
>
> @@ -1119,6 +1148,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);
> @@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> return;
>
> hdr = skb_vnet_hdr(skb);
> + if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
Only the second test is needed? It should be impossible to configure
the feature unless the device advertises 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;
> + }
Is this detailed protocol typing necessary? Most devices only pass a bit is_l4.
> + 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;
> @@ -2167,6 +2221,57 @@ static void virtnet_get_ringparam(struct net_device *dev,
> ring->tx_pending = ring->tx_max_pending;
> }
>
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> + struct net_device *dev = vi->dev;
> + struct scatterlist sgs[4];
> + unsigned int sg_buf_size;
> +
> + /* prepare sgs */
> + sg_init_table(sgs, 4);
> +
> + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> + sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> +
> + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> + sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> + sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> +
> + sg_buf_size = vi->rss_key_size;
> + sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> +
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> + 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;
> + }
> + return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> + u32 indir_val = 0;
> + int i = 0;
> +
> + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
Similar to above, and related to the next patch: is this very detailed
specification of supported hash types needed? When is this useful? It
is not customary to specify RSS to that degree.
> + vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> + vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> + vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> + for (; i < vi->rss_indir_table_size; ++i) {
> + indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> + vi->ctrl->rss.indirection_table[i] = indir_val;
> + }
> +
> + vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> + vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> +
> + netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> +}
> +
>
> static void virtnet_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> @@ -2395,6 +2500,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,
> @@ -2410,6 +2580,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)
> @@ -3040,7 +3215,10 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> "VIRTIO_NET_F_CTRL_VQ") ||
> VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> - "VIRTIO_NET_F_CTRL_VQ"))) {
> + "VIRTIO_NET_F_CTRL_VQ") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> + "VIRTIO_NET_F_HASH_REPORT"))) {
> return false;
> }
>
> @@ -3080,13 +3258,14 @@ static int virtnet_probe(struct virtio_device *vdev)
> u16 max_queue_pairs;
> int mtu;
>
> - /* Find if host supports multiqueue virtio_net device */
> - err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> - struct virtio_net_config,
> - max_virtqueue_pairs, &max_queue_pairs);
> + /* Find if host supports multiqueue/rss virtio_net device */
> + max_queue_pairs = 0;
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
Is VIRTIO_NET_F_RSS implied by VIRTIO_NET_F_MQ?
> + 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;
> @@ -3170,8 +3349,36 @@ 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_MRG_RXBUF) ||
> - virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> + vi->has_rss_hash_report = true;
> + vi->rss_indir_table_size = 1;
> + vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
> + }
> +
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> + vi->has_rss = true;
> + vi->rss_indir_table_size =
> + 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));
> + }
Please split adding the two features, hash report and rss, into two
separate patches.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
@ 2021-10-31 15:32 ` Willem de Bruijn
0 siblings, 0 replies; 27+ messages in thread
From: Willem de Bruijn @ 2021-10-31 15:32 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: mst, netdev, linux-kernel, virtualization, yuri.benditovich, yan,
kuba, davem
On Sun, Oct 31, 2021 at 1:00 AM Andrew Melnychenko <andrew@daynix.com> wrote:
>
> Added features for RSS and RSS hash report.
> Added initialization, RXHASH feature and ethtool ops.
> By default RSS/RXHASH is disabled.
> Added ethtools ops to set key and indirection table.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
> drivers/net/virtio_net.c | 232 +++++++++++++++++++++++++++++++++++++--
> 1 file changed, 223 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index abca2e93355d..cff7340f40bb 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -167,6 +167,28 @@ struct receive_queue {
> struct xdp_rxq_info xdp_rxq;
> };
>
> +/* This structure can contain rss message with maximum settings for indirection table and keysize
> + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> + * contains same info but can't handle table values.
> + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> + * because table sizes may be differ according to the device configuration.
> + */
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
Unless there is a technical reason, this probably should be no shorter
than TOEPLITZ_KEY_LEN
> +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> +struct virtio_net_ctrl_rss {
> + struct {
> + __le32 hash_types;
> + __le16 indirection_table_mask;
> + __le16 unclassified_queue;
Is this explicit variable needed?
> + } __packed table_info;
> + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> + struct {
> + u16 max_tx_vq; /* queues */
> + u8 hash_key_length;
> + } __packed key_info;
> + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> +};
> +
> /* Control VQ buffers: protected by the rtnl lock */
> struct control_buf {
> struct virtio_net_ctrl_hdr hdr;
> @@ -176,6 +198,7 @@ struct control_buf {
> u8 allmulti;
> __virtio16 vid;
> __virtio64 offloads;
> + struct virtio_net_ctrl_rss rss;
> };
>
> struct virtnet_info {
> @@ -204,6 +227,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;
Superfluous, can be derived form non-zero rss_key_size?
> + bool has_rss_hash_report;
> + u8 rss_key_size;
> + u16 rss_indir_table_size;
> +
> /* Has control virtqueue */
> bool has_cvq;
>
> @@ -1119,6 +1148,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);
> @@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> return;
>
> hdr = skb_vnet_hdr(skb);
> + if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
Only the second test is needed? It should be impossible to configure
the feature unless the device advertises 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;
> + }
Is this detailed protocol typing necessary? Most devices only pass a bit is_l4.
> + 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;
> @@ -2167,6 +2221,57 @@ static void virtnet_get_ringparam(struct net_device *dev,
> ring->tx_pending = ring->tx_max_pending;
> }
>
> +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> +{
> + struct net_device *dev = vi->dev;
> + struct scatterlist sgs[4];
> + unsigned int sg_buf_size;
> +
> + /* prepare sgs */
> + sg_init_table(sgs, 4);
> +
> + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> + sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> +
> + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> + sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> +
> + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> + sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> +
> + sg_buf_size = vi->rss_key_size;
> + sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> +
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> + 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;
> + }
> + return true;
> +}
> +
> +static void virtnet_init_default_rss(struct virtnet_info *vi)
> +{
> + u32 indir_val = 0;
> + int i = 0;
> +
> + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
Similar to above, and related to the next patch: is this very detailed
specification of supported hash types needed? When is this useful? It
is not customary to specify RSS to that degree.
> + vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> + vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> + vi->ctrl->rss.table_info.unclassified_queue = 0;
> +
> + for (; i < vi->rss_indir_table_size; ++i) {
> + indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> + vi->ctrl->rss.indirection_table[i] = indir_val;
> + }
> +
> + vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> + vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> +
> + netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> +}
> +
>
> static void virtnet_get_drvinfo(struct net_device *dev,
> struct ethtool_drvinfo *info)
> @@ -2395,6 +2500,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,
> @@ -2410,6 +2580,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)
> @@ -3040,7 +3215,10 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> "VIRTIO_NET_F_CTRL_VQ") ||
> VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> - "VIRTIO_NET_F_CTRL_VQ"))) {
> + "VIRTIO_NET_F_CTRL_VQ") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
> + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> + "VIRTIO_NET_F_HASH_REPORT"))) {
> return false;
> }
>
> @@ -3080,13 +3258,14 @@ static int virtnet_probe(struct virtio_device *vdev)
> u16 max_queue_pairs;
> int mtu;
>
> - /* Find if host supports multiqueue virtio_net device */
> - err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> - struct virtio_net_config,
> - max_virtqueue_pairs, &max_queue_pairs);
> + /* Find if host supports multiqueue/rss virtio_net device */
> + max_queue_pairs = 0;
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
Is VIRTIO_NET_F_RSS implied by VIRTIO_NET_F_MQ?
> + 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;
> @@ -3170,8 +3349,36 @@ 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_MRG_RXBUF) ||
> - virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> + vi->has_rss_hash_report = true;
> + vi->rss_indir_table_size = 1;
> + vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
> + }
> +
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> + vi->has_rss = true;
> + vi->rss_indir_table_size =
> + 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));
> + }
Please split adding the two features, hash report and rss, into two
separate patches.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
2021-10-31 15:32 ` Willem de Bruijn
@ 2021-10-31 15:37 ` Willem de Bruijn
-1 siblings, 0 replies; 27+ messages in thread
From: Willem de Bruijn @ 2021-10-31 15:37 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Andrew Melnychenko, mst, jasowang, davem, kuba, virtualization,
netdev, linux-kernel, yuri.benditovich, yan
> > + 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;
> > + }
>
> Is this detailed protocol typing necessary? Most devices only pass a bit is_l4.
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > + u32 indir_val = 0;
> > + int i = 0;
> > +
> > + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
>
> Similar to above, and related to the next patch: is this very detailed
> specification of supported hash types needed? When is this useful? It
> is not customary to specify RSS to that degree.
My bad. This is also implemented by bnxt, for one. I was unaware of
this feature.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
@ 2021-10-31 15:37 ` Willem de Bruijn
0 siblings, 0 replies; 27+ messages in thread
From: Willem de Bruijn @ 2021-10-31 15:37 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Andrew Melnychenko, mst, netdev, linux-kernel, virtualization,
yuri.benditovich, yan, kuba, davem
> > + 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;
> > + }
>
> Is this detailed protocol typing necessary? Most devices only pass a bit is_l4.
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > + u32 indir_val = 0;
> > + int i = 0;
> > +
> > + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
>
> Similar to above, and related to the next patch: is this very detailed
> specification of supported hash types needed? When is this useful? It
> is not customary to specify RSS to that degree.
My bad. This is also implemented by bnxt, for one. I was unaware of
this feature.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
2021-10-31 4:59 ` Andrew Melnychenko
(?)
@ 2021-10-31 16:11 ` kernel test robot
-1 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2021-10-31 16:11 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6339 bytes --]
Hi Andrew,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on mst-vhost/linux-next]
[also build test ERROR on net-next/master net/master linus/master v5.15-rc7 next-20211029]
[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/Added-RSS-support/20211031-130048
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/ca643e21484393b5386b7b86542c3ebb37445a70
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Melnychenko/Added-RSS-support/20211031-130048
git checkout ca643e21484393b5386b7b86542c3ebb37445a70
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=riscv SHELL=/bin/bash drivers/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Note: the linux-review/Andrew-Melnychenko/Added-RSS-support/20211031-130048 HEAD 385fe5b07bf0ded4667d57d48eacf27e3d4e3733 builds fine.
It only hurts bisectability.
All errors (new ones prefixed by >>):
drivers/net/virtio_net.c: In function 'page_to_skb':
>> drivers/net/virtio_net.c:396:15: error: 'struct virtnet_info' has no member named 'has_rss_hash_report'
396 | if (vi->has_rss_hash_report)
| ^~
vim +396 drivers/net/virtio_net.c
376
377 /* Called from bottom half context */
378 static struct sk_buff *page_to_skb(struct virtnet_info *vi,
379 struct receive_queue *rq,
380 struct page *page, unsigned int offset,
381 unsigned int len, unsigned int truesize,
382 bool hdr_valid, unsigned int metasize,
383 unsigned int headroom)
384 {
385 struct sk_buff *skb;
386 struct virtio_net_hdr_mrg_rxbuf *hdr;
387 unsigned int copy, hdr_len, hdr_padded_len;
388 struct page *page_to_free = NULL;
389 int tailroom, shinfo_size;
390 char *p, *hdr_p, *buf;
391
392 p = page_address(page) + offset;
393 hdr_p = p;
394
395 hdr_len = vi->hdr_len;
> 396 if (vi->has_rss_hash_report)
397 hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
398 else if (vi->mergeable_rx_bufs)
399 hdr_padded_len = sizeof(*hdr);
400 else
401 hdr_padded_len = sizeof(struct padded_vnet_hdr);
402
403 /* If headroom is not 0, there is an offset between the beginning of the
404 * data and the allocated space, otherwise the data and the allocated
405 * space are aligned.
406 *
407 * Buffers with headroom use PAGE_SIZE as alloc size, see
408 * add_recvbuf_mergeable() + get_mergeable_buf_len()
409 */
410 truesize = headroom ? PAGE_SIZE : truesize;
411 tailroom = truesize - headroom;
412 buf = p - headroom;
413
414 len -= hdr_len;
415 offset += hdr_padded_len;
416 p += hdr_padded_len;
417 tailroom -= hdr_padded_len + len;
418
419 shinfo_size = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
420
421 /* copy small packet so we can reuse these pages */
422 if (!NET_IP_ALIGN && len > GOOD_COPY_LEN && tailroom >= shinfo_size) {
423 skb = build_skb(buf, truesize);
424 if (unlikely(!skb))
425 return NULL;
426
427 skb_reserve(skb, p - buf);
428 skb_put(skb, len);
429
430 page = (struct page *)page->private;
431 if (page)
432 give_pages(rq, page);
433 goto ok;
434 }
435
436 /* copy small packet so we can reuse these pages for small data */
437 skb = napi_alloc_skb(&rq->napi, GOOD_COPY_LEN);
438 if (unlikely(!skb))
439 return NULL;
440
441 /* Copy all frame if it fits skb->head, otherwise
442 * we let virtio_net_hdr_to_skb() and GRO pull headers as needed.
443 */
444 if (len <= skb_tailroom(skb))
445 copy = len;
446 else
447 copy = ETH_HLEN + metasize;
448 skb_put_data(skb, p, copy);
449
450 len -= copy;
451 offset += copy;
452
453 if (vi->mergeable_rx_bufs) {
454 if (len)
455 skb_add_rx_frag(skb, 0, page, offset, len, truesize);
456 else
457 page_to_free = page;
458 goto ok;
459 }
460
461 /*
462 * Verify that we can indeed put this data into a skb.
463 * This is here to handle cases when the device erroneously
464 * tries to receive more than is possible. This is usually
465 * the case of a broken device.
466 */
467 if (unlikely(len > MAX_SKB_FRAGS * PAGE_SIZE)) {
468 net_dbg_ratelimited("%s: too much data\n", skb->dev->name);
469 dev_kfree_skb(skb);
470 return NULL;
471 }
472 BUG_ON(offset >= PAGE_SIZE);
473 while (len) {
474 unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len);
475 skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset,
476 frag_size, truesize);
477 len -= frag_size;
478 page = (struct page *)page->private;
479 offset = 0;
480 }
481
482 if (page)
483 give_pages(rq, page);
484
485 ok:
486 /* hdr_valid means no XDP, so we can copy the vnet header */
487 if (hdr_valid) {
488 hdr = skb_vnet_hdr(skb);
489 memcpy(hdr, hdr_p, hdr_len);
490 }
491 if (page_to_free)
492 put_page(page_to_free);
493
494 if (metasize) {
495 __skb_pull(skb, metasize);
496 skb_metadata_set(skb, metasize);
497 }
498
499 return skb;
500 }
501
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 71110 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-11-01 8:40 ` Michael S. Tsirkin
-1 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2021-11-01 8:40 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: jasowang, davem, kuba, virtualization, netdev, linux-kernel,
yuri.benditovich, yan
On Sun, Oct 31, 2021 at 06:59:56AM +0200, Andrew Melnychenko wrote:
> The header v1 provides additional info about RSS.
> Added changes to computing proper header length.
> In the next patches, the header may contain RSS hash info
> for the hash population.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4ad25a8b0870..b72b21ac8ebd 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -240,13 +240,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)
This is not helpful as a separate patch, just reserving extra space.
better squash with the patches making use of the change.
> @@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> struct virtnet_info *vi = sq->vq->vdev->priv;
> int num_sg;
> - unsigned hdr_len = vi->hdr_len;
> + unsigned int hdr_len = vi->hdr_len;
> bool can_push;
if we want this, pls make it a separate patch.
>
> pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> --
> 2.33.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
@ 2021-11-01 8:40 ` Michael S. Tsirkin
0 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2021-11-01 8:40 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: netdev, linux-kernel, virtualization, yuri.benditovich, yan, kuba, davem
On Sun, Oct 31, 2021 at 06:59:56AM +0200, Andrew Melnychenko wrote:
> The header v1 provides additional info about RSS.
> Added changes to computing proper header length.
> In the next patches, the header may contain RSS hash info
> for the hash population.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
> drivers/net/virtio_net.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4ad25a8b0870..b72b21ac8ebd 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -240,13 +240,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)
This is not helpful as a separate patch, just reserving extra space.
better squash with the patches making use of the change.
> @@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> struct virtnet_info *vi = sq->vq->vdev->priv;
> int num_sg;
> - unsigned hdr_len = vi->hdr_len;
> + unsigned int hdr_len = vi->hdr_len;
> bool can_push;
if we want this, pls make it a separate patch.
>
> pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> --
> 2.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
2021-10-31 4:59 ` Andrew Melnychenko
@ 2021-11-01 8:44 ` Michael S. Tsirkin
-1 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2021-11-01 8:44 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: jasowang, davem, kuba, virtualization, netdev, linux-kernel,
yuri.benditovich, yan
On Sun, Oct 31, 2021 at 06:59:57AM +0200, Andrew Melnychenko wrote:
> Now minimal virtual header length is may include the entire v1 header
> if the hash report were populated.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
subject isn't really descriptive. changed it how?
And I couldn't really decypher what this log entry means either.
> ---
> drivers/net/virtio_net.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b72b21ac8ebd..abca2e93355d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> hdr_p = p;
>
> hdr_len = vi->hdr_len;
> - if (vi->mergeable_rx_bufs)
> + if (vi->has_rss_hash_report)
> + hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> + else if (vi->mergeable_rx_bufs)
> hdr_padded_len = sizeof(*hdr);
> else
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -1252,7 +1254,7 @@ 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);
> + const size_t hdr_len = ((struct virtnet_info *)(rq->vq->vdev->priv))->hdr_len;
> unsigned int len;
>
> if (room)
Is this pointer chasing the best we can do?
> @@ -2817,7 +2819,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.33.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
@ 2021-11-01 8:44 ` Michael S. Tsirkin
0 siblings, 0 replies; 27+ messages in thread
From: Michael S. Tsirkin @ 2021-11-01 8:44 UTC (permalink / raw)
To: Andrew Melnychenko
Cc: netdev, linux-kernel, virtualization, yuri.benditovich, yan, kuba, davem
On Sun, Oct 31, 2021 at 06:59:57AM +0200, Andrew Melnychenko wrote:
> Now minimal virtual header length is may include the entire v1 header
> if the hash report were populated.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
subject isn't really descriptive. changed it how?
And I couldn't really decypher what this log entry means either.
> ---
> drivers/net/virtio_net.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b72b21ac8ebd..abca2e93355d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> hdr_p = p;
>
> hdr_len = vi->hdr_len;
> - if (vi->mergeable_rx_bufs)
> + if (vi->has_rss_hash_report)
> + hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> + else if (vi->mergeable_rx_bufs)
> hdr_padded_len = sizeof(*hdr);
> else
> hdr_padded_len = sizeof(struct padded_vnet_hdr);
> @@ -1252,7 +1254,7 @@ 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);
> + const size_t hdr_len = ((struct virtnet_info *)(rq->vq->vdev->priv))->hdr_len;
> unsigned int len;
>
> if (room)
Is this pointer chasing the best we can do?
> @@ -2817,7 +2819,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.33.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control.
2021-10-31 4:59 ` Andrew Melnychenko
(?)
@ 2021-11-01 19:49 ` kernel test robot
-1 siblings, 0 replies; 27+ messages in thread
From: kernel test robot @ 2021-11-01 19:49 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11642 bytes --]
Hi Andrew,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on net-next/master net/master v5.15 next-20211101]
[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/Added-RSS-support/20211031-130048
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-s021-20211101 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/385fe5b07bf0ded4667d57d48eacf27e3d4e3733
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Melnychenko/Added-RSS-support/20211031-130048
git checkout 385fe5b07bf0ded4667d57d48eacf27e3d4e3733
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 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:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1185:33: sparse: sparse: restricted __le16 degrades to integer
>> drivers/net/virtio_net.c:1203: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:1203:43: sparse: expected unsigned int [usertype] hash
drivers/net/virtio_net.c:1203:43: sparse: got restricted __le32 [usertype] hash_value
>> drivers/net/virtio_net.c:2262:45: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] hash_types @@ got unsigned int [usertype] rss_hash_types_supported @@
drivers/net/virtio_net.c:2262:45: sparse: expected restricted __le32 [usertype] hash_types
drivers/net/virtio_net.c:2262:45: sparse: got unsigned int [usertype] rss_hash_types_supported
>> drivers/net/virtio_net.c:2264:57: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] indirection_table_mask @@ got int @@
drivers/net/virtio_net.c:2264:57: sparse: expected restricted __le16 [usertype] indirection_table_mask
drivers/net/virtio_net.c:2264:57: sparse: got int
>> drivers/net/virtio_net.c:2396:53: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] hash_types @@ got unsigned int [usertype] rss_hash_types_saved @@
drivers/net/virtio_net.c:2396:53: sparse: expected restricted __le32 [usertype] hash_types
drivers/net/virtio_net.c:2396:53: sparse: got unsigned int [usertype] rss_hash_types_saved
drivers/net/virtio_net.c:2992:61: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le32 [usertype] hash_types @@ got unsigned int [usertype] rss_hash_types_saved @@
drivers/net/virtio_net.c:2992:61: sparse: expected restricted __le32 [usertype] hash_types
drivers/net/virtio_net.c:2992:61: sparse: got unsigned int [usertype] rss_hash_types_saved
vim +1185 drivers/net/virtio_net.c
23cde76d801246 Mark McLoughlin 2008-06-08 1145
7d9d60fd4ab696 Toshiaki Makita 2018-07-23 1146 static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
2471c75efed325 Jesper Dangaard Brouer 2018-06-26 1147 void *buf, unsigned int len, void **ctx,
a0929a44c2065d Toshiaki Makita 2018-07-23 1148 unsigned int *xdp_xmit,
d46eeeaf99bcfa Jason Wang 2018-07-31 1149 struct virtnet_rq_stats *stats)
9ab86bbcf8be75 Shirley Ma 2010-01-29 1150 {
e9d7417b97f420 Jason Wang 2012-12-07 1151 struct net_device *dev = vi->dev;
9ab86bbcf8be75 Shirley Ma 2010-01-29 1152 struct sk_buff *skb;
012873d057a449 Michael S. Tsirkin 2014-10-24 1153 struct virtio_net_hdr_mrg_rxbuf *hdr;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1154 struct virtio_net_hdr_v1_hash *hdr_hash;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1155 enum pkt_hash_types rss_hash_type;
fb6813f480806d Rusty Russell 2008-07-25 1156
bcff3162f3e027 Michael S. Tsirkin 2014-10-24 1157 if (unlikely(len < vi->hdr_len + ETH_HLEN)) {
9ab86bbcf8be75 Shirley Ma 2010-01-29 1158 pr_debug("%s: short packet %i\n", dev->name, len);
9ab86bbcf8be75 Shirley Ma 2010-01-29 1159 dev->stats.rx_length_errors++;
ab7db91705e95e Michael Dalton 2014-01-16 1160 if (vi->mergeable_rx_bufs) {
680557cf79f826 Michael S. Tsirkin 2017-03-06 1161 put_page(virt_to_head_page(buf));
ab7db91705e95e Michael Dalton 2014-01-16 1162 } else if (vi->big_packets) {
98bfd23cdb30e6 Michael Dalton 2013-12-05 1163 give_pages(rq, buf);
ab7db91705e95e Michael Dalton 2014-01-16 1164 } else {
f6b10209b90d48 Jason Wang 2017-02-21 1165 put_page(virt_to_head_page(buf));
ab7db91705e95e Michael Dalton 2014-01-16 1166 }
7d9d60fd4ab696 Toshiaki Makita 2018-07-23 1167 return;
9ab86bbcf8be75 Shirley Ma 2010-01-29 1168 }
9ab86bbcf8be75 Shirley Ma 2010-01-29 1169
f121159d72091f Michael S. Tsirkin 2013-11-28 1170 if (vi->mergeable_rx_bufs)
7d9d60fd4ab696 Toshiaki Makita 2018-07-23 1171 skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit,
a0929a44c2065d Toshiaki Makita 2018-07-23 1172 stats);
f121159d72091f Michael S. Tsirkin 2013-11-28 1173 else if (vi->big_packets)
a0929a44c2065d Toshiaki Makita 2018-07-23 1174 skb = receive_big(dev, vi, rq, buf, len, stats);
f121159d72091f Michael S. Tsirkin 2013-11-28 1175 else
a0929a44c2065d Toshiaki Makita 2018-07-23 1176 skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit, stats);
f121159d72091f Michael S. Tsirkin 2013-11-28 1177
8fc3b9e9a22977 Michael S. Tsirkin 2013-11-28 1178 if (unlikely(!skb))
7d9d60fd4ab696 Toshiaki Makita 2018-07-23 1179 return;
3f2c31d90327f2 Mark McLoughlin 2008-11-16 1180
9ab86bbcf8be75 Shirley Ma 2010-01-29 1181 hdr = skb_vnet_hdr(skb);
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1182 if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1183 hdr_hash = (struct virtio_net_hdr_v1_hash *)(hdr);
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1184
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 @1185 switch (hdr_hash->hash_report) {
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1186 case VIRTIO_NET_HASH_REPORT_TCPv4:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1187 case VIRTIO_NET_HASH_REPORT_UDPv4:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1188 case VIRTIO_NET_HASH_REPORT_TCPv6:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1189 case VIRTIO_NET_HASH_REPORT_UDPv6:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1190 case VIRTIO_NET_HASH_REPORT_TCPv6_EX:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1191 case VIRTIO_NET_HASH_REPORT_UDPv6_EX:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1192 rss_hash_type = PKT_HASH_TYPE_L4;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1193 break;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1194 case VIRTIO_NET_HASH_REPORT_IPv4:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1195 case VIRTIO_NET_HASH_REPORT_IPv6:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1196 case VIRTIO_NET_HASH_REPORT_IPv6_EX:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1197 rss_hash_type = PKT_HASH_TYPE_L3;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1198 break;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1199 case VIRTIO_NET_HASH_REPORT_NONE:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1200 default:
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1201 rss_hash_type = PKT_HASH_TYPE_NONE;
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1202 }
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 @1203 skb_set_hash(skb, hdr_hash->hash_value, rss_hash_type);
8bcc65dfaae785 Andrew Melnychenko 2021-10-31 1204 }
3fa2a1df909482 stephen hemminger 2011-06-15 1205
e858fae2b0b8f4 Mike Rapoport 2016-06-08 1206 if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID)
10a8d94a95742b Jason Wang 2011-06-10 1207 skb->ip_summed = CHECKSUM_UNNECESSARY;
296f96fcfc160e Rusty Russell 2007-10-22 1208
e858fae2b0b8f4 Mike Rapoport 2016-06-08 1209 if (virtio_net_hdr_to_skb(skb, &hdr->hdr,
e858fae2b0b8f4 Mike Rapoport 2016-06-08 1210 virtio_is_little_endian(vi->vdev))) {
e858fae2b0b8f4 Mike Rapoport 2016-06-08 1211 net_warn_ratelimited("%s: bad gso: type: %u, size: %u\n",
e858fae2b0b8f4 Mike Rapoport 2016-06-08 1212 dev->name, hdr->hdr.gso_type,
fdd819b21576c3 Michael S. Tsirkin 2014-10-07 1213 hdr->hdr.gso_size);
296f96fcfc160e Rusty Russell 2007-10-22 1214 goto frame_err;
296f96fcfc160e Rusty Russell 2007-10-22 1215 }
296f96fcfc160e Rusty Russell 2007-10-22 1216
133bbb18ab1a2f Willem de Bruijn 2019-01-17 1217 skb_record_rx_queue(skb, vq2rxq(rq->vq));
d1dc06dcd0f8fc Mike Rapoport 2016-06-14 1218 skb->protocol = eth_type_trans(skb, dev);
d1dc06dcd0f8fc Mike Rapoport 2016-06-14 1219 pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
d1dc06dcd0f8fc Mike Rapoport 2016-06-14 1220 ntohs(skb->protocol), skb->len, skb->pkt_type);
d1dc06dcd0f8fc Mike Rapoport 2016-06-14 1221
0fbd050a7d262b Eric Dumazet 2015-07-31 1222 napi_gro_receive(&rq->napi, skb);
7d9d60fd4ab696 Toshiaki Makita 2018-07-23 1223 return;
296f96fcfc160e Rusty Russell 2007-10-22 1224
296f96fcfc160e Rusty Russell 2007-10-22 1225 frame_err:
296f96fcfc160e Rusty Russell 2007-10-22 1226 dev->stats.rx_frame_errors++;
296f96fcfc160e Rusty Russell 2007-10-22 1227 dev_kfree_skb(skb);
296f96fcfc160e Rusty Russell 2007-10-22 1228 }
296f96fcfc160e Rusty Russell 2007-10-22 1229
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33622 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
2021-11-01 8:44 ` Michael S. Tsirkin
@ 2021-11-17 6:00 ` Andrew Melnichenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnichenko @ 2021-11-17 6:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, davem, kuba, virtualization, netdev, linux-kernel,
Yuri Benditovich, Yan Vugenfirer
On Mon, Nov 1, 2021 at 10:44 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Oct 31, 2021 at 06:59:57AM +0200, Andrew Melnychenko wrote:
> > Now minimal virtual header length is may include the entire v1 header
> > if the hash report were populated.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
>
> subject isn't really descriptive. changed it how?
>
> And I couldn't really decypher what this log entry means either.
>
I'll change it in the next patch.
So, I've tried to ensure that the v1 header with the hash report will
be available if required in new patches.
> > ---
> > drivers/net/virtio_net.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index b72b21ac8ebd..abca2e93355d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > hdr_p = p;
> >
> > hdr_len = vi->hdr_len;
> > - if (vi->mergeable_rx_bufs)
> > + if (vi->has_rss_hash_report)
> > + hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > + else if (vi->mergeable_rx_bufs)
> > hdr_padded_len = sizeof(*hdr);
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -1252,7 +1254,7 @@ 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);
> > + const size_t hdr_len = ((struct virtnet_info *)(rq->vq->vdev->priv))->hdr_len;
> > unsigned int len;
> >
> > if (room)
>
> Is this pointer chasing the best we can do?
I'll change that.
>
> > @@ -2817,7 +2819,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.33.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation.
@ 2021-11-17 6:00 ` Andrew Melnichenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnichenko @ 2021-11-17 6:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, virtualization, Yuri Benditovich,
Yan Vugenfirer, kuba, davem
On Mon, Nov 1, 2021 at 10:44 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Oct 31, 2021 at 06:59:57AM +0200, Andrew Melnychenko wrote:
> > Now minimal virtual header length is may include the entire v1 header
> > if the hash report were populated.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
>
> subject isn't really descriptive. changed it how?
>
> And I couldn't really decypher what this log entry means either.
>
I'll change it in the next patch.
So, I've tried to ensure that the v1 header with the hash report will
be available if required in new patches.
> > ---
> > drivers/net/virtio_net.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index b72b21ac8ebd..abca2e93355d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -393,7 +393,9 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> > hdr_p = p;
> >
> > hdr_len = vi->hdr_len;
> > - if (vi->mergeable_rx_bufs)
> > + if (vi->has_rss_hash_report)
> > + hdr_padded_len = sizeof(struct virtio_net_hdr_v1_hash);
> > + else if (vi->mergeable_rx_bufs)
> > hdr_padded_len = sizeof(*hdr);
> > else
> > hdr_padded_len = sizeof(struct padded_vnet_hdr);
> > @@ -1252,7 +1254,7 @@ 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);
> > + const size_t hdr_len = ((struct virtnet_info *)(rq->vq->vdev->priv))->hdr_len;
> > unsigned int len;
> >
> > if (room)
>
> Is this pointer chasing the best we can do?
I'll change that.
>
> > @@ -2817,7 +2819,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.33.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
2021-11-01 8:40 ` Michael S. Tsirkin
@ 2021-11-17 6:00 ` Andrew Melnichenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnichenko @ 2021-11-17 6:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, davem, kuba, virtualization, netdev, linux-kernel,
Yuri Benditovich, Yan Vugenfirer
On Mon, Nov 1, 2021 at 10:40 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Oct 31, 2021 at 06:59:56AM +0200, Andrew Melnychenko wrote:
> > The header v1 provides additional info about RSS.
> > Added changes to computing proper header length.
> > In the next patches, the header may contain RSS hash info
> > for the hash population.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> > drivers/net/virtio_net.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 4ad25a8b0870..b72b21ac8ebd 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -240,13 +240,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)
>
>
> This is not helpful as a separate patch, just reserving extra space.
> better squash with the patches making use of the change.
Ok.
>
> > @@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> > const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> > struct virtnet_info *vi = sq->vq->vdev->priv;
> > int num_sg;
> > - unsigned hdr_len = vi->hdr_len;
> > + unsigned int hdr_len = vi->hdr_len;
> > bool can_push;
>
>
> if we want this, pls make it a separate patch.
Ok. I've added that change after checkpatch warnings. Technically,
checkpatch should not fail on the patch without that line.
>
>
> >
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> > --
> > 2.33.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
2021-10-31 15:32 ` Willem de Bruijn
@ 2021-11-17 6:00 ` Andrew Melnichenko
-1 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnichenko @ 2021-11-17 6:00 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, Jason Wang, davem, kuba, virtualization,
netdev, linux-kernel, Yuri Benditovich, Yan Vugenfirer
On Sun, Oct 31, 2021 at 5:33 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Sun, Oct 31, 2021 at 1:00 AM Andrew Melnychenko <andrew@daynix.com> wrote:
> >
> > Added features for RSS and RSS hash report.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> > drivers/net/virtio_net.c | 232 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 223 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index abca2e93355d..cff7340f40bb 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -167,6 +167,28 @@ struct receive_queue {
> > struct xdp_rxq_info xdp_rxq;
> > };
> >
> > +/* This structure can contain rss message with maximum settings for indirection table and keysize
> > + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
>
> Unless there is a technical reason, this probably should be no shorter
> than TOEPLITZ_KEY_LEN
Well yeah, technically if the device requests for shorter key, we
still may provide it.
I think we may check and 'disable' RSS if some configurations are inadequate.
>
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> > +struct virtio_net_ctrl_rss {
> > + struct {
> > + __le32 hash_types;
> > + __le16 indirection_table_mask;
> > + __le16 unclassified_queue;
>
> Is this explicit variable needed?
Yes, it's a part of the message to be sent to the device.
>
> > + } __packed table_info;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + struct {
> > + u16 max_tx_vq; /* queues */
> > + u8 hash_key_length;
> > + } __packed key_info;
> > + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
> > /* Control VQ buffers: protected by the rtnl lock */
> > struct control_buf {
> > struct virtio_net_ctrl_hdr hdr;
> > @@ -176,6 +198,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -204,6 +227,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;
>
> Superfluous, can be derived form non-zero rss_key_size?
I think that the explicit 'has_rss' field is better. "non-zero
rss_key_size" should work, I'll change RSS derivation in the next
patches.
>
> > + bool has_rss_hash_report;
> > + u8 rss_key_size;
> > + u16 rss_indir_table_size;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> >
> > @@ -1119,6 +1148,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);
> > @@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> > return;
> >
> > hdr = skb_vnet_hdr(skb);
> > + if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
>
> Only the second test is needed? It should be impossible to configure
> the feature unless the device advertises has_rss_hash_report
Well, you can have RSS without a hash population. So, need to check,
is the device supports hash population and rxhash is enabled(g.e.
through ethtool).
>
> > + 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;
> > + }
>
> Is this detailed protocol typing necessary? Most devices only pass a bit is_l4.
Yes, in theory, there is real hardware that may support only L3 hash
calculations.
>
> > + 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;
> > @@ -2167,6 +2221,57 @@ static void virtnet_get_ringparam(struct net_device *dev,
> > ring->tx_pending = ring->tx_max_pending;
> > }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > + struct net_device *dev = vi->dev;
> > + struct scatterlist sgs[4];
> > + unsigned int sg_buf_size;
> > +
> > + /* prepare sgs */
> > + sg_init_table(sgs, 4);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> > + sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > + sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> > + sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> > +
> > + sg_buf_size = vi->rss_key_size;
> > + sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> > +
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > + 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;
> > + }
> > + return true;
> > +}
> > +
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > + u32 indir_val = 0;
> > + int i = 0;
> > +
> > + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
>
> Similar to above, and related to the next patch: is this very detailed
> specification of supported hash types needed? When is this useful? It
> is not customary to specify RSS to that degree.
In theory, there are real devices that implement virtio_net with(or
without) some supported hashes.
>
> > + vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> > + vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> > + vi->ctrl->rss.table_info.unclassified_queue = 0;
> > +
> > + for (; i < vi->rss_indir_table_size; ++i) {
> > + indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> > + vi->ctrl->rss.indirection_table[i] = indir_val;
> > + }
> > +
> > + vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> > + vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> > +
> > + netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> > +}
> > +
> >
> > static void virtnet_get_drvinfo(struct net_device *dev,
> > struct ethtool_drvinfo *info)
> > @@ -2395,6 +2500,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,
> > @@ -2410,6 +2580,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)
> > @@ -3040,7 +3215,10 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> > "VIRTIO_NET_F_CTRL_VQ") ||
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> > - "VIRTIO_NET_F_CTRL_VQ"))) {
> > + "VIRTIO_NET_F_CTRL_VQ") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > + "VIRTIO_NET_F_HASH_REPORT"))) {
> > return false;
> > }
> >
> > @@ -3080,13 +3258,14 @@ static int virtnet_probe(struct virtio_device *vdev)
> > u16 max_queue_pairs;
> > int mtu;
> >
> > - /* Find if host supports multiqueue virtio_net device */
> > - err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> > - struct virtio_net_config,
> > - max_virtqueue_pairs, &max_queue_pairs);
> > + /* Find if host supports multiqueue/rss virtio_net device */
> > + max_queue_pairs = 0;
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
>
> Is VIRTIO_NET_F_RSS implied by VIRTIO_NET_F_MQ?
MQ and/or RSS sets multiqueue, like so:
> virtio_net_set_multiqueue(n,
> virtio_has_feature(features, VIRTIO_NET_F_RSS) ||
> virtio_has_feature(features, VIRTIO_NET_F_MQ));
So, technically it's possible to create a virtual net only with RSS without mq.
>
> > + 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;
> > @@ -3170,8 +3349,36 @@ 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_MRG_RXBUF) ||
> > - virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> > + vi->has_rss_hash_report = true;
> > + vi->rss_indir_table_size = 1;
> > + vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
> > + }
> > +
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > + vi->has_rss = true;
> > + vi->rss_indir_table_size =
> > + 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));
> > + }
>
> Please split adding the two features, hash report and rss, into two
> separate patches.
It may provide dead code that will be replaced by code in 'hash
report' report patch.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1.
@ 2021-11-17 6:00 ` Andrew Melnichenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnichenko @ 2021-11-17 6:00 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, linux-kernel, virtualization, Yuri Benditovich,
Yan Vugenfirer, kuba, davem
On Mon, Nov 1, 2021 at 10:40 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Sun, Oct 31, 2021 at 06:59:56AM +0200, Andrew Melnychenko wrote:
> > The header v1 provides additional info about RSS.
> > Added changes to computing proper header length.
> > In the next patches, the header may contain RSS hash info
> > for the hash population.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> > drivers/net/virtio_net.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 4ad25a8b0870..b72b21ac8ebd 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -240,13 +240,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)
>
>
> This is not helpful as a separate patch, just reserving extra space.
> better squash with the patches making use of the change.
Ok.
>
> > @@ -1636,7 +1636,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> > const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
> > struct virtnet_info *vi = sq->vq->vdev->priv;
> > int num_sg;
> > - unsigned hdr_len = vi->hdr_len;
> > + unsigned int hdr_len = vi->hdr_len;
> > bool can_push;
>
>
> if we want this, pls make it a separate patch.
Ok. I've added that change after checkpatch warnings. Technically,
checkpatch should not fail on the patch without that line.
>
>
> >
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> > --
> > 2.33.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support.
@ 2021-11-17 6:00 ` Andrew Melnichenko
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Melnichenko @ 2021-11-17 6:00 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
Yuri Benditovich, Yan Vugenfirer, kuba, davem
On Sun, Oct 31, 2021 at 5:33 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Sun, Oct 31, 2021 at 1:00 AM Andrew Melnychenko <andrew@daynix.com> wrote:
> >
> > Added features for RSS and RSS hash report.
> > Added initialization, RXHASH feature and ethtool ops.
> > By default RSS/RXHASH is disabled.
> > Added ethtools ops to set key and indirection table.
> >
> > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > ---
> > drivers/net/virtio_net.c | 232 +++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 223 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index abca2e93355d..cff7340f40bb 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -167,6 +167,28 @@ struct receive_queue {
> > struct xdp_rxq_info xdp_rxq;
> > };
> >
> > +/* This structure can contain rss message with maximum settings for indirection table and keysize
> > + * Note, that default structure that describes RSS configuration virtio_net_rss_config
> > + * contains same info but can't handle table values.
> > + * In any case, structure would be passed to virtio hw through sg_buf split by parts
> > + * because table sizes may be differ according to the device configuration.
> > + */
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE 40
>
> Unless there is a technical reason, this probably should be no shorter
> than TOEPLITZ_KEY_LEN
Well yeah, technically if the device requests for shorter key, we
still may provide it.
I think we may check and 'disable' RSS if some configurations are inadequate.
>
> > +#define VIRTIO_NET_RSS_MAX_TABLE_LEN 128
> > +struct virtio_net_ctrl_rss {
> > + struct {
> > + __le32 hash_types;
> > + __le16 indirection_table_mask;
> > + __le16 unclassified_queue;
>
> Is this explicit variable needed?
Yes, it's a part of the message to be sent to the device.
>
> > + } __packed table_info;
> > + u16 indirection_table[VIRTIO_NET_RSS_MAX_TABLE_LEN];
> > + struct {
> > + u16 max_tx_vq; /* queues */
> > + u8 hash_key_length;
> > + } __packed key_info;
> > + u8 key[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> > +};
> > +
> > /* Control VQ buffers: protected by the rtnl lock */
> > struct control_buf {
> > struct virtio_net_ctrl_hdr hdr;
> > @@ -176,6 +198,7 @@ struct control_buf {
> > u8 allmulti;
> > __virtio16 vid;
> > __virtio64 offloads;
> > + struct virtio_net_ctrl_rss rss;
> > };
> >
> > struct virtnet_info {
> > @@ -204,6 +227,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;
>
> Superfluous, can be derived form non-zero rss_key_size?
I think that the explicit 'has_rss' field is better. "non-zero
rss_key_size" should work, I'll change RSS derivation in the next
patches.
>
> > + bool has_rss_hash_report;
> > + u8 rss_key_size;
> > + u16 rss_indir_table_size;
> > +
> > /* Has control virtqueue */
> > bool has_cvq;
> >
> > @@ -1119,6 +1148,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);
> > @@ -1145,6 +1176,29 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> > return;
> >
> > hdr = skb_vnet_hdr(skb);
> > + if (vi->has_rss_hash_report && (dev->features & NETIF_F_RXHASH)) {
>
> Only the second test is needed? It should be impossible to configure
> the feature unless the device advertises has_rss_hash_report
Well, you can have RSS without a hash population. So, need to check,
is the device supports hash population and rxhash is enabled(g.e.
through ethtool).
>
> > + 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;
> > + }
>
> Is this detailed protocol typing necessary? Most devices only pass a bit is_l4.
Yes, in theory, there is real hardware that may support only L3 hash
calculations.
>
> > + 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;
> > @@ -2167,6 +2221,57 @@ static void virtnet_get_ringparam(struct net_device *dev,
> > ring->tx_pending = ring->tx_max_pending;
> > }
> >
> > +static bool virtnet_commit_rss_command(struct virtnet_info *vi)
> > +{
> > + struct net_device *dev = vi->dev;
> > + struct scatterlist sgs[4];
> > + unsigned int sg_buf_size;
> > +
> > + /* prepare sgs */
> > + sg_init_table(sgs, 4);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.table_info);
> > + sg_set_buf(&sgs[0], &vi->ctrl->rss.table_info, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(uint16_t) * vi->rss_indir_table_size;
> > + sg_set_buf(&sgs[1], vi->ctrl->rss.indirection_table, sg_buf_size);
> > +
> > + sg_buf_size = sizeof(vi->ctrl->rss.key_info);
> > + sg_set_buf(&sgs[2], &vi->ctrl->rss.key_info, sg_buf_size);
> > +
> > + sg_buf_size = vi->rss_key_size;
> > + sg_set_buf(&sgs[3], vi->ctrl->rss.key, sg_buf_size);
> > +
> > + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MQ,
> > + 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;
> > + }
> > + return true;
> > +}
> > +
> > +static void virtnet_init_default_rss(struct virtnet_info *vi)
> > +{
> > + u32 indir_val = 0;
> > + int i = 0;
> > +
> > + vi->ctrl->rss.table_info.hash_types = vi->rss_hash_types_supported;
>
> Similar to above, and related to the next patch: is this very detailed
> specification of supported hash types needed? When is this useful? It
> is not customary to specify RSS to that degree.
In theory, there are real devices that implement virtio_net with(or
without) some supported hashes.
>
> > + vi->rss_hash_types_saved = vi->rss_hash_types_supported;
> > + vi->ctrl->rss.table_info.indirection_table_mask = vi->rss_indir_table_size - 1;
> > + vi->ctrl->rss.table_info.unclassified_queue = 0;
> > +
> > + for (; i < vi->rss_indir_table_size; ++i) {
> > + indir_val = ethtool_rxfh_indir_default(i, vi->max_queue_pairs);
> > + vi->ctrl->rss.indirection_table[i] = indir_val;
> > + }
> > +
> > + vi->ctrl->rss.key_info.max_tx_vq = vi->curr_queue_pairs;
> > + vi->ctrl->rss.key_info.hash_key_length = vi->rss_key_size;
> > +
> > + netdev_rss_key_fill(vi->ctrl->rss.key, vi->rss_key_size);
> > +}
> > +
> >
> > static void virtnet_get_drvinfo(struct net_device *dev,
> > struct ethtool_drvinfo *info)
> > @@ -2395,6 +2500,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,
> > @@ -2410,6 +2580,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)
> > @@ -3040,7 +3215,10 @@ static bool virtnet_validate_features(struct virtio_device *vdev)
> > "VIRTIO_NET_F_CTRL_VQ") ||
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_MQ, "VIRTIO_NET_F_CTRL_VQ") ||
> > VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR,
> > - "VIRTIO_NET_F_CTRL_VQ"))) {
> > + "VIRTIO_NET_F_CTRL_VQ") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_RSS, "VIRTIO_NET_F_RSS") ||
> > + VIRTNET_FAIL_ON(vdev, VIRTIO_NET_F_HASH_REPORT,
> > + "VIRTIO_NET_F_HASH_REPORT"))) {
> > return false;
> > }
> >
> > @@ -3080,13 +3258,14 @@ static int virtnet_probe(struct virtio_device *vdev)
> > u16 max_queue_pairs;
> > int mtu;
> >
> > - /* Find if host supports multiqueue virtio_net device */
> > - err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ,
> > - struct virtio_net_config,
> > - max_virtqueue_pairs, &max_queue_pairs);
> > + /* Find if host supports multiqueue/rss virtio_net device */
> > + max_queue_pairs = 0;
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
>
> Is VIRTIO_NET_F_RSS implied by VIRTIO_NET_F_MQ?
MQ and/or RSS sets multiqueue, like so:
> virtio_net_set_multiqueue(n,
> virtio_has_feature(features, VIRTIO_NET_F_RSS) ||
> virtio_has_feature(features, VIRTIO_NET_F_MQ));
So, technically it's possible to create a virtual net only with RSS without mq.
>
> > + 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;
> > @@ -3170,8 +3349,36 @@ 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_MRG_RXBUF) ||
> > - virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
> > + vi->has_rss_hash_report = true;
> > + vi->rss_indir_table_size = 1;
> > + vi->rss_key_size = VIRTIO_NET_RSS_MAX_KEY_SIZE;
> > + }
> > +
> > + if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > + vi->has_rss = true;
> > + vi->rss_indir_table_size =
> > + 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));
> > + }
>
> Please split adding the two features, hash report and rss, into two
> separate patches.
It may provide dead code that will be replaced by code in 'hash
report' report patch.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2021-11-17 6:01 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-31 4:59 [RFC PATCH 0/4] Added RSS support Andrew Melnychenko
2021-10-31 4:59 ` Andrew Melnychenko
2021-10-31 4:59 ` [RFC PATCH 1/4] drivers/net/virtio_net: Fixed vheader to use v1 Andrew Melnychenko
2021-10-31 4:59 ` Andrew Melnychenko
2021-11-01 8:40 ` Michael S. Tsirkin
2021-11-01 8:40 ` Michael S. Tsirkin
2021-11-17 6:00 ` Andrew Melnichenko
2021-11-17 6:00 ` Andrew Melnichenko
2021-10-31 4:59 ` [RFC PATCH 2/4] drivers/net/virtio_net: Changed mergeable buffer length calculation Andrew Melnychenko
2021-10-31 4:59 ` Andrew Melnychenko
2021-10-31 16:11 ` kernel test robot
2021-11-01 8:44 ` Michael S. Tsirkin
2021-11-01 8:44 ` Michael S. Tsirkin
2021-11-17 6:00 ` Andrew Melnichenko
2021-11-17 6:00 ` Andrew Melnichenko
2021-10-31 4:59 ` [RFC PATCH 3/4] drivers/net/virtio_net: Added basic RSS support Andrew Melnychenko
2021-10-31 4:59 ` Andrew Melnychenko
2021-10-31 15:30 ` kernel test robot
2021-10-31 15:32 ` Willem de Bruijn
2021-10-31 15:32 ` Willem de Bruijn
2021-10-31 15:37 ` Willem de Bruijn
2021-10-31 15:37 ` Willem de Bruijn
2021-11-17 6:00 ` Andrew Melnichenko
2021-11-17 6:00 ` Andrew Melnichenko
2021-10-31 4:59 ` [RFC PATCH 4/4] drivers/net/virtio_net: Added RSS hash report control Andrew Melnychenko
2021-10-31 4:59 ` Andrew Melnychenko
2021-11-01 19:49 ` kernel test robot
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.