public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
@ 2026-02-06 12:01 Srujana Challa
  2026-02-06 15:02 ` Michael S. Tsirkin
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Srujana Challa @ 2026-02-06 12:01 UTC (permalink / raw)
  To: netdev, virtualization
  Cc: pabeni, mst, jasowang, xuanzhuo, eperezma, davem, edumazet, kuba,
	ndabilpuram, kshankar, schalla

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.

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

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index db88dcaefb20..1fa4197dffc3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -381,7 +381,8 @@ struct receive_queue {
 	struct xdp_buff **xsk_buffs;
 };
 
-#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
+#define VIRTIO_NET_RSS_MAX_KEY_SIZE     NETDEV_RSS_KEY_LEN
+#define VIRTIO_NET_RSS_MIN_KEY_SIZE     40
 
 /* Control VQ buffers: protected by the rtnl lock */
 struct control_buf {
@@ -6823,38 +6824,47 @@ static int virtnet_probe(struct virtio_device *vdev)
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
 		vi->has_rss_hash_report = true;
 
-	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
+	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
 		vi->has_rss = true;
 
-		vi->rss_indir_table_size =
-			virtio_cread16(vdev, offsetof(struct virtio_net_config,
-				rss_max_indirection_table_length));
-	}
-	vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi), GFP_KERNEL);
-	if (!vi->rss_hdr) {
-		err = -ENOMEM;
-		goto free;
-	}
-
 	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 &=
-				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
-				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
-				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
+		/* Spec requires at least 40 bytes */
+		if (vi->rss_key_size < VIRTIO_NET_RSS_MIN_KEY_SIZE) {
+			dev_warn(&vdev->dev,
+				 "rss_max_key_size=%u is less than spec minimum %u, disabling RSS\n",
+				 vi->rss_key_size, VIRTIO_NET_RSS_MIN_KEY_SIZE);
+			vi->has_rss = false;
+			vi->has_rss_hash_report = false;
+		} else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
+			dev_warn(&vdev->dev,
+				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
+				 vi->rss_key_size, VIRTIO_NET_RSS_MAX_KEY_SIZE);
+			vi->has_rss = false;
+			vi->has_rss_hash_report = false;
+		} else {
+			vi->rss_indir_table_size =
+				virtio_cread16(vdev, offsetof(struct virtio_net_config,
+							      rss_max_indirection_table_length));
+			vi->rss_hash_types_supported =
+			    virtio_cread32(vdev, offsetof(struct virtio_net_config,
+							  supported_hash_types));
+			vi->rss_hash_types_supported &=
+					~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
+					  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
+					  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
+
+			dev->hw_features |= NETIF_F_RXHASH;
+			dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
+		}
+	}
 
-		dev->hw_features |= NETIF_F_RXHASH;
-		dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
+	vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi), GFP_KERNEL);
+	if (!vi->rss_hdr) {
+		err = -ENOMEM;
+		goto free;
 	}
 
 	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||
-- 
2.25.1


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

* Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-06 12:01 [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN Srujana Challa
@ 2026-02-06 15:02 ` Michael S. Tsirkin
  2026-02-11  7:22   ` [EXTERNAL] " Srujana Challa
  2026-02-07  3:13 ` Jakub Kicinski
  2026-02-07  5:03 ` kernel test robot
  2 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-02-06 15:02 UTC (permalink / raw)
  To: Srujana Challa
  Cc: netdev, virtualization, pabeni, jasowang, xuanzhuo, eperezma,
	davem, edumazet, kuba, ndabilpuram, kshankar

On Fri, Feb 06, 2026 at 05:31:54PM +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.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>

I'd CC stable on this actually. failing when device is more capable
was a bad idea.
can you add a Fixes: tag, too? Thanks!

> ---
>  drivers/net/virtio_net.c | 62 +++++++++++++++++++++++-----------------
>  1 file changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index db88dcaefb20..1fa4197dffc3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -381,7 +381,8 @@ struct receive_queue {
>  	struct xdp_buff **xsk_buffs;
>  };
>  
> -#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     NETDEV_RSS_KEY_LEN
> +#define VIRTIO_NET_RSS_MIN_KEY_SIZE     40
>  
>  /* Control VQ buffers: protected by the rtnl lock */
>  struct control_buf {
> @@ -6823,38 +6824,47 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
>  		vi->has_rss_hash_report = true;
>  
> -	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
>  		vi->has_rss = true;
>  
> -		vi->rss_indir_table_size =
> -			virtio_cread16(vdev, offsetof(struct virtio_net_config,
> -				rss_max_indirection_table_length));
> -	}
> -	vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi), GFP_KERNEL);
> -	if (!vi->rss_hdr) {
> -		err = -ENOMEM;
> -		goto free;
> -	}
> -
>  	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 &=
> -				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> -				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> -				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> +		/* Spec requires at least 40 bytes */
> +		if (vi->rss_key_size < VIRTIO_NET_RSS_MIN_KEY_SIZE) {
> +			dev_warn(&vdev->dev,
> +				 "rss_max_key_size=%u is less than spec minimum %u, disabling RSS\n",
> +				 vi->rss_key_size, VIRTIO_NET_RSS_MIN_KEY_SIZE);
> +			vi->has_rss = false;
> +			vi->has_rss_hash_report = false;
> +		} else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> +			dev_warn(&vdev->dev,
> +				 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
> +				 vi->rss_key_size, VIRTIO_NET_RSS_MAX_KEY_SIZE);
> +			vi->has_rss = false;
> +			vi->has_rss_hash_report = false;
> +		} else {
> +			vi->rss_indir_table_size =
> +				virtio_cread16(vdev, offsetof(struct virtio_net_config,
> +							      rss_max_indirection_table_length));
> +			vi->rss_hash_types_supported =
> +			    virtio_cread32(vdev, offsetof(struct virtio_net_config,
> +							  supported_hash_types));
> +			vi->rss_hash_types_supported &=
> +					~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> +					  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> +					  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> +
> +			dev->hw_features |= NETIF_F_RXHASH;
> +			dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> +		}
> +	}
>  
> -		dev->hw_features |= NETIF_F_RXHASH;
> -		dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> +	vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi), GFP_KERNEL);
> +	if (!vi->rss_hdr) {
> +		err = -ENOMEM;
> +		goto free;
>  	}

I feel these checks belong in virtnet_validate. Then we can disable
features e.g. by clearing VIRTIO_NET_F_RSS and host will have some
visibility that RSS can't be used.



>  
>  	if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||
> -- 
> 2.25.1


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

* Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-06 12:01 [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN Srujana Challa
  2026-02-06 15:02 ` Michael S. Tsirkin
@ 2026-02-07  3:13 ` Jakub Kicinski
  2026-02-07  9:56   ` Michael S. Tsirkin
  2026-02-07 10:36   ` Michael S. Tsirkin
  2026-02-07  5:03 ` kernel test robot
  2 siblings, 2 replies; 12+ messages in thread
From: Jakub Kicinski @ 2026-02-07  3:13 UTC (permalink / raw)
  To: Srujana Challa
  Cc: netdev, virtualization, pabeni, mst, jasowang, xuanzhuo, eperezma,
	davem, edumazet, ndabilpuram, kshankar

On Fri, 6 Feb 2026 17:31:54 +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.

Hm, FWIW clang says:

drivers/net/virtio_net.c:6841:31: warning: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
 6841 |                 } else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
      |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~

Which is kinda annoying because the value was increased in net-next.
If Machael wants this backported then we need to keep the check
and follow up in net-next? We could try to cast the u32 away but
that feels dirty..

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

* Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-06 12:01 [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN Srujana Challa
  2026-02-06 15:02 ` Michael S. Tsirkin
  2026-02-07  3:13 ` Jakub Kicinski
@ 2026-02-07  5:03 ` kernel test robot
  2 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2026-02-07  5:03 UTC (permalink / raw)
  To: Srujana Challa, netdev, virtualization
  Cc: llvm, oe-kbuild-all, pabeni, mst, jasowang, xuanzhuo, eperezma,
	davem, edumazet, kuba, ndabilpuram, kshankar, schalla

Hi Srujana,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Srujana-Challa/virtio_net-Improve-RSS-key-size-validation-and-use-NETDEV_RSS_KEY_LEN/20260206-200801
base:   net-next/main
patch link:    https://lore.kernel.org/r/20260206120154.2548079-1-schalla%40marvell.com
patch subject: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20260207/202602071211.N0SvyotM-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260207/202602071211.N0SvyotM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602071211.N0SvyotM-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/virtio_net.c:6841:31: warning: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
    6841 |                 } else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
         |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning generated.


vim +6841 drivers/net/virtio_net.c

  6699	
  6700	static int virtnet_probe(struct virtio_device *vdev)
  6701	{
  6702		int i, err = -ENOMEM;
  6703		struct net_device *dev;
  6704		struct virtnet_info *vi;
  6705		u16 max_queue_pairs;
  6706		int mtu = 0;
  6707	
  6708		/* Find if host supports multiqueue/rss virtio_net device */
  6709		max_queue_pairs = 1;
  6710		if (virtio_has_feature(vdev, VIRTIO_NET_F_MQ) || virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
  6711			max_queue_pairs =
  6712			     virtio_cread16(vdev, offsetof(struct virtio_net_config, max_virtqueue_pairs));
  6713	
  6714		/* We need at least 2 queue's */
  6715		if (max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN ||
  6716		    max_queue_pairs > VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX ||
  6717		    !virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
  6718			max_queue_pairs = 1;
  6719	
  6720		/* Allocate ourselves a network device with room for our info */
  6721		dev = alloc_etherdev_mq(sizeof(struct virtnet_info), max_queue_pairs);
  6722		if (!dev)
  6723			return -ENOMEM;
  6724	
  6725		/* Set up network device as normal. */
  6726		dev->priv_flags |= IFF_UNICAST_FLT | IFF_LIVE_ADDR_CHANGE |
  6727				   IFF_TX_SKB_NO_LINEAR;
  6728		dev->netdev_ops = &virtnet_netdev;
  6729		dev->stat_ops = &virtnet_stat_ops;
  6730		dev->features = NETIF_F_HIGHDMA;
  6731	
  6732		dev->ethtool_ops = &virtnet_ethtool_ops;
  6733		SET_NETDEV_DEV(dev, &vdev->dev);
  6734	
  6735		/* Do we support "hardware" checksums? */
  6736		if (virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) {
  6737			/* This opens up the world of extra features. */
  6738			dev->hw_features |= NETIF_F_HW_CSUM | NETIF_F_SG;
  6739			if (csum)
  6740				dev->features |= NETIF_F_HW_CSUM | NETIF_F_SG;
  6741	
  6742			if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
  6743				dev->hw_features |= NETIF_F_TSO
  6744					| NETIF_F_TSO_ECN | NETIF_F_TSO6;
  6745			}
  6746			/* Individual feature bits: what can host handle? */
  6747			if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO4))
  6748				dev->hw_features |= NETIF_F_TSO;
  6749			if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO6))
  6750				dev->hw_features |= NETIF_F_TSO6;
  6751			if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
  6752				dev->hw_features |= NETIF_F_TSO_ECN;
  6753			if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_USO))
  6754				dev->hw_features |= NETIF_F_GSO_UDP_L4;
  6755	
  6756			if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO)) {
  6757				dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
  6758				dev->hw_enc_features = dev->hw_features;
  6759			}
  6760			if (dev->hw_features & NETIF_F_GSO_UDP_TUNNEL &&
  6761			    virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM)) {
  6762				dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
  6763				dev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM;
  6764			}
  6765	
  6766			dev->features |= NETIF_F_GSO_ROBUST;
  6767	
  6768			if (gso)
  6769				dev->features |= dev->hw_features;
  6770			/* (!csum && gso) case will be fixed by register_netdev() */
  6771		}
  6772	
  6773		/* 1. With VIRTIO_NET_F_GUEST_CSUM negotiation, the driver doesn't
  6774		 * need to calculate checksums for partially checksummed packets,
  6775		 * as they're considered valid by the upper layer.
  6776		 * 2. Without VIRTIO_NET_F_GUEST_CSUM negotiation, the driver only
  6777		 * receives fully checksummed packets. The device may assist in
  6778		 * validating these packets' checksums, so the driver won't have to.
  6779		 */
  6780		dev->features |= NETIF_F_RXCSUM;
  6781	
  6782		if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
  6783		    virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6))
  6784			dev->features |= NETIF_F_GRO_HW;
  6785		if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS))
  6786			dev->hw_features |= NETIF_F_GRO_HW;
  6787	
  6788		dev->vlan_features = dev->features;
  6789		dev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
  6790			NETDEV_XDP_ACT_XSK_ZEROCOPY;
  6791	
  6792		/* MTU range: 68 - 65535 */
  6793		dev->min_mtu = MIN_MTU;
  6794		dev->max_mtu = MAX_MTU;
  6795	
  6796		/* Configuration may specify what MAC to use.  Otherwise random. */
  6797		if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
  6798			u8 addr[ETH_ALEN];
  6799	
  6800			virtio_cread_bytes(vdev,
  6801					   offsetof(struct virtio_net_config, mac),
  6802					   addr, ETH_ALEN);
  6803			eth_hw_addr_set(dev, addr);
  6804		} else {
  6805			eth_hw_addr_random(dev);
  6806			dev_info(&vdev->dev, "Assigned random MAC address %pM\n",
  6807				 dev->dev_addr);
  6808		}
  6809	
  6810		/* Set up our device-specific information */
  6811		vi = netdev_priv(dev);
  6812		vi->dev = dev;
  6813		vi->vdev = vdev;
  6814		vdev->priv = vi;
  6815	
  6816		INIT_WORK(&vi->config_work, virtnet_config_changed_work);
  6817		INIT_WORK(&vi->rx_mode_work, virtnet_rx_mode_work);
  6818	
  6819		if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) {
  6820			vi->mergeable_rx_bufs = true;
  6821			dev->xdp_features |= NETDEV_XDP_ACT_RX_SG;
  6822		}
  6823	
  6824		if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
  6825			vi->has_rss_hash_report = true;
  6826	
  6827		if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
  6828			vi->has_rss = true;
  6829	
  6830		if (vi->has_rss || vi->has_rss_hash_report) {
  6831			vi->rss_key_size =
  6832				virtio_cread8(vdev, offsetof(struct virtio_net_config, rss_max_key_size));
  6833	
  6834			/* Spec requires at least 40 bytes */
  6835			if (vi->rss_key_size < VIRTIO_NET_RSS_MIN_KEY_SIZE) {
  6836				dev_warn(&vdev->dev,
  6837					 "rss_max_key_size=%u is less than spec minimum %u, disabling RSS\n",
  6838					 vi->rss_key_size, VIRTIO_NET_RSS_MIN_KEY_SIZE);
  6839				vi->has_rss = false;
  6840				vi->has_rss_hash_report = false;
> 6841			} else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
  6842				dev_warn(&vdev->dev,
  6843					 "rss_max_key_size=%u exceeds driver limit %u, disabling RSS\n",
  6844					 vi->rss_key_size, VIRTIO_NET_RSS_MAX_KEY_SIZE);
  6845				vi->has_rss = false;
  6846				vi->has_rss_hash_report = false;
  6847			} else {
  6848				vi->rss_indir_table_size =
  6849					virtio_cread16(vdev, offsetof(struct virtio_net_config,
  6850								      rss_max_indirection_table_length));
  6851				vi->rss_hash_types_supported =
  6852				    virtio_cread32(vdev, offsetof(struct virtio_net_config,
  6853								  supported_hash_types));
  6854				vi->rss_hash_types_supported &=
  6855						~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
  6856						  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
  6857						  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
  6858	
  6859				dev->hw_features |= NETIF_F_RXHASH;
  6860				dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
  6861			}
  6862		}
  6863	
  6864		vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi), GFP_KERNEL);
  6865		if (!vi->rss_hdr) {
  6866			err = -ENOMEM;
  6867			goto free;
  6868		}
  6869	
  6870		if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||
  6871		    virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO))
  6872			vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash_tunnel);
  6873		else if (vi->has_rss_hash_report)
  6874			vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
  6875		else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
  6876			 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
  6877			vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
  6878		else
  6879			vi->hdr_len = sizeof(struct virtio_net_hdr);
  6880	
  6881		if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM))
  6882			vi->rx_tnl_csum = true;
  6883		if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO))
  6884			vi->rx_tnl = true;
  6885		if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO))
  6886			vi->tx_tnl = true;
  6887	
  6888		if (virtio_has_feature(vdev, VIRTIO_F_ANY_LAYOUT) ||
  6889		    virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
  6890			vi->any_header_sg = true;
  6891	
  6892		if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
  6893			vi->has_cvq = true;
  6894	
  6895		mutex_init(&vi->cvq_lock);
  6896	
  6897		if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
  6898			mtu = virtio_cread16(vdev,
  6899					     offsetof(struct virtio_net_config,
  6900						      mtu));
  6901			if (mtu < dev->min_mtu) {
  6902				/* Should never trigger: MTU was previously validated
  6903				 * in virtnet_validate.
  6904				 */
  6905				dev_err(&vdev->dev,
  6906					"device MTU appears to have changed it is now %d < %d",
  6907					mtu, dev->min_mtu);
  6908				err = -EINVAL;
  6909				goto free;
  6910			}
  6911	
  6912			dev->mtu = mtu;
  6913			dev->max_mtu = mtu;
  6914		}
  6915	
  6916		virtnet_set_big_packets(vi, mtu);
  6917	
  6918		if (vi->any_header_sg)
  6919			dev->needed_headroom = vi->hdr_len;
  6920	
  6921		/* Enable multiqueue by default */
  6922		if (num_online_cpus() >= max_queue_pairs)
  6923			vi->curr_queue_pairs = max_queue_pairs;
  6924		else
  6925			vi->curr_queue_pairs = num_online_cpus();
  6926		vi->max_queue_pairs = max_queue_pairs;
  6927	
  6928		/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
  6929		err = init_vqs(vi);
  6930		if (err)
  6931			goto free;
  6932	
  6933		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
  6934			vi->intr_coal_rx.max_usecs = 0;
  6935			vi->intr_coal_tx.max_usecs = 0;
  6936			vi->intr_coal_rx.max_packets = 0;
  6937	
  6938			/* Keep the default values of the coalescing parameters
  6939			 * aligned with the default napi_tx state.
  6940			 */
  6941			if (vi->sq[0].napi.weight)
  6942				vi->intr_coal_tx.max_packets = 1;
  6943			else
  6944				vi->intr_coal_tx.max_packets = 0;
  6945		}
  6946	
  6947		if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL)) {
  6948			/* The reason is the same as VIRTIO_NET_F_NOTF_COAL. */
  6949			for (i = 0; i < vi->max_queue_pairs; i++)
  6950				if (vi->sq[i].napi.weight)
  6951					vi->sq[i].intr_coal.max_packets = 1;
  6952	
  6953			err = virtnet_init_irq_moder(vi);
  6954			if (err)
  6955				goto free;
  6956		}
  6957	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-07  3:13 ` Jakub Kicinski
@ 2026-02-07  9:56   ` Michael S. Tsirkin
  2026-02-11  7:44     ` [EXTERNAL] " Srujana Challa
  2026-02-07 10:36   ` Michael S. Tsirkin
  1 sibling, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-02-07  9:56 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Srujana Challa, netdev, virtualization, pabeni, jasowang,
	xuanzhuo, eperezma, davem, edumazet, ndabilpuram, kshankar

On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> On Fri, 6 Feb 2026 17:31:54 +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.
> 
> Hm, FWIW clang says:
> 
> drivers/net/virtio_net.c:6841:31: warning: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
>  6841 |                 } else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
>       |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Which is kinda annoying because the value was increased in net-next.
> If Machael wants this backported then we need to keep the check
> and follow up in net-next? We could try to cast the u32 away but
> that feels dirty..


I'd say yes. the warning is harmless.
so
patch 1 - this code
patch 2 - replace with BUILD_BUG_ON

but i ask then whether this code was actually tested against net-next.

-- 
MST


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

* Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-07  3:13 ` Jakub Kicinski
  2026-02-07  9:56   ` Michael S. Tsirkin
@ 2026-02-07 10:36   ` Michael S. Tsirkin
  1 sibling, 0 replies; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-02-07 10:36 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Srujana Challa, netdev, virtualization, pabeni, jasowang,
	xuanzhuo, eperezma, davem, edumazet, ndabilpuram, kshankar

On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> On Fri, 6 Feb 2026 17:31:54 +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.
> 
> Hm, FWIW clang says:
> 
> drivers/net/virtio_net.c:6841:31: warning: result of comparison of constant 256 with expression of type 'u8' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
>  6841 |                 } else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
>       |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Which is kinda annoying because the value was increased in net-next.
> If Machael wants this backported then we need to keep the check
> and follow up in net-next? We could try to cast the u32 away but
> that feels dirty..


for net next we will presumably replace with 
	BUILD_BUG_ON(type_max(vi->rss_key_size) > NETDEV_RSS_KEY_LEN) 

and I think we should get rid of VIRTIO_NET_RSS_MAX_KEY_SIZE while
we are at it.

-- 
MST


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

* RE: [EXTERNAL] Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-06 15:02 ` Michael S. Tsirkin
@ 2026-02-11  7:22   ` Srujana Challa
  0 siblings, 0 replies; 12+ messages in thread
From: Srujana Challa @ 2026-02-11  7: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

> On Fri, Feb 06, 2026 at 05:31:54PM +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.
> >
> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> 
> I'd CC stable on this actually. failing when device is more capable was a bad
> idea.
> can you add a Fixes: tag, too? Thanks!
Sure.

> 
> > ---
> >  drivers/net/virtio_net.c | 62
> > +++++++++++++++++++++++-----------------
> >  1 file changed, 36 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
> > db88dcaefb20..1fa4197dffc3 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -381,7 +381,8 @@ struct receive_queue {
> >  	struct xdp_buff **xsk_buffs;
> >  };
> >
> > -#define VIRTIO_NET_RSS_MAX_KEY_SIZE     40
> > +#define VIRTIO_NET_RSS_MAX_KEY_SIZE     NETDEV_RSS_KEY_LEN
> > +#define VIRTIO_NET_RSS_MIN_KEY_SIZE     40
> >
> >  /* Control VQ buffers: protected by the rtnl lock */  struct
> > control_buf { @@ -6823,38 +6824,47 @@ static int virtnet_probe(struct
> > virtio_device *vdev)
> >  	if (virtio_has_feature(vdev, VIRTIO_NET_F_HASH_REPORT))
> >  		vi->has_rss_hash_report = true;
> >
> > -	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS)) {
> > +	if (virtio_has_feature(vdev, VIRTIO_NET_F_RSS))
> >  		vi->has_rss = true;
> >
> > -		vi->rss_indir_table_size =
> > -			virtio_cread16(vdev, offsetof(struct virtio_net_config,
> > -				rss_max_indirection_table_length));
> > -	}
> > -	vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi),
> GFP_KERNEL);
> > -	if (!vi->rss_hdr) {
> > -		err = -ENOMEM;
> > -		goto free;
> > -	}
> > -
> >  	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 &=
> > -				~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> > -				  VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> > -				  VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> > +		/* Spec requires at least 40 bytes */
> > +		if (vi->rss_key_size < VIRTIO_NET_RSS_MIN_KEY_SIZE) {
> > +			dev_warn(&vdev->dev,
> > +				 "rss_max_key_size=%u is less than spec
> minimum %u, disabling RSS\n",
> > +				 vi->rss_key_size,
> VIRTIO_NET_RSS_MIN_KEY_SIZE);
> > +			vi->has_rss = false;
> > +			vi->has_rss_hash_report = false;
> > +		} else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE) {
> > +			dev_warn(&vdev->dev,
> > +				 "rss_max_key_size=%u exceeds driver limit
> %u, disabling RSS\n",
> > +				 vi->rss_key_size,
> VIRTIO_NET_RSS_MAX_KEY_SIZE);
> > +			vi->has_rss = false;
> > +			vi->has_rss_hash_report = false;
> > +		} else {
> > +			vi->rss_indir_table_size =
> > +				virtio_cread16(vdev, offsetof(struct
> virtio_net_config,
> > +
> rss_max_indirection_table_length));
> > +			vi->rss_hash_types_supported =
> > +			    virtio_cread32(vdev, offsetof(struct
> virtio_net_config,
> > +
> supported_hash_types));
> > +			vi->rss_hash_types_supported &=
> > +
> 	~(VIRTIO_NET_RSS_HASH_TYPE_IP_EX |
> > +
> VIRTIO_NET_RSS_HASH_TYPE_TCP_EX |
> > +
> VIRTIO_NET_RSS_HASH_TYPE_UDP_EX);
> > +
> > +			dev->hw_features |= NETIF_F_RXHASH;
> > +			dev->xdp_metadata_ops =
> &virtnet_xdp_metadata_ops;
> > +		}
> > +	}
> >
> > -		dev->hw_features |= NETIF_F_RXHASH;
> > -		dev->xdp_metadata_ops = &virtnet_xdp_metadata_ops;
> > +	vi->rss_hdr = devm_kzalloc(&vdev->dev, virtnet_rss_hdr_size(vi),
> GFP_KERNEL);
> > +	if (!vi->rss_hdr) {
> > +		err = -ENOMEM;
> > +		goto free;
> >  	}
> 
> I feel these checks belong in virtnet_validate. Then we can disable features e.g.
> by clearing VIRTIO_NET_F_RSS and host will have some visibility that RSS can't
> be used.
Sure, will move to virtnet_validate() and clear feature bits.
Thanks!
> 
> 
> 
> >
> >  	if (virtio_has_feature(vdev,
> VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||
> > --
> > 2.25.1


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

* RE: [EXTERNAL] Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-07  9:56   ` Michael S. Tsirkin
@ 2026-02-11  7:44     ` Srujana Challa
  2026-02-11  8:09       ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Srujana Challa @ 2026-02-11  7:44 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jakub Kicinski
  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, Nithin Kumar Dabilpuram,
	Shiva Shankar Kommula

> On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> > On Fri, 6 Feb 2026 17:31:54 +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.
> >
> > Hm, FWIW clang says:
> >
> > drivers/net/virtio_net.c:6841:31: warning: result of comparison of constant
> 256 with expression of type 'u8' (aka 'unsigned char') is always false [-
> Wtautological-constant-out-of-range-compare]
> >  6841 |                 } else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE)
> {
> >       |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Which is kinda annoying because the value was increased in net-next.
> > If Machael wants this backported then we need to keep the check and
> > follow up in net-next? We could try to cast the u32 away but that
> > feels dirty..
> 
> 
> I'd say yes. the warning is harmless.
> so
> patch 1 - this code
> patch 2 - replace with BUILD_BUG_ON
> 
> but i ask then whether this code was actually tested against net-next.
Yes, tested against net-next (before NETDEV_RSS_KEY_LEN was changed to 256).
BUILD_BUG_ON would always fail as-is on net-next since rss_key_size is u8.
Can we cast the comparison to u32 instead?
Thanks!

> 
> --
> MST


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

* Re: [EXTERNAL] Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-11  7:44     ` [EXTERNAL] " Srujana Challa
@ 2026-02-11  8:09       ` Michael S. Tsirkin
  2026-02-11  9:33         ` Srujana Challa
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-02-11  8:09 UTC (permalink / raw)
  To: Srujana Challa
  Cc: Jakub Kicinski, 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,
	Nithin Kumar Dabilpuram, Shiva Shankar Kommula

On Wed, Feb 11, 2026 at 07:44:54AM +0000, Srujana Challa wrote:
> > On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> > > On Fri, 6 Feb 2026 17:31:54 +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.
> > >
> > > Hm, FWIW clang says:
> > >
> > > drivers/net/virtio_net.c:6841:31: warning: result of comparison of constant
> > 256 with expression of type 'u8' (aka 'unsigned char') is always false [-
> > Wtautological-constant-out-of-range-compare]
> > >  6841 |                 } else if (vi->rss_key_size > VIRTIO_NET_RSS_MAX_KEY_SIZE)
> > {
> > >       |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Which is kinda annoying because the value was increased in net-next.
> > > If Machael wants this backported then we need to keep the check and
> > > follow up in net-next? We could try to cast the u32 away but that
> > > feels dirty..
> > 
> > 
> > I'd say yes. the warning is harmless.
> > so
> > patch 1 - this code
> > patch 2 - replace with BUILD_BUG_ON
> > 
> > but i ask then whether this code was actually tested against net-next.
> Yes, tested against net-next (before NETDEV_RSS_KEY_LEN was changed to 256).
> BUILD_BUG_ON would always fail as-is on net-next since rss_key_size is u8.

BUILD_BUG_ON(type_max(vi->rss_key_size) >= VIRTIO_NET_RSS_MAX_KEY_SIZE)

why would it fail?


> Can we cast the comparison to u32 instead?
> Thanks!
> 
> > 
> > --
> > MST
> 

forcing compiler to generate code where we do not need any
really seems silly.

-- 
MST


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

* RE: [EXTERNAL] Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-11  8:09       ` Michael S. Tsirkin
@ 2026-02-11  9:33         ` Srujana Challa
  2026-02-11  9:36           ` Michael S. Tsirkin
  0 siblings, 1 reply; 12+ messages in thread
From: Srujana Challa @ 2026-02-11  9:33 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jakub Kicinski, 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,
	Nithin Kumar Dabilpuram, Shiva Shankar Kommula

> On Wed, Feb 11, 2026 at 07:44:54AM +0000, Srujana Challa wrote:
> > > On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> > > > On Fri, 6 Feb 2026 17:31:54 +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.
> > > >
> > > > Hm, FWIW clang says:
> > > >
> > > > drivers/net/virtio_net.c:6841:31: warning: result of comparison of
> > > > constant
> > > 256 with expression of type 'u8' (aka 'unsigned char') is always
> > > false [- Wtautological-constant-out-of-range-compare]
> > > >  6841 |                 } else if (vi->rss_key_size >
> VIRTIO_NET_RSS_MAX_KEY_SIZE)
> > > {
> > > >       |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >
> > > > Which is kinda annoying because the value was increased in net-next.
> > > > If Machael wants this backported then we need to keep the check
> > > > and follow up in net-next? We could try to cast the u32 away but
> > > > that feels dirty..
> > >
> > >
> > > I'd say yes. the warning is harmless.
> > > so
> > > patch 1 - this code
> > > patch 2 - replace with BUILD_BUG_ON
> > >
> > > but i ask then whether this code was actually tested against net-next.
> > Yes, tested against net-next (before NETDEV_RSS_KEY_LEN was changed to
> 256).
> > BUILD_BUG_ON would always fail as-is on net-next since rss_key_size is u8.
> 
> BUILD_BUG_ON(type_max(vi->rss_key_size) >=
> VIRTIO_NET_RSS_MAX_KEY_SIZE)
> 
> why would it fail?
I was assuming BUILD_BUG_ON was intended to catch the type mismatch 
(when VIRTIO_NET_RSS_MAX_KEY_SIZE exceeds what u8 can hold). I understand 
now - thanks for clarifying!
> 
> 
> > Can we cast the comparison to u32 instead?
> > Thanks!
> >
> > >
> > > --
> > > MST
> >
> 
> forcing compiler to generate code where we do not need any really seems silly.
> 
> --
> MST


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

* Re: [EXTERNAL] Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-11  9:33         ` Srujana Challa
@ 2026-02-11  9:36           ` Michael S. Tsirkin
  2026-02-11  9:48             ` Srujana Challa
  0 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2026-02-11  9:36 UTC (permalink / raw)
  To: Srujana Challa
  Cc: Jakub Kicinski, 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,
	Nithin Kumar Dabilpuram, Shiva Shankar Kommula

On Wed, Feb 11, 2026 at 09:33:26AM +0000, Srujana Challa wrote:
> > On Wed, Feb 11, 2026 at 07:44:54AM +0000, Srujana Challa wrote:
> > > > On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> > > > > On Fri, 6 Feb 2026 17:31:54 +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.
> > > > >
> > > > > Hm, FWIW clang says:
> > > > >
> > > > > drivers/net/virtio_net.c:6841:31: warning: result of comparison of
> > > > > constant
> > > > 256 with expression of type 'u8' (aka 'unsigned char') is always
> > > > false [- Wtautological-constant-out-of-range-compare]
> > > > >  6841 |                 } else if (vi->rss_key_size >
> > VIRTIO_NET_RSS_MAX_KEY_SIZE)
> > > > {
> > > > >       |                            ~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >
> > > > > Which is kinda annoying because the value was increased in net-next.
> > > > > If Machael wants this backported then we need to keep the check
> > > > > and follow up in net-next? We could try to cast the u32 away but
> > > > > that feels dirty..
> > > >
> > > >
> > > > I'd say yes. the warning is harmless.
> > > > so
> > > > patch 1 - this code
> > > > patch 2 - replace with BUILD_BUG_ON
> > > >
> > > > but i ask then whether this code was actually tested against net-next.
> > > Yes, tested against net-next (before NETDEV_RSS_KEY_LEN was changed to
> > 256).
> > > BUILD_BUG_ON would always fail as-is on net-next since rss_key_size is u8.
> > 
> > BUILD_BUG_ON(type_max(vi->rss_key_size) >=
> > VIRTIO_NET_RSS_MAX_KEY_SIZE)
> > 
> > why would it fail?
> I was assuming BUILD_BUG_ON was intended to catch the type mismatch 
> (when VIRTIO_NET_RSS_MAX_KEY_SIZE exceeds what u8 can hold). I understand 
> now - thanks for clarifying!

maybe we should just 

#define VIRTIO_NET_RSS_MAX_KEY_SIZE type_max(vi->rss_key_size) + 1


> > 
> > 
> > > Can we cast the comparison to u32 instead?
> > > Thanks!
> > >
> > > >
> > > > --
> > > > MST
> > >
> > 
> > forcing compiler to generate code where we do not need any really seems silly.
> > 
> > --
> > MST
> 


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

* RE: [EXTERNAL] Re: [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN
  2026-02-11  9:36           ` Michael S. Tsirkin
@ 2026-02-11  9:48             ` Srujana Challa
  0 siblings, 0 replies; 12+ messages in thread
From: Srujana Challa @ 2026-02-11  9:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jakub Kicinski, 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,
	Nithin Kumar Dabilpuram, Shiva Shankar Kommula

> On Wed, Feb 11, 2026 at 09:33:26AM +0000, Srujana Challa wrote:
> > > On Wed, Feb 11, 2026 at 07:44:54AM +0000, Srujana Challa wrote:
> > > > > On Fri, Feb 06, 2026 at 07:13:08PM -0800, Jakub Kicinski wrote:
> > > > > > On Fri, 6 Feb 2026 17:31:54 +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.
> > > > > >
> > > > > > Hm, FWIW clang says:
> > > > > >
> > > > > > drivers/net/virtio_net.c:6841:31: warning: result of
> > > > > > comparison of constant
> > > > > 256 with expression of type 'u8' (aka 'unsigned char') is always
> > > > > false [- Wtautological-constant-out-of-range-compare]
> > > > > >  6841 |                 } else if (vi->rss_key_size >
> > > VIRTIO_NET_RSS_MAX_KEY_SIZE)
> > > > > {
> > > > > >       |                            ~~~~~~~~~~~~~~~~ ^
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > >
> > > > > > Which is kinda annoying because the value was increased in net-next.
> > > > > > If Machael wants this backported then we need to keep the
> > > > > > check and follow up in net-next? We could try to cast the u32
> > > > > > away but that feels dirty..
> > > > >
> > > > >
> > > > > I'd say yes. the warning is harmless.
> > > > > so
> > > > > patch 1 - this code
> > > > > patch 2 - replace with BUILD_BUG_ON
> > > > >
> > > > > but i ask then whether this code was actually tested against net-next.
> > > > Yes, tested against net-next (before NETDEV_RSS_KEY_LEN was
> > > > changed to
> > > 256).
> > > > BUILD_BUG_ON would always fail as-is on net-next since rss_key_size is
> u8.
> > >
> > > BUILD_BUG_ON(type_max(vi->rss_key_size) >=
> > > VIRTIO_NET_RSS_MAX_KEY_SIZE)
> > >
> > > why would it fail?
> > I was assuming BUILD_BUG_ON was intended to catch the type mismatch
> > (when VIRTIO_NET_RSS_MAX_KEY_SIZE exceeds what u8 can hold). I
> > understand now - thanks for clarifying!
> 
> maybe we should just
> 
> #define VIRTIO_NET_RSS_MAX_KEY_SIZE type_max(vi->rss_key_size) + 1
Good idea! Yes, then patch 2 also wouldn't be required.
> 
> 
> > >
> > >
> > > > Can we cast the comparison to u32 instead?
> > > > Thanks!
> > > >
> > > > >
> > > > > --
> > > > > MST
> > > >
> > >
> > > forcing compiler to generate code where we do not need any really seems
> silly.
> > >
> > > --
> > > MST
> >


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

end of thread, other threads:[~2026-02-11  9:48 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-06 12:01 [PATCH v2 net-next] virtio_net: Improve RSS key size validation and use NETDEV_RSS_KEY_LEN Srujana Challa
2026-02-06 15:02 ` Michael S. Tsirkin
2026-02-11  7:22   ` [EXTERNAL] " Srujana Challa
2026-02-07  3:13 ` Jakub Kicinski
2026-02-07  9:56   ` Michael S. Tsirkin
2026-02-11  7:44     ` [EXTERNAL] " Srujana Challa
2026-02-11  8:09       ` Michael S. Tsirkin
2026-02-11  9:33         ` Srujana Challa
2026-02-11  9:36           ` Michael S. Tsirkin
2026-02-11  9:48             ` Srujana Challa
2026-02-07 10:36   ` Michael S. Tsirkin
2026-02-07  5:03 ` kernel test robot

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