Thread (29 messages) 29 messages, 4 authors, 2026-02-26

Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2026-02-25 12:37:14
Also in: stable, virtualization

On Wed, Feb 25, 2026 at 12:34:28PM +0000, Srujana Challa wrote:
quoted
quoted
quoted
On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
quoted
Replace hardcoded RSS max key size limit with NETDEV_RSS_KEY_LEN
to align with kernel's standard RSS key length. Add validation for
RSS key size against spec minimum (40 bytes) and driver maximum.
When validation fails, gracefully disable RSS features and
continue initialization rather than failing completely.

Cc: stable@vger.kernel.org
Fixes: 3f7d9c1964fc ("virtio_net: Add hash_key_length check")
Signed-off-by: Srujana Challa <schalla@marvell.com>
--- should come here before changelog.
quoted
v3:
- Moved RSS key validation checks to virtnet_validate.
- Add fixes: tag and CC -stable
v4:
- Use NETDEV_RSS_KEY_LEN instead of type_max for the maximum rss
key
size.
quoted
---
 drivers/net/virtio_net.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index
db88dcaefb20..eeefe8abc122 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -381,8 +381,6 @@ struct receive_queue {
 	struct xdp_buff **xsk_buffs;
 };

-#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
-
 /* Control VQ buffers: protected by the rtnl lock */  struct
control_buf {
 	struct virtio_net_ctrl_hdr hdr;
@@ -486,7 +484,7 @@ struct virtnet_info {

 	/* Must be last as it ends in a flexible-array member. */
 	TRAILING_OVERLAP(struct virtio_net_rss_config_trailer,
rss_trailer,
hash_key_data,
quoted
-		u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
+		u8 rss_hash_key_data[NETDEV_RSS_KEY_LEN];
 	);
 };
 static_assert(offsetof(struct virtnet_info,
rss_trailer.hash_key_data) == @@ -6627,6 +6625,29 @@ static int
virtnet_validate(struct virtio_device *vdev)
quoted
 		__virtio_clear_bit(vdev, VIRTIO_NET_F_STANDBY);
 	}

+	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS) ||
+	    virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT)) {
+		u8 key_sz = virtio_cread8(vdev,
+					  offsetof(struct virtio_net_config,
+						   rss_max_key_size));
+		/* Spec requires at least 40 bytes */ #define
+VIRTIO_NET_RSS_MIN_KEY_SIZE 40
+		if (key_sz < VIRTIO_NET_RSS_MIN_KEY_SIZE) {
+			dev_warn(&vdev->dev,
+				 "rss_max_key_size=%u is less than spec
minimum %u, disabling RSS\n",
quoted
+				 key_sz, VIRTIO_NET_RSS_MIN_KEY_SIZE);
+			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
+			__virtio_clear_bit(vdev,
VIRTIO_NET_F_HASH_REPORT);
quoted
+		}
+		if (key_sz > NETDEV_RSS_KEY_LEN) {
+			dev_warn(&vdev->dev,
+				 "rss_max_key_size=%u exceeds driver limit
%u, disabling RSS\n",
quoted
+				 key_sz, NETDEV_RSS_KEY_LEN);
+			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
+			__virtio_clear_bit(vdev,
VIRTIO_NET_F_HASH_REPORT);

you flipped the logic here and it makes no sense now.

Did you test this path?
Yes, tested with Marvell's Octeon device.
quoted

So if device is powerful and supports a very big key size then...
we disable the feature? how does this make sense?
The intent isn’t to disable the feature on capable devices, but to
ensure the driver never advertises support for RSS key sizes larger
than what the net device can actually handle. Even if a device reports a very
large key size, the driver is constrained by NETDEV_RSS_KEY_LEN, since
netdev_rss_key_fill() enforces:
quoted
BUG_ON(len > sizeof(netdev_rss_key));
so cap it to NETDEV_RSS_KEY_LEN. Why is that a reason to clear the feature?
Our device mandates that hash_key_length must be identical to rss_max_key_size
to guarantee symmetric bidirectional flow hashing. If rss_max_key_size is larger than
VIRTIO_NET_RSS_MAX_KEY_SIZE, clamping the value is not feasible.
I don't know what to tell you. rss_max_key_size is just the max device
supports. driver should be free to use a smaller size.

quoted
quoted
quoted
quoted
+		}
+	}
+
 	return 0;
 }
@@ -6839,13 +6860,6 @@ static int virtnet_probe(struct
virtio_device
*vdev)
quoted
 	if (vi->has_rss || vi->has_rss_hash_report) {
 		vi->rss_key_size =
 			virtio_cread8(vdev, offsetof(struct virtio_net_config,
rss_max_key_size));
quoted
-		if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
-			dev_err(&vdev->dev, "rss_max_key_size=%u exceeds
the limit %u.\n",
quoted
-				vi->rss_key_size,
VIRTIO_NET_RSS_MAX_KEY_SIZE);
quoted
-			err = -EINVAL;
-			goto free;
-		}
-
 		vi->rss_hash_types_supported =
 		    virtio_cread32(vdev, offsetof(struct virtio_net_config,
supported_hash_types));
quoted
 		vi->rss_hash_types_supported &=
--
2.25.1
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help