From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C16123B2FC7 for ; Sat, 8 Aug 2026 08:33:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786177985; cv=none; b=oSlK2pUWzXfQKBrLo3/RdPcGazqTMsmJuJIHT5MTKKEIeqJyFS5q/ZYU0Jp0CNgDzgFEo/cH9H9oYmwcbtcV7ehM0OfjAdo1TIlNUEGUpOfaQzFqYiKqaFcERz/oo4y/BjL6tL2fPk/k0JUPqQS13IX1MHtB8ZTFVjcqn2MvL2s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786177985; c=relaxed/simple; bh=erfPTrBfMbY4b/aNL7QZiThmkR/g3Qs7RE3mV3lPFzU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BHEc0PBMsRRC9EUZJbLWbGQq5vrD52ETcrhKWRjCzQhlACy4yDY2Y9fHSNOTmG8sCwF9hqNdx3WVqFat7b2zEZ/TYPl1ed0ccj+Kld12DWFGN/93DT8N4DZizvb7k+AkMn6uM0ngMDCdXBwvT6OzIFNZnnbrg45CjvHcN+KbMog= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=TURwtRsh; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=YfWjWXcb; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="TURwtRsh"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="YfWjWXcb" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1786177978; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=qJODVPjCODu+Iox4eT9aAOb7RYJ/0RkXxsjTuZ6pISo=; b=TURwtRshUpFCC5iJUG6FSYfMtuf8hE2VLJVylmCda0qwP8v5zBq0v2nQD3MVUngoGD7lav UMLDsuqsaABygH8NedJeD1UrR7Y3eSGC2xHEVHqwIaGhEVInvxAaXM0NTc0AFaQyXfGcXE 27rG4a6yrMXalgxE2cHCito/hge0HyU= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-122-QPlqrRrFPvGsBk-ja1XJgg-1; Sat, 08 Aug 2026 04:32:47 -0400 X-MC-Unique: QPlqrRrFPvGsBk-ja1XJgg-1 X-Mimecast-MFC-AGG-ID: QPlqrRrFPvGsBk-ja1XJgg_1786177964 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-47f835ac1aeso199531f8f.3 for ; Sat, 08 Aug 2026 01:32:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1786177964; x=1786782764; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=qJODVPjCODu+Iox4eT9aAOb7RYJ/0RkXxsjTuZ6pISo=; b=YfWjWXcbKbeegYasoL4p9adH7U8B2ijmEfTM5FVbQwQf8xApih3q3H9i0GKPuBdWhj ITsvqFYCm8yuLm2WyV3SXFYRc25cuIr0mONug5ys9lS1b3IiYnOwe9tR2yr97DVrZ6nA 0w0AFpMT2lXEeuKmj46YK2VsEpTqrPSY8DG/8yFRrZiUchvDYq7AmP5Knr2wOgHAMdaP rm8EsBcOG+UijZcRIN3gX/oqnSq7x4ZKv00e4WBTaIc4mU1pE9t9uEDITEaEyie8F+VI 2DBHlsua2gbjw7EwezvIWV79f3uLvOrQUswF896ts7ZdnK3vxK75YGyOIjYFqpvXmevc b2GA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786177964; x=1786782764; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=qJODVPjCODu+Iox4eT9aAOb7RYJ/0RkXxsjTuZ6pISo=; b=O+Wf1FPUyvNmErNOPpBX/yeUTW4q5b3GseH3Iwmt+PtYL7pub/vGvcskfNdpw/Pk+s gtr12V8HfFdp5RG1gH2yZeMBjXzgW4CfeKKRcjzWWSACBNMWmtHbcESVLxXOS5XGNmrS 76lxK9gPEin7pPeuYLuOeK9g5e6TEur4sj5tc8R1IVBu49HS3vzZkSXu7fp0EfPYawKS 6/UZbnTPeEiicV6my2HEROyHrKCK6ceHSm6O+YoaE2anpzBnyTnA1koDYyurt1MQYo5f /v5G8WvGO5/uG5eDwPo0CgRlXn8amfxOYxKIPds+WOIStsO6dL9rpbGoMY4mcKpHWQue r1eg== X-Forwarded-Encrypted: i=1; AHgh+RpH9zZ8iDr7gJrUl5XIp9r4DH+1HWemLpN+kGM1VwQV+OAoZnOI7jyCAv3SUdCLtMHtcbi27cOh9uCG6+A=@vger.kernel.org X-Gm-Message-State: AOJu0YzgL7tmGqnYVj7zInoZFr0f4X1gIZhNfScSsBehl4IBJ/2W1Qvh U5tKRs2nyUXvkAqvK6jBUF6BDr9sYgWG6kp/qJetMN+MJ1aHEHT7unRI5MWOgy7cL7PK2tS31iZ kGVIlwP5/MXxSVdQ31OvLDtkiVClz3cAoFOMRV1jWRWG3IiLB9iITsJdswpxrcRjfpw== X-Gm-Gg: AR+sD12fXyssC4dIB1/NdfZfEX8cjde60+lYIYtzoKPklfrKNwISHHjVROmfdDqCvPH vwUK8xKaHhfC2DMEU4XLboLaObTVjdzyLBLe9CtzQWY8BTlKzAbdQqoz1/4k+J9lBCfoA9OgXbf M4Fw7VduOMyoXl/jcNwTKQ5fw0Pe8mMLp5NKSKQLAvfaWk/y8erzYKKKxU1yutGsyzqdMQTdoA5 XH4CP9/B3Hw4mFJXAkIlRSsKNhNAqyaWgXzvcp+NJ9cms2cL2yg9obqkuOLk3DdNweUdmBc/eGP S86RoF36yf1zttD9JGVgAnU13PTvnVloQRccGJzgu1XKFXjwB30vh9piHrze6r8LV3aJqWHb5py 1e/eKDwCcICl+9IeHlvqEpb3nD4cX3q0= X-Received: by 2002:a05:600c:3ba2:b0:496:c0f6:78d6 with SMTP id 5b1f17b1804b1-4995e07f525mr128342475e9.2.1786177964345; Sat, 08 Aug 2026 01:32:44 -0700 (PDT) X-Received: by 2002:a05:600c:3ba2:b0:496:c0f6:78d6 with SMTP id 5b1f17b1804b1-4995e07f525mr128341815e9.2.1786177963849; Sat, 08 Aug 2026 01:32:43 -0700 (PDT) Received: from redhat.com (bzq-79-177-145-168.red.bezeqint.net. [79.177.145.168]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-480021506cbsm13744768f8f.14.2026.08.08.01.32.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 08 Aug 2026 01:32:43 -0700 (PDT) Date: Sat, 8 Aug 2026 04:32:40 -0400 From: "Michael S. Tsirkin" To: Jakub Kicinski Cc: Xiong Weimin , Jason Wang , Xuan Zhuo , Eugenio =?iso-8859-1?Q?P=E9rez?= , Andrew Lunn , "David S. Miller" , Eric Dumazet , Paolo Abeni , netdev@vger.kernel.org, virtualization@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] virtio_net: roll back RSS state on control failure Message-ID: <20260808042344-mutt-send-email-mst@kernel.org> References: <20260804073654.1293243-1-xiongweimin@kylinos.cn> <20260807181916.63395d4f@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260807181916.63395d4f@kernel.org> On Fri, Aug 07, 2026 at 06:19:16PM -0700, Jakub Kicinski wrote: > 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); kvmemdup maybe, just in case - I think it can get as high as 128k after all. > > + 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;