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

[Srujana Challa <schalla@marvell.com>]

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