Re: [PATCH] virtio_net: roll back RSS state on control failure

[Jakub Kicinski <kuba@kernel.org>]

On Tue,  4 Aug 2026 15:36:54 +0800 Xiong Weimin wrote:
> The ethtool RSS and RXHASH paths update the driver's cached RSS state
> before committing the change to the device.  If the control virtqueue
> command fails, the cached hash types, key or indirection table can then
> report a configuration that the device did not accept.
> 
> Preserve the previous local state around RSS/hash control commands and
> restore it when the device update fails, while propagating the error to
> the caller.

I guess.. saving the data and then copying it back doesn't seem super
clean but I guess since we DMA directly from the info struct..

Michael, Jason, looks okay? 

> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3e2a5876c..ccd96315a 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4295,6 +4295,7 @@ static int virtnet_set_hashflow(struct net_device *dev,
>  				struct netlink_ext_ack *extack)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> +	u32 old_hashtypes = vi->rss_hash_types_saved;
>  	u32 new_hashtypes = vi->rss_hash_types_saved;
>  	bool is_disable = info->data & RXH_DISCARD;
>  	bool is_l4 = info->data == (RXH_IP_SRC | RXH_IP_DST | RXH_L4_B_0_1 | RXH_L4_B_2_3);
> @@ -4350,9 +4351,13 @@ static int virtnet_set_hashflow(struct net_device *dev,
>  	if (new_hashtypes != vi->rss_hash_types_saved) {
>  		vi->rss_hash_types_saved = new_hashtypes;
>  		vi->rss_hdr->hash_types = cpu_to_le32(vi->rss_hash_types_saved);
> -		if (vi->dev->features & NETIF_F_RXHASH)
> -			if (!virtnet_commit_rss_command(vi))
> +		if (vi->dev->features & NETIF_F_RXHASH) {
> +			if (!virtnet_commit_rss_command(vi)) {
> +				vi->rss_hash_types_saved = old_hashtypes;
> +				vi->rss_hdr->hash_types = cpu_to_le32(old_hashtypes);
>  				return -EINVAL;
> +			}
> +		}
>  	}
>  
>  	return 0;
> @@ -5546,6 +5551,8 @@ static int virtnet_set_rxfh(struct net_device *dev,
>  			    struct netlink_ext_ack *extack)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> +	struct virtio_net_rss_config_hdr *old_rss_hdr = NULL;
> +	u8 old_rss_key[NETDEV_RSS_KEY_LEN];
>  	bool update = false;
>  	int i;
>  
> @@ -5553,14 +5560,8 @@ static int virtnet_set_rxfh(struct net_device *dev,
>  	    rxfh->hfunc != ETH_RSS_HASH_TOP)
>  		return -EOPNOTSUPP;
>  
> -	if (rxfh->indir) {
> -		if (!vi->has_rss)
> -			return -EOPNOTSUPP;
> -
> -		for (i = 0; i < vi->rss_indir_table_size; ++i)
> -			vi->rss_hdr->indirection_table[i] = cpu_to_le16(rxfh->indir[i]);
> -		update = true;
> -	}
> +	if (rxfh->indir && !vi->has_rss)
> +		return -EOPNOTSUPP;
>  
>  	if (rxfh->key) {
>  		/* If either _F_HASH_REPORT or _F_RSS are negotiated, the
> @@ -5569,13 +5570,36 @@ static int virtnet_set_rxfh(struct net_device *dev,
>  		 */
>  		if (!vi->has_rss && !vi->has_rss_hash_report)
>  			return -EOPNOTSUPP;
> +	}
> +
> +	if (rxfh->indir) {
> +		old_rss_hdr = kmemdup(vi->rss_hdr, virtnet_rss_hdr_size(vi),
> +				      GFP_KERNEL);
> +		if (!old_rss_hdr)
> +			return -ENOMEM;
> +
> +		for (i = 0; i < vi->rss_indir_table_size; ++i)
> +			vi->rss_hdr->indirection_table[i] =
> +				cpu_to_le16(rxfh->indir[i]);
> +		update = true;
> +	}
>  
> +	if (rxfh->key) {
> +		memcpy(old_rss_key, vi->rss_hash_key_data, vi->rss_key_size);
>  		memcpy(vi->rss_hash_key_data, rxfh->key, vi->rss_key_size);
>  		update = true;
>  	}
>  
> -	if (update)
> -		virtnet_commit_rss_command(vi);
> +	if (update && !virtnet_commit_rss_command(vi)) {
> +		if (old_rss_hdr)
> +			memcpy(vi->rss_hdr, old_rss_hdr, virtnet_rss_hdr_size(vi));
> +		if (rxfh->key)
> +			memcpy(vi->rss_hash_key_data, old_rss_key, vi->rss_key_size);
> +		kfree(old_rss_hdr);
> +		return -EINVAL;
> +	}
> +
> +	kfree(old_rss_hdr);
>  
>  	return 0;
>  }
> @@ -6171,13 +6195,17 @@ static int virtnet_set_features(struct net_device *dev,
>  	}
>  
>  	if ((dev->features ^ features) & NETIF_F_RXHASH) {
> +		__le32 hash_types = vi->rss_hdr->hash_types;
> +
>  		if (features & NETIF_F_RXHASH)
>  			vi->rss_hdr->hash_types = cpu_to_le32(vi->rss_hash_types_saved);
>  		else
>  			vi->rss_hdr->hash_types = cpu_to_le32(VIRTIO_NET_HASH_REPORT_NONE);
>  
> -		if (!virtnet_commit_rss_command(vi))
> +		if (!virtnet_commit_rss_command(vi)) {
> +			vi->rss_hdr->hash_types = hash_types;
>  			return -EINVAL;
> +		}
>  	}
>  
>  	return 0;