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