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

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

[Srujana Challa]

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>

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.

---
 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,
-		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)
 		__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",
+				 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);
+		}
+		if (key_sz > NETDEV_RSS_KEY_LEN) {
+			dev_warn(&vdev->dev,
+				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
+				 key_sz, NETDEV_RSS_KEY_LEN);
+			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
+			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
+		}
+	}
+
 	return 0;
 }
 
@@ -6839,13 +6860,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 	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));
-		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",
-				vi->rss_key_size, VIRTIO_NET_RSS_MAX_KEY_SIZE);
-			err = -EINVAL;
-			goto free;
-		}
-
 		vi->rss_hash_types_supported =
 		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
 		vi->rss_hash_types_supported &=
-- 
2.25.1

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

[Srujana Challa]

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>

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.
---
 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,
-		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)
 		__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",
+				 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);
+		}
+		if (key_sz > NETDEV_RSS_KEY_LEN) {
+			dev_warn(&vdev->dev,
+				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
+				 key_sz, NETDEV_RSS_KEY_LEN);
+			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
+			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
+		}
+	}
+
 	return 0;
 }
 
@@ -6839,13 +6860,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 	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));
-		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",
-				vi->rss_key_size, VIRTIO_NET_RSS_MAX_KEY_SIZE);
-			err = -EINVAL;
-			goto free;
-		}
-
 		vi->rss_hash_types_supported =
 		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
 		vi->rss_hash_types_supported &=
-- 
2.25.1

[PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Srujana Challa]

Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
BUILD_BUG_ON to enforce the limit at compile time and remove the
redundant runtime max check.

Signed-off-by: Srujana Challa <schalla@marvell.com>
---
 drivers/net/virtio_net.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index eeefe8abc122..768ad5523dfa 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
 			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
 			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
 		}
-		if (key_sz > NETDEV_RSS_KEY_LEN) {
-			dev_warn(&vdev->dev,
-				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
-				 key_sz, NETDEV_RSS_KEY_LEN);
-			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
-			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
-		}
+		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
 	}
 
 	return 0;
-- 
2.25.1

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Xuan Zhuo]

On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> BUILD_BUG_ON to enforce the limit at compile time and remove the
> redundant runtime max check.
>
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>  drivers/net/virtio_net.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index eeefe8abc122..768ad5523dfa 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
>  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
>  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
>  		}
> -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> -			dev_warn(&vdev->dev,
> -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> -				 key_sz, NETDEV_RSS_KEY_LEN);
> -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> -		}
> +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);

Do we really need this check?

If I understand correctly, the intention is to cap key_sz at 256. However, since
key_sz is of type u8, its maximum value is inherently 255, making this check
redundant. This is not only limited by this kernel code, the virtio-net spec
defines this.

Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
the future, this check would no longer enforce the intended limit correctly.


Moreover, you should add a cover letter.

Thanks.





>  	}
>
>  	return 0;
> --
> 2.25.1
>

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > redundant runtime max check.
> >
> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > ---
> >  drivers/net/virtio_net.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index eeefe8abc122..768ad5523dfa 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> >  		}
> > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > -			dev_warn(&vdev->dev,
> > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > -		}
> > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> 
> Do we really need this check?
> 
> If I understand correctly, the intention is to cap key_sz at 256. However, since
> key_sz is of type u8, its maximum value is inherently 255, making this check
> redundant. This is not only limited by this kernel code, the virtio-net spec
> defines this.

That's why it's BUILD_BUG_ON. It checks it has the right type.

We never *need* BUILD_BUG_ON by definition, what this does is
document the assumption.


> Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> the future, this check would no longer enforce the intended limit correctly.

then it would fail build.

> 
> Moreover, you should add a cover letter.
> 
> Thanks.
> 
> 
> 
> 
> 
> >  	}
> >
> >  	return 0;
> > --
> > 2.25.1
> >

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Xuan Zhuo]

On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > redundant runtime max check.
> > >
> > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > ---
> > >  drivers/net/virtio_net.c | 8 +-------
> > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index eeefe8abc122..768ad5523dfa 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > >  		}
> > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > -			dev_warn(&vdev->dev,
> > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > -		}
> > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> >
> > Do we really need this check?
> >
> > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > key_sz is of type u8, its maximum value is inherently 255, making this check
> > redundant. This is not only limited by this kernel code, the virtio-net spec
> > defines this.
>
> That's why it's BUILD_BUG_ON. It checks it has the right type.
>
> We never *need* BUILD_BUG_ON by definition, what this does is
> document the assumption.
>
>
> > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > the future, this check would no longer enforce the intended limit correctly.
>
> then it would fail build.

So, does this mean we don't need to account for the case where
NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?

Thanks.


>
> >
> > Moreover, you should add a cover letter.
> >
> > Thanks.
> >
> >
> >
> >
> >
> > >  	}
> > >
> > >  	return 0;
> > > --
> > > 2.25.1
> > >
>

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > > redundant runtime max check.
> > > >
> > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > ---
> > > >  drivers/net/virtio_net.c | 8 +-------
> > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > index eeefe8abc122..768ad5523dfa 100644
> > > > --- a/drivers/net/virtio_net.c
> > > > +++ b/drivers/net/virtio_net.c
> > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > >  		}
> > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > -			dev_warn(&vdev->dev,
> > > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > -		}
> > > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> > >
> > > Do we really need this check?
> > >
> > > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > > key_sz is of type u8, its maximum value is inherently 255, making this check
> > > redundant. This is not only limited by this kernel code, the virtio-net spec
> > > defines this.
> >
> > That's why it's BUILD_BUG_ON. It checks it has the right type.
> >
> > We never *need* BUILD_BUG_ON by definition, what this does is
> > document the assumption.
> >
> >
> > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > > the future, this check would no longer enforce the intended limit correctly.
> >
> > then it would fail build.
> 
> So, does this mean we don't need to account for the case where
> NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?
> 
> Thanks.
> 

yes. the code makes assumptions but it documents them and not
just documents them, build will fail if they are violated.


> >
> > >
> > > Moreover, you should add a cover letter.
> > >
> > > Thanks.
> > >
> > >
> > >
> > >
> > >
> > > >  	}
> > > >
> > > >  	return 0;
> > > > --
> > > > 2.25.1
> > > >
> >

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Xuan Zhuo]

On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > > > redundant runtime max check.
> > > > >
> > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > ---
> > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > index eeefe8abc122..768ad5523dfa 100644
> > > > > --- a/drivers/net/virtio_net.c
> > > > > +++ b/drivers/net/virtio_net.c
> > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > >  		}
> > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > -			dev_warn(&vdev->dev,
> > > > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > -		}
> > > > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> > > >
> > > > Do we really need this check?
> > > >
> > > > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > > > key_sz is of type u8, its maximum value is inherently 255, making this check
> > > > redundant. This is not only limited by this kernel code, the virtio-net spec
> > > > defines this.
> > >
> > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > >
> > > We never *need* BUILD_BUG_ON by definition, what this does is
> > > document the assumption.
> > >
> > >
> > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > > > the future, this check would no longer enforce the intended limit correctly.
> > >
> > > then it would fail build.
> >
> > So, does this mean we don't need to account for the case where
> > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?
> >
> > Thanks.
> >
>
> yes.

Why?

If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of 64, does this
violate the spec?

> the code makes assumptions but it documents them and not
> just documents them, build will fail if they are violated.

About this, I am ok.

Thanks.


>
>
> > >
> > > >
> > > > Moreover, you should add a cover letter.
> > > >
> > > > Thanks.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >  	}
> > > > >
> > > > >  	return 0;
> > > > > --
> > > > > 2.25.1
> > > > >
> > >
>

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > > > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > > > > redundant runtime max check.
> > > > > >
> > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > ---
> > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > index eeefe8abc122..768ad5523dfa 100644
> > > > > > --- a/drivers/net/virtio_net.c
> > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > >  		}
> > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > -			dev_warn(&vdev->dev,
> > > > > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > -		}
> > > > > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> > > > >
> > > > > Do we really need this check?
> > > > >
> > > > > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > > > > key_sz is of type u8, its maximum value is inherently 255, making this check
> > > > > redundant. This is not only limited by this kernel code, the virtio-net spec
> > > > > defines this.
> > > >
> > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > >
> > > > We never *need* BUILD_BUG_ON by definition, what this does is
> > > > document the assumption.
> > > >
> > > >
> > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > > > > the future, this check would no longer enforce the intended limit correctly.
> > > >
> > > > then it would fail build.
> > >
> > > So, does this mean we don't need to account for the case where
> > > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?
> > >
> > > Thanks.
> > >
> >
> > yes.
> 
> Why?
> 
> If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of 64, does this
> violate the spec?

not the value of key_sz. If type of key_sz 


i actually do not understand the question. this is not what BUILD_BUG_ON
checks.

> > the code makes assumptions but it documents them and not
> > just documents them, build will fail if they are violated.
> 
> About this, I am ok.
> 
> Thanks.
> 
> 
> >
> >
> > > >
> > > > >
> > > > > Moreover, you should add a cover letter.
> > > > >
> > > > > Thanks.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > >  	}
> > > > > >
> > > > > >  	return 0;
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > >
> >

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Xuan Zhuo]

On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > > > > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > > > > > redundant runtime max check.
> > > > > > >
> > > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > > ---
> > > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > > index eeefe8abc122..768ad5523dfa 100644
> > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > >  		}
> > > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > -			dev_warn(&vdev->dev,
> > > > > > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > > -		}
> > > > > > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> > > > > >
> > > > > > Do we really need this check?
> > > > > >
> > > > > > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > > > > > key_sz is of type u8, its maximum value is inherently 255, making this check
> > > > > > redundant. This is not only limited by this kernel code, the virtio-net spec
> > > > > > defines this.
> > > > >
> > > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > > >
> > > > > We never *need* BUILD_BUG_ON by definition, what this does is
> > > > > document the assumption.
> > > > >
> > > > >
> > > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > > > > > the future, this check would no longer enforce the intended limit correctly.
> > > > >
> > > > > then it would fail build.
> > > >
> > > > So, does this mean we don't need to account for the case where
> > > > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?
> > > >
> > > > Thanks.
> > > >
> > >
> > > yes.
> >
> > Why?
> >
> > If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of 64, does this
> > violate the spec?
>
> not the value of key_sz. If type of key_sz
>
>
> i actually do not understand the question. this is not what BUILD_BUG_ON
> checks.

So this is the issue. Originally, the code checked whether the value of key_sz
was less than NETDEV_RSS_KEY_LEN. However, switching to a type_max check means
it no longer covers the scenario I described. Therefore, I think this is
unreasonable.

Thanks

>
> > > the code makes assumptions but it documents them and not
> > > just documents them, build will fail if they are violated.
> >
> > About this, I am ok.
> >
> > Thanks.
> >
> >
> > >
> > >
> > > > >
> > > > > >
> > > > > > Moreover, you should add a cover letter.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >  	}
> > > > > > >
> > > > > > >  	return 0;
> > > > > > > --
> > > > > > > 2.25.1
> > > > > > >
> > > > >
> > >
>

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 05:52:29PM +0800, Xuan Zhuo wrote:
> On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > > > > > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > > > > > > redundant runtime max check.
> > > > > > > >
> > > > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > > > ---
> > > > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > > > index eeefe8abc122..768ad5523dfa 100644
> > > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > > >  		}
> > > > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > > -			dev_warn(&vdev->dev,
> > > > > > > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > > > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > -		}
> > > > > > > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> > > > > > >
> > > > > > > Do we really need this check?
> > > > > > >
> > > > > > > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > > > > > > key_sz is of type u8, its maximum value is inherently 255, making this check
> > > > > > > redundant. This is not only limited by this kernel code, the virtio-net spec
> > > > > > > defines this.
> > > > > >
> > > > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > > > >
> > > > > > We never *need* BUILD_BUG_ON by definition, what this does is
> > > > > > document the assumption.
> > > > > >
> > > > > >
> > > > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > > > > > > the future, this check would no longer enforce the intended limit correctly.
> > > > > >
> > > > > > then it would fail build.
> > > > >
> > > > > So, does this mean we don't need to account for the case where
> > > > > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?
> > > > >
> > > > > Thanks.
> > > > >
> > > >
> > > > yes.
> > >
> > > Why?
> > >
> > > If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of 64, does this
> > > violate the spec?
> >
> > not the value of key_sz. If type of key_sz
> >
> >
> > i actually do not understand the question. this is not what BUILD_BUG_ON
> > checks.
> 
> So this is the issue. Originally, the code checked whether the value of key_sz
> was less than NETDEV_RSS_KEY_LEN.


No, and the check was plain wrong:

-               if (key_sz > NETDEV_RSS_KEY_LEN) {
-

so if device supports a larger key then .... we disable the feature
completely? Silly.


But the issue is present in net just the same.

So I think the commit log should be rewritten to explain the real issue,
and target net, not net-next.

> However, switching to a type_max check means
> it no longer covers the scenario I described. Therefore, I think this is
> unreasonable.
> 
> Thanks




> >
> > > > the code makes assumptions but it documents them and not
> > > > just documents them, build will fail if they are violated.
> > >
> > > About this, I am ok.
> > >
> > > Thanks.
> > >
> > >
> > > >
> > > >
> > > > > >
> > > > > > >
> > > > > > > Moreover, you should add a cover letter.
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >  	}
> > > > > > > >
> > > > > > > >  	return 0;
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > >
> > > > > >
> > > >
> >

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

[Michael S. Tsirkin]

On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> 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.

> 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.
> ---
>  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,
> -		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)
>  		__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",
> +				 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);
> +		}
> +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> +			dev_warn(&vdev->dev,
> +				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> +				 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?


So if device is powerful and supports a very big key size then...
we disable the feature? how does this make sense?


> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	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));
> -		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",
> -				vi->rss_key_size, VIRTIO_NET_RSS_MAX_KEY_SIZE);
> -			err = -EINVAL;
> -			goto free;
> -		}
> -
>  		vi->rss_hash_types_supported =
>  		    virtio_cread32(vdev, offsetof(struct virtio_net_config, supported_hash_types));
>  		vi->rss_hash_types_supported &=
> -- 
> 2.25.1

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 05:52:29PM +0800, Xuan Zhuo wrote:
> On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa <schalla@marvell.com> wrote:
> > > > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > > > > > > > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > > > > > > > redundant runtime max check.
> > > > > > > >
> > > > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > > > ---
> > > > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > > > > > > index eeefe8abc122..768ad5523dfa 100644
> > > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> > > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > > >  		}
> > > > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > > -			dev_warn(&vdev->dev,
> > > > > > > > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > > > > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > -		}
> > > > > > > > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> > > > > > >
> > > > > > > Do we really need this check?
> > > > > > >
> > > > > > > If I understand correctly, the intention is to cap key_sz at 256. However, since
> > > > > > > key_sz is of type u8, its maximum value is inherently 255, making this check
> > > > > > > redundant. This is not only limited by this kernel code, the virtio-net spec
> > > > > > > defines this.
> > > > > >
> > > > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > > > >
> > > > > > We never *need* BUILD_BUG_ON by definition, what this does is
> > > > > > document the assumption.
> > > > > >
> > > > > >
> > > > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value smaller than 256 in
> > > > > > > the future, this check would no longer enforce the intended limit correctly.
> > > > > >
> > > > > > then it would fail build.
> > > > >
> > > > > So, does this mean we don't need to account for the case where
> > > > > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is 64?
> > > > >
> > > > > Thanks.
> > > > >
> > > >
> > > > yes.
> > >
> > > Why?
> > >
> > > If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of 64, does this
> > > violate the spec?
> >
> > not the value of key_sz. If type of key_sz
> >
> >
> > i actually do not understand the question. this is not what BUILD_BUG_ON
> > checks.
> 
> So this is the issue. Originally, the code checked whether the value of key_sz
> was less than NETDEV_RSS_KEY_LEN. However, switching to a type_max check means
> it no longer covers the scenario I described. Therefore, I think this is
> unreasonable.
> 
> Thanks


patch 1 is unreasonable i think.

which is why patchsets should have a cover letter btw so
one can reply to just patch 1.


> >
> > > > the code makes assumptions but it documents them and not
> > > > just documents them, build will fail if they are violated.
> > >
> > > About this, I am ok.
> > >
> > > Thanks.
> > >
> > >
> > > >
> > > >
> > > > > >
> > > > > > >
> > > > > > > Moreover, you should add a cover letter.
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >  	}
> > > > > > > >
> > > > > > > >  	return 0;
> > > > > > > > --
> > > > > > > > 2.25.1
> > > > > > > >
> > > > > >
> > > >
> >

RE: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Srujana Challa]

> -----Original Message-----
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, February 25, 2026 3:35 PM
> To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> Cc: Srujana Challa <schalla@marvell.com>; pabeni@redhat.com;
> jasowang@redhat.com; eperezma@redhat.com; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Shiva Shankar Kommula
> <kshankar@marvell.com>; netdev@vger.kernel.org;
> virtualization@lists.linux.dev
> Subject: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size
> max check with BUILD_BUG_ON
> 
> On Wed, Feb 25, 2026 at 05: 52: 29PM +0800, Xuan Zhuo wrote: > On Wed,
> 25 Feb 2026 04: 47: 22 -0500, "Michael S. Tsirkin" <mst@ redhat. com> wrote:
> > > On Wed, Feb 25, 2026 at 05: 36: 06PM +0800, Xuan Zhuo wrote: > > > On
> Wed, ZjQcmQRYFpfptBannerStart Prioritize security for external emails:
> Confirm sender and content safety before clicking links or opening
> attachments <https://us-phishalarm-
> ewt.proofpoint.com/EWT/v1/CRVmXkqW!tc3Z1f8UYnWatK-
> 8Gd3a7iLAAhjzwNasdLDRrDCP-M_xUEAECz9j2fwxWlWq3EugFfzY8uKAhQ-
> Q2XIdQs89ldnlzrpsl9A$>
> Report Suspicious
> 
> ZjQcmQRYFpfptBannerEnd
> On Wed, Feb 25, 2026 at 05:52:29PM +0800, Xuan Zhuo wrote:
> > On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin"
> <mst@redhat.com> wrote:
> > > On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > > > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin"
> <mst@redhat.com> wrote:
> > > > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin"
> <mst@redhat.com> wrote:
> > > > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa
> <schalla@marvell.com> wrote:
> > > > > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in
> > > > > > > > > net-next, use BUILD_BUG_ON to enforce the limit at
> > > > > > > > > compile time and remove the redundant runtime max check.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/net/virtio_net.c
> > > > > > > > > b/drivers/net/virtio_net.c index
> > > > > > > > > eeefe8abc122..768ad5523dfa 100644
> > > > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct
> virtio_device *vdev)
> > > > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > > >  			__virtio_clear_bit(vdev,
> VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > >  		}
> > > > > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > > > -			dev_warn(&vdev->dev,
> > > > > > > > > -				 "rss_max_key_size=%u exceeds driver
> limit %u, disabling RSS\n",
> > > > > > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > > > -			__virtio_clear_bit(vdev,
> VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > -		}
> > > > > > > > > +		BUILD_BUG_ON(type_max(key_sz) >=
> NETDEV_RSS_KEY_LEN);
> > > > > > > >
> > > > > > > > Do we really need this check?
> > > > > > > >
> > > > > > > > If I understand correctly, the intention is to cap key_sz
> > > > > > > > at 256. However, since key_sz is of type u8, its maximum
> > > > > > > > value is inherently 255, making this check redundant. This
> > > > > > > > is not only limited by this kernel code, the virtio-net spec defines
> this.
> > > > > > >
> > > > > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > > > > >
> > > > > > > We never *need* BUILD_BUG_ON by definition, what this does
> > > > > > > is document the assumption.
> > > > > > >
> > > > > > >
> > > > > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value
> > > > > > > > smaller than 256 in the future, this check would no longer enforce
> the intended limit correctly.
> > > > > > >
> > > > > > > then it would fail build.
> > > > > >
> > > > > > So, does this mean we don't need to account for the case where
> > > > > > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is
> 64?
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > >
> > > > > yes.
> > > >
> > > > Why?
> > > >
> > > > If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of
> > > > 64, does this violate the spec?
> > >
> > > not the value of key_sz. If type of key_sz
> > >
> > >
> > > i actually do not understand the question. this is not what
> > > BUILD_BUG_ON checks.
> >
> > So this is the issue. Originally, the code checked whether the value
> > of key_sz was less than NETDEV_RSS_KEY_LEN. However, switching to a
> > type_max check means it no longer covers the scenario I described.
> > Therefore, I think this is unreasonable.
> >
> > Thanks
> 
> 
> patch 1 is unreasonable i think.

Patch 1 is targeted for net, addressing an issue where VIRTIO_NET_RSS_MAX_KEY_SIZE is fixed at 40, 
which is only the minimum required by the spec. This led to virtio‑net probe failures when devices
reported an RSS key size greater than 40. However, the driver also cannot handle keys larger than
NETDEV_RSS_KEY_LEN (previously 52) due to the BUG_ON(len > sizeof(netdev_rss_key)) in 
netdev_rss_key_fill. To resolve both issues, VIRTIO_NET_RSS_MAX_KEY_SIZE has been replaced
with NETDEV_RSS_KEY_LEN.

Patch 2 is added as per your suggestion to address following warning in net-next after  NETDEV_RSS_KEY_LEN
was increased to 256.
+../drivers/net/virtio_net.c:6642:14: warning: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
+ 6642 |                 if (key_sz > NETDEV_RSS_KEY_LEN) {
+      |                     ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~
+1 warning generated.
Sorry, I forgot the cover page.

Thanks! 
> 
> which is why patchsets should have a cover letter btw so one can reply to just
> patch 1.
> 
> 
> > >
> > > > > the code makes assumptions but it documents them and not just
> > > > > documents them, build will fail if they are violated.
> > > >
> > > > About this, I am ok.
> > > >
> > > > Thanks.
> > > >
> > > >
> > > > >
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Moreover, you should add a cover letter.
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >  	}
> > > > > > > > >
> > > > > > > > >  	return 0;
> > > > > > > > > --
> > > > > > > > > 2.25.1
> > > > > > > > >
> > > > > > >
> > > > >
> > >

Re: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 12:13:01PM +0000, Srujana Challa wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, February 25, 2026 3:35 PM
> > To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Cc: Srujana Challa <schalla@marvell.com>; pabeni@redhat.com;
> > jasowang@redhat.com; eperezma@redhat.com; davem@davemloft.net;
> > edumazet@google.com; kuba@kernel.org; Nithin Kumar Dabilpuram
> > <ndabilpuram@marvell.com>; Shiva Shankar Kommula
> > <kshankar@marvell.com>; netdev@vger.kernel.org;
> > virtualization@lists.linux.dev
> > Subject: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size
> > max check with BUILD_BUG_ON
> > 
> > On Wed, Feb 25, 2026 at 05: 52: 29PM +0800, Xuan Zhuo wrote: > On Wed,
> > 25 Feb 2026 04: 47: 22 -0500, "Michael S. Tsirkin" <mst@ redhat. com> wrote:
> > > > On Wed, Feb 25, 2026 at 05: 36: 06PM +0800, Xuan Zhuo wrote: > > > On
> > Wed, ZjQcmQRYFpfptBannerStart Prioritize security for external emails:
> > Confirm sender and content safety before clicking links or opening
> > attachments <https://us-phishalarm-
> > ewt.proofpoint.com/EWT/v1/CRVmXkqW!tc3Z1f8UYnWatK-
> > 8Gd3a7iLAAhjzwNasdLDRrDCP-M_xUEAECz9j2fwxWlWq3EugFfzY8uKAhQ-
> > Q2XIdQs89ldnlzrpsl9A$>
> > Report Suspicious
> > 
> > ZjQcmQRYFpfptBannerEnd
> > On Wed, Feb 25, 2026 at 05:52:29PM +0800, Xuan Zhuo wrote:
> > > On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin"
> > <mst@redhat.com> wrote:
> > > > On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > > > > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin"
> > <mst@redhat.com> wrote:
> > > > > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin"
> > <mst@redhat.com> wrote:
> > > > > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa
> > <schalla@marvell.com> wrote:
> > > > > > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in
> > > > > > > > > > net-next, use BUILD_BUG_ON to enforce the limit at
> > > > > > > > > > compile time and remove the redundant runtime max check.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/net/virtio_net.c
> > > > > > > > > > b/drivers/net/virtio_net.c index
> > > > > > > > > > eeefe8abc122..768ad5523dfa 100644
> > > > > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > > > > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct
> > virtio_device *vdev)
> > > > > > > > > >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > > > >  			__virtio_clear_bit(vdev,
> > VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > >  		}
> > > > > > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > > > > -			dev_warn(&vdev->dev,
> > > > > > > > > > -				 "rss_max_key_size=%u exceeds driver
> > limit %u, disabling RSS\n",
> > > > > > > > > > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > > > > > > > > > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > > > > > > > > > -			__virtio_clear_bit(vdev,
> > VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > > -		}
> > > > > > > > > > +		BUILD_BUG_ON(type_max(key_sz) >=
> > NETDEV_RSS_KEY_LEN);
> > > > > > > > >
> > > > > > > > > Do we really need this check?
> > > > > > > > >
> > > > > > > > > If I understand correctly, the intention is to cap key_sz
> > > > > > > > > at 256. However, since key_sz is of type u8, its maximum
> > > > > > > > > value is inherently 255, making this check redundant. This
> > > > > > > > > is not only limited by this kernel code, the virtio-net spec defines
> > this.
> > > > > > > >
> > > > > > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > > > > > >
> > > > > > > > We never *need* BUILD_BUG_ON by definition, what this does
> > > > > > > > is document the assumption.
> > > > > > > >
> > > > > > > >
> > > > > > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a value
> > > > > > > > > smaller than 256 in the future, this check would no longer enforce
> > the intended limit correctly.
> > > > > > > >
> > > > > > > > then it would fail build.
> > > > > > >
> > > > > > > So, does this mean we don't need to account for the case where
> > > > > > > NETDEV_RSS_KEY_LEN is 128, but the key_sz reported by the device is
> > 64?
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > >
> > > > > > yes.
> > > > >
> > > > > Why?
> > > > >
> > > > > If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz of
> > > > > 64, does this violate the spec?
> > > >
> > > > not the value of key_sz. If type of key_sz
> > > >
> > > >
> > > > i actually do not understand the question. this is not what
> > > > BUILD_BUG_ON checks.
> > >
> > > So this is the issue. Originally, the code checked whether the value
> > > of key_sz was less than NETDEV_RSS_KEY_LEN. However, switching to a
> > > type_max check means it no longer covers the scenario I described.
> > > Therefore, I think this is unreasonable.
> > >
> > > Thanks
> > 
> > 
> > patch 1 is unreasonable i think.
> 
> Patch 1 is targeted for net, addressing an issue where VIRTIO_NET_RSS_MAX_KEY_SIZE is fixed at 40, 
> which is only the minimum required by the spec. This led to virtio‑net probe failures when devices
> reported an RSS key size greater than 40. However, the driver also cannot handle keys larger than
> NETDEV_RSS_KEY_LEN (previously 52) due to the BUG_ON(len > sizeof(netdev_rss_key)) in 
> netdev_rss_key_fill. To resolve both issues, VIRTIO_NET_RSS_MAX_KEY_SIZE has been replaced
> with NETDEV_RSS_KEY_LEN.

but where would driver *get* keys larger than sizeof(netdev_rss_key),
even if device supports more.



> Patch 2 is added as per your suggestion to address following warning in net-next after  NETDEV_RSS_KEY_LEN
> was increased to 256.
> +../drivers/net/virtio_net.c:6642:14: warning: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
> + 6642 |                 if (key_sz > NETDEV_RSS_KEY_LEN) {
> +      |                     ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~
> +1 warning generated.
> Sorry, I forgot the cover page.
> 
> Thanks! 
> > 
> > which is why patchsets should have a cover letter btw so one can reply to just
> > patch 1.
> > 
> > 
> > > >
> > > > > > the code makes assumptions but it documents them and not just
> > > > > > documents them, build will fail if they are violated.
> > > > >
> > > > > About this, I am ok.
> > > > >
> > > > > Thanks.
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Moreover, you should add a cover letter.
> > > > > > > > >
> > > > > > > > > Thanks.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >  	}
> > > > > > > > > >
> > > > > > > > > >  	return 0;
> > > > > > > > > > --
> > > > > > > > > > 2.25.1
> > > > > > > > > >
> > > > > > > >
> > > > > >
> > > >
> 

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

[Srujana Challa]

> On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> > 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.
> 
> > 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.
> > ---
> >  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,
> > -		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)
> >  		__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",
> > +				 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);
> > +		}
> > +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > +			dev_warn(&vdev->dev,
> > +				 "rss_max_key_size=%u exceeds driver limit
> %u, disabling RSS\n",
> > +				 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.
> 
> 
> 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:
BUG_ON(len > sizeof(netdev_rss_key));

> 
> 
> > +		}
> > +	}
> > +
> >  	return 0;
> >  }
> >
> > @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct virtio_device
> *vdev)
> >  	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));
> > -		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",
> > -				vi->rss_key_size,
> VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > -			err = -EINVAL;
> > -			goto free;
> > -		}
> > -
> >  		vi->rss_hash_types_supported =
> >  		    virtio_cread32(vdev, offsetof(struct virtio_net_config,
> supported_hash_types));
> >  		vi->rss_hash_types_supported &=
> > --
> > 2.25.1

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

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 12:22:50PM +0000, Srujana Challa wrote:
> > On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> > > 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.
> > 
> > > 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.
> > > ---
> > >  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,
> > > -		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)
> > >  		__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",
> > > +				 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);
> > > +		}
> > > +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > +			dev_warn(&vdev->dev,
> > > +				 "rss_max_key_size=%u exceeds driver limit
> > %u, disabling RSS\n",
> > > +				 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.
> > 
> > 
> > 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:
> BUG_ON(len > sizeof(netdev_rss_key));

so cap it to NETDEV_RSS_KEY_LEN. Why is that a reason to clear the feature?

> > 
> > 
> > > +		}
> > > +	}
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct virtio_device
> > *vdev)
> > >  	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));
> > > -		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",
> > > -				vi->rss_key_size,
> > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > -			err = -EINVAL;
> > > -			goto free;
> > > -		}
> > > -
> > >  		vi->rss_hash_types_supported =
> > >  		    virtio_cread32(vdev, offsetof(struct virtio_net_config,
> > supported_hash_types));
> > >  		vi->rss_hash_types_supported &=
> > > --
> > > 2.25.1
> 

RE: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Srujana Challa]

> > > ZjQcmQRYFpfptBannerEnd
> > > On Wed, Feb 25, 2026 at 05:52:29PM +0800, Xuan Zhuo wrote:
> > > > On Wed, 25 Feb 2026 04:47:22 -0500, "Michael S. Tsirkin"
> > > <mst@redhat.com> wrote:
> > > > > On Wed, Feb 25, 2026 at 05:36:06PM +0800, Xuan Zhuo wrote:
> > > > > > On Wed, 25 Feb 2026 04:33:57 -0500, "Michael S. Tsirkin"
> > > <mst@redhat.com> wrote:
> > > > > > > On Wed, Feb 25, 2026 at 05:30:33PM +0800, Xuan Zhuo wrote:
> > > > > > > > On Wed, 25 Feb 2026 04:24:14 -0500, "Michael S. Tsirkin"
> > > <mst@redhat.com> wrote:
> > > > > > > > > On Wed, Feb 25, 2026 at 05:11:42PM +0800, Xuan Zhuo wrote:
> > > > > > > > > > On Tue, 24 Feb 2026 12:28:50 +0530, Srujana Challa
> > > <schalla@marvell.com> wrote:
> > > > > > > > > > > Since NETDEV_RSS_KEY_LEN was increased to 256 in
> > > > > > > > > > > net-next, use BUILD_BUG_ON to enforce the limit at
> > > > > > > > > > > compile time and remove the redundant runtime max check.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > > > > > > > > > > ---
> > > > > > > > > > >  drivers/net/virtio_net.c | 8 +-------
> > > > > > > > > > >  1 file changed, 1 insertion(+), 7 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/drivers/net/virtio_net.c
> > > > > > > > > > > b/drivers/net/virtio_net.c index
> > > > > > > > > > > eeefe8abc122..768ad5523dfa 100644
> > > > > > > > > > > --- a/drivers/net/virtio_net.c
> > > > > > > > > > > +++ b/drivers/net/virtio_net.c
> > > > > > > > > > > @@ -6639,13 +6639,7 @@ static int
> > > > > > > > > > > virtnet_validate(struct
> > > virtio_device *vdev)
> > > > > > > > > > >  			__virtio_clear_bit(vdev,
> VIRTIO_NET_F_RSS);
> > > > > > > > > > >  			__virtio_clear_bit(vdev,
> > > VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > > >  		}
> > > > > > > > > > > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > > > > > -			dev_warn(&vdev->dev,
> > > > > > > > > > > -				 "rss_max_key_size=%u
> exceeds driver
> > > limit %u, disabling RSS\n",
> > > > > > > > > > > -				 key_sz,
> NETDEV_RSS_KEY_LEN);
> > > > > > > > > > > -			__virtio_clear_bit(vdev,
> VIRTIO_NET_F_RSS);
> > > > > > > > > > > -			__virtio_clear_bit(vdev,
> > > VIRTIO_NET_F_HASH_REPORT);
> > > > > > > > > > > -		}
> > > > > > > > > > > +		BUILD_BUG_ON(type_max(key_sz) >=
> > > NETDEV_RSS_KEY_LEN);
> > > > > > > > > >
> > > > > > > > > > Do we really need this check?
> > > > > > > > > >
> > > > > > > > > > If I understand correctly, the intention is to cap
> > > > > > > > > > key_sz at 256. However, since key_sz is of type u8,
> > > > > > > > > > its maximum value is inherently 255, making this check
> > > > > > > > > > redundant. This is not only limited by this kernel
> > > > > > > > > > code, the virtio-net spec defines
> > > this.
> > > > > > > > >
> > > > > > > > > That's why it's BUILD_BUG_ON. It checks it has the right type.
> > > > > > > > >
> > > > > > > > > We never *need* BUILD_BUG_ON by definition, what this
> > > > > > > > > does is document the assumption.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > Moreover, if NETDEV_RSS_KEY_LEN is ever reduced to a
> > > > > > > > > > value smaller than 256 in the future, this check would
> > > > > > > > > > no longer enforce
> > > the intended limit correctly.
> > > > > > > > >
> > > > > > > > > then it would fail build.
> > > > > > > >
> > > > > > > > So, does this mean we don't need to account for the case
> > > > > > > > where NETDEV_RSS_KEY_LEN is 128, but the key_sz reported
> > > > > > > > by the device is
> > > 64?
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > > >
> > > > > > >
> > > > > > > yes.
> > > > > >
> > > > > > Why?
> > > > > >
> > > > > > If NETDEV_RSS_KEY_LEN is 128 but the device reports a key_sz
> > > > > > of 64, does this violate the spec?
> > > > >
> > > > > not the value of key_sz. If type of key_sz
> > > > >
> > > > >
> > > > > i actually do not understand the question. this is not what
> > > > > BUILD_BUG_ON checks.
> > > >
> > > > So this is the issue. Originally, the code checked whether the
> > > > value of key_sz was less than NETDEV_RSS_KEY_LEN. However,
> > > > switching to a type_max check means it no longer covers the scenario I
> described.
> > > > Therefore, I think this is unreasonable.
> > > >
> > > > Thanks
> > >
> > >
> > > patch 1 is unreasonable i think.
> >
> > Patch 1 is targeted for net, addressing an issue where
> > VIRTIO_NET_RSS_MAX_KEY_SIZE is fixed at 40, which is only the minimum
> > required by the spec. This led to virtio‑net probe failures when
> > devices reported an RSS key size greater than 40. However, the driver
> > also cannot handle keys larger than NETDEV_RSS_KEY_LEN (previously 52)
> > due to the BUG_ON(len > sizeof(netdev_rss_key)) in netdev_rss_key_fill. To
> resolve both issues, VIRTIO_NET_RSS_MAX_KEY_SIZE has been replaced with
> NETDEV_RSS_KEY_LEN.
> 
> but where would driver *get* keys larger than sizeof(netdev_rss_key), even if
> device supports more.
In virtio‑net, we rely on netdev_rss_key_fill() during virtnet_init_default_rss() to populate the RSS hash key,
which uses device supported length.
The code path is:
static void virtnet_init_default_rss(struct virtnet_info *vi)
{
        vi->rss_hdr->hash_types = cpu_to_le32(vi->rss_hash_types_supported);
        vi->rss_hash_types_saved = vi->rss_hash_types_supported;
        vi->rss_hdr->indirection_table_mask = vi->rss_indir_table_size
                                                ? cpu_to_le16(vi->rss_indir_table_size - 1) : 0;
        vi->rss_hdr->unclassified_queue = 0;

        virtnet_rss_update_by_qpairs(vi, vi->curr_queue_pairs);

        vi->rss_trailer.hash_key_length = vi->rss_key_size;

        netdev_rss_key_fill(vi->rss_hash_key_data, vi->rss_key_size);
}
> 
> 
> 
> > Patch 2 is added as per your suggestion to address following warning
> > in net-next after  NETDEV_RSS_KEY_LEN was increased to 256.
> > +../drivers/net/virtio_net.c:6642:14: warning: result of comparison of
> constant 256 with expression of type 'u8' (aka 'unsigned char') is always false
> [-Wtautological-constant-out-of-range-compare]
> > + 6642 |                 if (key_sz > NETDEV_RSS_KEY_LEN) {
> > +      |                     ~~~~~~ ^ ~~~~~~~~~~~~~~~~~~
> > +1 warning generated.
> > Sorry, I forgot the cover page.
> >
> > Thanks!
> > >
> > > which is why patchsets should have a cover letter btw so one can
> > > reply to just patch 1.
> > >
> > >
> > > > >
> > > > > > > the code makes assumptions but it documents them and not
> > > > > > > just documents them, build will fail if they are violated.
> > > > > >
> > > > > > About this, I am ok.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Moreover, you should add a cover letter.
> > > > > > > > > >
> > > > > > > > > > Thanks.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > >  	}
> > > > > > > > > > >
> > > > > > > > > > >  	return 0;
> > > > > > > > > > > --
> > > > > > > > > > > 2.25.1
> > > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > >
> >

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

[Srujana Challa]

> > > On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> > > > 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.
> > >
> > > > 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.
> > > > ---
> > > >  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,
> > > > -		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)
> > > >  		__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",
> > > > +				 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);
> > > > +		}
> > > > +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > +			dev_warn(&vdev->dev,
> > > > +				 "rss_max_key_size=%u exceeds driver limit
> > > %u, disabling RSS\n",
> > > > +				 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.
> > >
> > >
> > > 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:
> > 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.
> 
> > >
> > >
> > > > +		}
> > > > +	}
> > > > +
> > > >  	return 0;
> > > >  }
> > > >
> > > > @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct
> > > > virtio_device
> > > *vdev)
> > > >  	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));
> > > > -		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",
> > > > -				vi->rss_key_size,
> > > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > > -			err = -EINVAL;
> > > > -			goto free;
> > > > -		}
> > > > -
> > > >  		vi->rss_hash_types_supported =
> > > >  		    virtio_cread32(vdev, offsetof(struct virtio_net_config,
> > > supported_hash_types));
> > > >  		vi->rss_hash_types_supported &=
> > > > --
> > > > 2.25.1
> >

Re: [EXTERNAL] Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 12:29:41PM +0000, Srujana Challa wrote:
> > > > patch 1 is unreasonable i think.
> > >
> > > Patch 1 is targeted for net, addressing an issue where
> > > VIRTIO_NET_RSS_MAX_KEY_SIZE is fixed at 40, which is only the minimum
> > > required by the spec. This led to virtio‑net probe failures when
> > > devices reported an RSS key size greater than 40. However, the driver
> > > also cannot handle keys larger than NETDEV_RSS_KEY_LEN (previously 52)
> > > due to the BUG_ON(len > sizeof(netdev_rss_key)) in netdev_rss_key_fill. To
> > resolve both issues, VIRTIO_NET_RSS_MAX_KEY_SIZE has been replaced with
> > NETDEV_RSS_KEY_LEN.
> > 
> > but where would driver *get* keys larger than sizeof(netdev_rss_key), even if
> > device supports more.
> In virtio‑net, we rely on netdev_rss_key_fill() during virtnet_init_default_rss() to populate the RSS hash key,
> which uses device supported length.
> The code path is:
> static void virtnet_init_default_rss(struct virtnet_info *vi)
> {
>         vi->rss_hdr->hash_types = cpu_to_le32(vi->rss_hash_types_supported);
>         vi->rss_hash_types_saved = vi->rss_hash_types_supported;
>         vi->rss_hdr->indirection_table_mask = vi->rss_indir_table_size
>                                                 ? cpu_to_le16(vi->rss_indir_table_size - 1) : 0;
>         vi->rss_hdr->unclassified_queue = 0;
> 
>         virtnet_rss_update_by_qpairs(vi, vi->curr_queue_pairs);
> 
>         vi->rss_trailer.hash_key_length = vi->rss_key_size;
> 
>         netdev_rss_key_fill(vi->rss_hash_key_data, vi->rss_key_size);
> }


Is there a point you are trying to make? disabling features because user
bought a stronger device is counter intuitive. Make it work
in a sane way pls.

-- 
MST

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

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 12:34:28PM +0000, Srujana Challa wrote:
> > > > On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> > > > > 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.
> > > >
> > > > > 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.
> > > > > ---
> > > > >  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,
> > > > > -		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)
> > > > >  		__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",
> > > > > +				 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);
> > > > > +		}
> > > > > +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > +			dev_warn(&vdev->dev,
> > > > > +				 "rss_max_key_size=%u exceeds driver limit
> > > > %u, disabling RSS\n",
> > > > > +				 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.
> > > >
> > > >
> > > > 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:
> > > 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.


> > 
> > > >
> > > >
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > >  	return 0;
> > > > >  }
> > > > >
> > > > > @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct
> > > > > virtio_device
> > > > *vdev)
> > > > >  	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));
> > > > > -		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",
> > > > > -				vi->rss_key_size,
> > > > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > > > -			err = -EINVAL;
> > > > > -			goto free;
> > > > > -		}
> > > > > -
> > > > >  		vi->rss_hash_types_supported =
> > > > >  		    virtio_cread32(vdev, offsetof(struct virtio_net_config,
> > > > supported_hash_types));
> > > > >  		vi->rss_hash_types_supported &=
> > > > > --
> > > > > 2.25.1
> > >
> 

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

[Srujana Challa]

> -----Original Message-----
> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, February 25, 2026 6:07 PM
> To: Srujana Challa <schalla@marvell.com>
> Cc: netdev@vger.kernel.org; virtualization@lists.linux.dev;
> pabeni@redhat.com; jasowang@redhat.com; xuanzhuo@linux.alibaba.com;
> eperezma@redhat.com; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>;
> Shiva Shankar Kommula <kshankar@marvell.com>; stable@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key
> size validation and use NETDEV_RSS_KEY_LEN
> 
> On Wed, Feb 25, 2026 at 12: 34: 28PM +0000, Srujana Challa wrote: > > > >
> On Tue, Feb 24, 2026 at 12: 28: 49PM +0530, Srujana Challa wrote: > > > > >
> Replace hardcoded RSS max key size limit with NETDEV_RSS_KEY_LEN >
> ZjQcmQRYFpfptBannerStart Prioritize security for external emails:
> Confirm sender and content safety before clicking links or opening
> attachments <https://us-phishalarm-
> ewt.proofpoint.com/EWT/v1/CRVmXkqW!tc3Z1f8UYnWatK-
> 8Wb36Dpr9FJXZMBwEugHj1xCGwRl-
> dNXM_I8Yk7hbbjwCHe9WhgQwmGx2Ms85fIkSmKM2dBQeH9Dkzak$>
> Report Suspicious
> 
> ZjQcmQRYFpfptBannerEnd
> On Wed, Feb 25, 2026 at 12:34:28PM +0000, Srujana Challa wrote:
> > > > > On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> > > > > > 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.
> > > > >
> > > > > > 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.
> > > > > > ---
> > > > > >  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,
> > > > > > -		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)
> > > > > >  		__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",
> > > > > > +				 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);
> > > > > > +		}
> > > > > > +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > +			dev_warn(&vdev->dev,
> > > > > > +				 "rss_max_key_size=%u exceeds driver
> limit
> > > > > %u, disabling RSS\n",
> > > > > > +				 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.
> > > > >
> > > > >
> > > > > 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:
> > > > 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.
My understanding is that this patch prevents the probe from failing by disabling the feature instead.
Given the current implementation, the driver becomes unusable when this condition is hit.
> 
> 
> > >
> > > > >
> > > > >
> > > > > > +		}
> > > > > > +	}
> > > > > > +
> > > > > >  	return 0;
> > > > > >  }
> > > > > >
> > > > > > @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct
> > > > > > virtio_device
> > > > > *vdev)
> > > > > >  	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));
> > > > > > -		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",
> > > > > > -				vi->rss_key_size,
> > > > > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > > > > -			err = -EINVAL;
> > > > > > -			goto free;
> > > > > > -		}
> > > > > > -
> > > > > >  		vi->rss_hash_types_supported =
> > > > > >  		    virtio_cread32(vdev, offsetof(struct
> virtio_net_config,
> > > > > supported_hash_types));
> > > > > >  		vi->rss_hash_types_supported &=
> > > > > > --
> > > > > > 2.25.1
> > > >
> >

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

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 12:47:02PM +0000, Srujana Challa wrote:
> > 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.
> My understanding is that this patch prevents the probe from failing by disabling the feature instead.
> Given the current implementation, the driver becomes unusable when this condition is hit.

current implementation even more broken. but the real fix is to
intepret max key size as max.

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

[Srujana Challa]

> > -----Original Message-----
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Wednesday, February 25, 2026 6:07 PM
> > To: Srujana Challa <schalla@marvell.com>
> > Cc: netdev@vger.kernel.org; virtualization@lists.linux.dev;
> > pabeni@redhat.com; jasowang@redhat.com; xuanzhuo@linux.alibaba.com;
> > eperezma@redhat.com; davem@davemloft.net; edumazet@google.com;
> > kuba@kernel.org; Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>;
> > Shiva Shankar Kommula <kshankar@marvell.com>; stable@vger.kernel.org
> > Subject: Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS
> > key size validation and use NETDEV_RSS_KEY_LEN
> >
> > On Wed, Feb 25, 2026 at 12: 34: 28PM +0000, Srujana Challa wrote: > >
> > > > On Tue, Feb 24, 2026 at 12: 28: 49PM +0530, Srujana Challa wrote:
> > > > > > > Replace hardcoded RSS max key size limit with
> > NETDEV_RSS_KEY_LEN > ZjQcmQRYFpfptBannerStart Prioritize security for
> external emails:
> > Confirm sender and content safety before clicking links or opening
> > attachments <https://us-phishalarm-
> > ewt.proofpoint.com/EWT/v1/CRVmXkqW!tc3Z1f8UYnWatK-
> > 8Wb36Dpr9FJXZMBwEugHj1xCGwRl-
> > dNXM_I8Yk7hbbjwCHe9WhgQwmGx2Ms85fIkSmKM2dBQeH9Dkzak$>
> > Report Suspicious
> >
> > ZjQcmQRYFpfptBannerEnd
> > On Wed, Feb 25, 2026 at 12:34:28PM +0000, Srujana Challa wrote:
> > > > > > On Tue, Feb 24, 2026 at 12:28:49PM +0530, Srujana Challa wrote:
> > > > > > > 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.
> > > > > >
> > > > > > > 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.
> > > > > > > ---
> > > > > > >  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,
> > > > > > > -		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)
> > > > > > >  		__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",
> > > > > > > +				 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);
> > > > > > > +		}
> > > > > > > +		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > > > > > > +			dev_warn(&vdev->dev,
> > > > > > > +				 "rss_max_key_size=%u exceeds driver
> > limit
> > > > > > %u, disabling RSS\n",
> > > > > > > +				 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.
> > > > > >
> > > > > >
> > > > > > 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:
> > > > > 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.
> My understanding is that this patch prevents the probe from failing by
> disabling the feature instead.
> Given the current implementation, the driver becomes unusable when this
> condition is hit.

I understand that the driver is allowed to use a smaller RSS key than the device’s advertised rss_max_key_size.
But, our hardware does not behave correctly in that configuration. For symmetric bidirectional hashing,
the device requires that the hash_key_length match rss_max_key_size exactly.
If the driver uses a smaller key, the hardware produces inconsistent hash values for forward vs reverse flows.
Because of this device requirement, we cannot cap the key to NETDEV_RSS_KEY_LEN when the device advertises
a larger rss_max_key_size.
> >
> >
> > > >
> > > > > >
> > > > > >
> > > > > > > +		}
> > > > > > > +	}
> > > > > > > +
> > > > > > >  	return 0;
> > > > > > >  }
> > > > > > >
> > > > > > > @@ -6839,13 +6860,6 @@ static int virtnet_probe(struct
> > > > > > > virtio_device
> > > > > > *vdev)
> > > > > > >  	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));
> > > > > > > -		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",
> > > > > > > -				vi->rss_key_size,
> > > > > > VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > > > > > > -			err = -EINVAL;
> > > > > > > -			goto free;
> > > > > > > -		}
> > > > > > > -
> > > > > > >  		vi->rss_hash_types_supported =
> > > > > > >  		    virtio_cread32(vdev, offsetof(struct
> > virtio_net_config,
> > > > > > supported_hash_types));
> > > > > > >  		vi->rss_hash_types_supported &=
> > > > > > > --
> > > > > > > 2.25.1
> > > > >
> > >

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

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 12:56:19PM +0000, Srujana Challa wrote:
> > > > > > >
> > > > > > >
> > > > > > > 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:
> > > > > > 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.
> > My understanding is that this patch prevents the probe from failing by
> > disabling the feature instead.
> > Given the current implementation, the driver becomes unusable when this
> > condition is hit.
> 
> I understand that the driver is allowed to use a smaller RSS key than the device’s advertised rss_max_key_size.
> But, our hardware does not behave correctly in that configuration. For symmetric bidirectional hashing,
> the device requires that the hash_key_length match rss_max_key_size exactly.
> If the driver uses a smaller key, the hardware produces inconsistent hash values for forward vs reverse flows.
> Because of this device requirement, we cannot cap the key to NETDEV_RSS_KEY_LEN when the device advertises
> a larger rss_max_key_size.

Would you not say it's a buggy device then?

-- 
MST

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

[Srujana Challa]

> On Wed, Feb 25, 2026 at 12:56:19PM +0000, Srujana Challa wrote:
> > > > > > > >
> > > > > > > >
> > > > > > > > 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:
> > > > > > > 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.
> > > My understanding is that this patch prevents the probe from failing
> > > by disabling the feature instead.
> > > Given the current implementation, the driver becomes unusable when
> > > this condition is hit.
> >
> > I understand that the driver is allowed to use a smaller RSS key than the
> device’s advertised rss_max_key_size.
> > But, our hardware does not behave correctly in that configuration. For
> > symmetric bidirectional hashing, the device requires that the
> hash_key_length match rss_max_key_size exactly.
> > If the driver uses a smaller key, the hardware produces inconsistent hash
> values for forward vs reverse flows.
> > Because of this device requirement, we cannot cap the key to
> > NETDEV_RSS_KEY_LEN when the device advertises a larger
> rss_max_key_size.
> 
> Would you not say it's a buggy device then?
No. The device works correctly when a smaller key is used. The limitation only affects
symmetric bidirectional hashing. For the other use cases capping the key size is fine.
> 
> --
> MST

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

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 01:31:50PM +0000, Srujana Challa wrote:
> > On Wed, Feb 25, 2026 at 12:56:19PM +0000, Srujana Challa wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 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:
> > > > > > > > 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.
> > > > My understanding is that this patch prevents the probe from failing
> > > > by disabling the feature instead.
> > > > Given the current implementation, the driver becomes unusable when
> > > > this condition is hit.
> > >
> > > I understand that the driver is allowed to use a smaller RSS key than the
> > device’s advertised rss_max_key_size.
> > > But, our hardware does not behave correctly in that configuration. For
> > > symmetric bidirectional hashing, the device requires that the
> > hash_key_length match rss_max_key_size exactly.
> > > If the driver uses a smaller key, the hardware produces inconsistent hash
> > values for forward vs reverse flows.
> > > Because of this device requirement, we cannot cap the key to
> > > NETDEV_RSS_KEY_LEN when the device advertises a larger
> > rss_max_key_size.
> > 
> > Would you not say it's a buggy device then?
> No. The device works correctly when a smaller key is used. The limitation only affects
> symmetric bidirectional hashing. For the other use cases capping the key size is fine.


yes the device seems buggy in that configuration, in that it has this
requirement which does not seem to be in the spec.

so tell me, what is the actual rss_max_key_size of that device?



> > 
> > --
> > MST
> 

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[David Laight]

On Tue, 24 Feb 2026 12:28:50 +0530
Srujana Challa <schalla@marvell.com> wrote:

> Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> BUILD_BUG_ON to enforce the limit at compile time and remove the
> redundant runtime max check.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>  drivers/net/virtio_net.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index eeefe8abc122..768ad5523dfa 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
>  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
>  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
>  		}
> -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> -			dev_warn(&vdev->dev,
> -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> -				 key_sz, NETDEV_RSS_KEY_LEN);
> -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> -		}
> +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);

type_max is horrid.
I did read the later discussion (but it has fallen out of my brain)
but isn't that check broken and/or backwards anyway?

I'd also question why you are using u8 for a local (I didn't find this
version of the file), it will generate worse code that [unsigned] int.

	David

>  	}
>  
>  	return 0;

Re: [PATCH net-next,2/2] virtio_net: replace RSS key size max check with BUILD_BUG_ON

[Michael S. Tsirkin]

On Wed, Feb 25, 2026 at 02:50:27PM +0000, David Laight wrote:
> On Tue, 24 Feb 2026 12:28:50 +0530
> Srujana Challa <schalla@marvell.com> wrote:
> 
> > Since NETDEV_RSS_KEY_LEN was increased to 256 in net-next, use
> > BUILD_BUG_ON to enforce the limit at compile time and remove the
> > redundant runtime max check.
> > 
> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> > ---
> >  drivers/net/virtio_net.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index eeefe8abc122..768ad5523dfa 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -6639,13 +6639,7 @@ static int virtnet_validate(struct virtio_device *vdev)
> >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> >  			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> >  		}
> > -		if (key_sz > NETDEV_RSS_KEY_LEN) {
> > -			dev_warn(&vdev->dev,
> > -				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> > -				 key_sz, NETDEV_RSS_KEY_LEN);
> > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_RSS);
> > -			__virtio_clear_bit(vdev, VIRTIO_NET_F_HASH_REPORT);
> > -		}
> > +		BUILD_BUG_ON(type_max(key_sz) >= NETDEV_RSS_KEY_LEN);
> 
> type_max is horrid.
> I did read the later discussion (but it has fallen out of my brain)
> but isn't that check broken and/or backwards anyway?

it is just trying to say "i check at build time that hardware
key size will never exceed NETDEV_RSS_KEY_LEN just because it's not
large enough".

> I'd also question why you are using u8 for a local (I didn't find this
> version of the file), it will generate worse code that [unsigned] int.

that's hardware format. but yes it should be config->max_key_size or
whatever it is.

> 	David
> 
> >  	}
> >  
> >  	return 0;

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

[Srujana Challa]

> ZjQcmQRYFpfptBannerEnd
> On Wed, Feb 25, 2026 at 01:31:50PM +0000, Srujana Challa wrote:
> > > On Wed, Feb 25, 2026 at 12:56:19PM +0000, Srujana Challa wrote:
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > 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:
> > > > > > > > > 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.
> > > > > My understanding is that this patch prevents the probe from
> > > > > failing by disabling the feature instead.
> > > > > Given the current implementation, the driver becomes unusable
> > > > > when this condition is hit.
> > > >
> > > > I understand that the driver is allowed to use a smaller RSS key
> > > > than the
> > > device’s advertised rss_max_key_size.
> > > > But, our hardware does not behave correctly in that configuration.
> > > > For symmetric bidirectional hashing, the device requires that the
> > > hash_key_length match rss_max_key_size exactly.
> > > > If the driver uses a smaller key, the hardware produces
> > > > inconsistent hash
> > > values for forward vs reverse flows.
> > > > Because of this device requirement, we cannot cap the key to
> > > > NETDEV_RSS_KEY_LEN when the device advertises a larger
> > > rss_max_key_size.
> > >
> > > Would you not say it's a buggy device then?
> > No. The device works correctly when a smaller key is used. The
> > limitation only affects symmetric bidirectional hashing. For the other use
> cases capping the key size is fine.
> 
> 
> yes the device seems buggy in that configuration, in that it has this
> requirement which does not seem to be in the spec.
> 
> so tell me, what is the actual rss_max_key_size of that device?
>
We've confirmed our device works with clamping the key size. Our device's rss_max_key_size is 48 bytes.
We will update the patch to use min(device rss_max_key_size, NETDEV_RSS_KEY_LEN) and drop the logic
that disables RSS when "device rss_max_key_size > NETDEV_RSS_KEY_LEN" . And also drop the patch2.
Will send a revised patch shortly.

Thanks!
> 
> 
> > >
> > > --
> > > MST
> >