public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
@ 2026-02-24  6:58 Srujana Challa
  2026-02-25 10:03 ` Michael S. Tsirkin
  0 siblings, 1 reply; 13+ messages in thread
From: Srujana Challa @ 2026-02-24  6:58 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: pabeni, mst, jasowang, xuanzhuo, eperezma, davem, edumazet, kuba,
	ndabilpuram, kshankar, schalla, stable

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


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

* Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-24  6:58 [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN Srujana Challa
@ 2026-02-25 10:03 ` Michael S. Tsirkin
  2026-02-25 12:22   ` [EXTERNAL] " Srujana Challa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-02-25 10:03 UTC (permalink / raw)
  To: Srujana Challa
  Cc: netdev, virtualization, pabeni, jasowang, xuanzhuo, eperezma,
	davem, edumazet, kuba, ndabilpuram, kshankar, stable

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


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

* RE: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 10:03 ` Michael S. Tsirkin
@ 2026-02-25 12:22   ` Srujana Challa
  2026-02-25 12:24     ` Michael S. Tsirkin
  0 siblings, 1 reply; 13+ messages in thread
From: Srujana Challa @ 2026-02-25 12:22 UTC (permalink / raw)
  To: Michael S. Tsirkin
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

> 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


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

* Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:22   ` [EXTERNAL] " Srujana Challa
@ 2026-02-25 12:24     ` Michael S. Tsirkin
  2026-02-25 12:34       ` Srujana Challa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-02-25 12:24 UTC (permalink / raw)
  To: Srujana Challa
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

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
> 


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

* RE: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:24     ` Michael S. Tsirkin
@ 2026-02-25 12:34       ` Srujana Challa
  2026-02-25 12:37         ` Michael S. Tsirkin
  0 siblings, 1 reply; 13+ messages in thread
From: Srujana Challa @ 2026-02-25 12:34 UTC (permalink / raw)
  To: Michael S. Tsirkin
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

> > > 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
> >


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

* Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:34       ` Srujana Challa
@ 2026-02-25 12:37         ` Michael S. Tsirkin
  2026-02-25 12:47           ` Srujana Challa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-02-25 12:37 UTC (permalink / raw)
  To: Srujana Challa
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

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
> > >
> 


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

* RE: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:37         ` Michael S. Tsirkin
@ 2026-02-25 12:47           ` Srujana Challa
  2026-02-25 12:52             ` Michael S. Tsirkin
  2026-02-25 12:56             ` Srujana Challa
  0 siblings, 2 replies; 13+ messages in thread
From: Srujana Challa @ 2026-02-25 12:47 UTC (permalink / raw)
  To: Michael S. Tsirkin
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org



> -----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
> > > >
> >


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

* Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:47           ` Srujana Challa
@ 2026-02-25 12:52             ` Michael S. Tsirkin
  2026-02-25 12:56             ` Srujana Challa
  1 sibling, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-02-25 12:52 UTC (permalink / raw)
  To: Srujana Challa
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

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.


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

* RE: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:47           ` Srujana Challa
  2026-02-25 12:52             ` Michael S. Tsirkin
@ 2026-02-25 12:56             ` Srujana Challa
  2026-02-25 13:21               ` Michael S. Tsirkin
  1 sibling, 1 reply; 13+ messages in thread
From: Srujana Challa @ 2026-02-25 12:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

> > -----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
> > > > >
> > >


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

* Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 12:56             ` Srujana Challa
@ 2026-02-25 13:21               ` Michael S. Tsirkin
  2026-02-25 13:31                 ` Srujana Challa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-02-25 13:21 UTC (permalink / raw)
  To: Srujana Challa
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

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


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

* RE: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 13:21               ` Michael S. Tsirkin
@ 2026-02-25 13:31                 ` Srujana Challa
  2026-02-25 13:57                   ` Michael S. Tsirkin
  0 siblings, 1 reply; 13+ messages in thread
From: Srujana Challa @ 2026-02-25 13:31 UTC (permalink / raw)
  To: Michael S. Tsirkin
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

> 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


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

* Re: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 13:31                 ` Srujana Challa
@ 2026-02-25 13:57                   ` Michael S. Tsirkin
  2026-02-26 12:38                     ` Srujana Challa
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2026-02-25 13:57 UTC (permalink / raw)
  To: Srujana Challa
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

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
> 


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

* RE: [EXTERNAL] Re: [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-25 13:57                   ` Michael S. Tsirkin
@ 2026-02-26 12:38                     ` Srujana Challa
  0 siblings, 0 replies; 13+ messages in thread
From: Srujana Challa @ 2026-02-26 12:38 UTC (permalink / raw)
  To: Michael S. Tsirkin
  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, Shiva Shankar Kommula,
	stable@vger.kernel.org

> 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
> >


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

end of thread, other threads:[~2026-02-26 12:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24  6:58 [PATCH net,v4,1/2] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN Srujana Challa
2026-02-25 10:03 ` Michael S. Tsirkin
2026-02-25 12:22   ` [EXTERNAL] " Srujana Challa
2026-02-25 12:24     ` Michael S. Tsirkin
2026-02-25 12:34       ` Srujana Challa
2026-02-25 12:37         ` Michael S. Tsirkin
2026-02-25 12:47           ` Srujana Challa
2026-02-25 12:52             ` Michael S. Tsirkin
2026-02-25 12:56             ` Srujana Challa
2026-02-25 13:21               ` Michael S. Tsirkin
2026-02-25 13:31                 ` Srujana Challa
2026-02-25 13:57                   ` Michael S. Tsirkin
2026-02-26 12:38                     ` Srujana Challa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox