From patchwork Mon Jun 17 13:15:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heng Qi X-Patchwork-Id: 13700596 X-Patchwork-Delegate: kuba@kernel.org Received: from out30-100.freemail.mail.aliyun.com (out30-100.freemail.mail.aliyun.com [115.124.30.100]) (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 1B1A11E4A4 for ; Mon, 17 Jun 2024 13:15:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.100 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718630137; cv=none; b=D8nopY1FVAVmYJvVAT8cNwAeRvaEt+5wTIz8DZxYj51ik35cU0kEJiMy33AtcCyiy5LPvoga+45Y5DPY0aq4wLwoOvVB/1MZtjX7TJuvlilcsCg3QUBI0mxc5ctqotMLn0QpaWfFWZakd82N912dU/hn/OkdphernxbvQFgj1/w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718630137; c=relaxed/simple; bh=YKWSFpD2t9smMqHR0OzAK7RWGnx1qZClN3jUQUQbWik=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=uAfc/mGmAu2daxcqRpUhHqUUh2hzI49zn1p+UDXT3MELcBqrArvvRsa5CuVkmKsFBS3bLiItmskrGzSFHo1wqQYw/A42V+f9vdEDd3pE69PDapC4AR0Rm7jjvDp1UiDz9apwiVnD6b97XTeeonpvLyEK23Wp1k8wLtzdyqkDAzQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=LwwaE5lY; arc=none smtp.client-ip=115.124.30.100 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="LwwaE5lY" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1718630127; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=gWO6nXsDoeWzECozLUEzwbIZkC4rZqgmByjQv8l6vag=; b=LwwaE5lYuaFhzlzhwNJTI8Fuv3hzUFwaxvUYmk5lqC709E+q6C/equasRGDkp7YoDW7l6D3ENVI/oBhS4yeww/2BbYxEVifKjZESFiS9iNLKiXh5Efe2AjZCw4luPLYzzL49D0z4EfiMNjtN6hlHzAI18NlnwxmDKSQOK3INPmU= X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R111e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033045046011;MF=hengqi@linux.alibaba.com;NM=1;PH=DS;RN=15;SR=0;TI=SMTPD_---0W8enhhB_1718630125; Received: from localhost(mailfrom:hengqi@linux.alibaba.com fp:SMTPD_---0W8enhhB_1718630125) by smtp.aliyun-inc.com; Mon, 17 Jun 2024 21:15:26 +0800 From: Heng Qi To: netdev@vger.kernel.org, virtualization@lists.linux.dev Cc: Thomas Huth , Jason Wang , "Michael S. Tsirkin" , Xuan Zhuo , =?utf-8?q?Eugenio_P=C3=A9rez?= , Eric Dumazet , "David S. Miller" , Jakub Kicinski , Paolo Abeni , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend Subject: [PATCH 1/2] virtio_net: checksum offloading handling fix Date: Mon, 17 Jun 2024 21:15:23 +0800 Message-Id: <20240617131524.63662-2-hengqi@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240617131524.63662-1-hengqi@linux.alibaba.com> References: <20240617131524.63662-1-hengqi@linux.alibaba.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org In virtio spec 0.95, VIRTIO_NET_F_GUEST_CSUM was designed to handle partially checksummed packets, and the validation of fully checksummed packets by the device is independent of VIRTIO_NET_F_GUEST_CSUM negotiation. However, the specification erroneously stated: "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set flags to zero and SHOULD supply a fully checksummed packet to the driver." This statement is inaccurate because even without VIRTIO_NET_F_GUEST_CSUM negotiation, the device can still set the VIRTIO_NET_HDR_F_DATA_VALID flag. Essentially, the device can facilitate the validation of these packets' checksums - a process known as RX checksum offloading - removing the need for the driver to do so. This scenario is currently not implemented in the driver and requires correction. The necessary specification correction[1] has been made and approved in the virtio TC vote. [1] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00011.html Fixes: 4f49129be6fa ("virtio-net: Set RXCSUM feature if GUEST_CSUM is available") Signed-off-by: Heng Qi Reviewed-by: Jiri Pirko Acked-by: Jason Wang --- drivers/net/virtio_net.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 61a57d134544..aa70a7ed8072 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -5666,8 +5666,16 @@ static int virtnet_probe(struct virtio_device *vdev) dev->features |= dev->hw_features & NETIF_F_ALL_TSO; /* (!csum && gso) case will be fixed by register_netdev() */ } - if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM)) - dev->features |= NETIF_F_RXCSUM; + + /* 1. With VIRTIO_NET_F_GUEST_CSUM negotiation, the driver doesn't + * need to calculate checksums for partially checksummed packets, + * as they're considered valid by the upper layer. + * 2. Without VIRTIO_NET_F_GUEST_CSUM negotiation, the driver only + * receives fully checksummed packets. The device may assist in + * validating these packets' checksums, so the driver won't have to. + */ + dev->features |= NETIF_F_RXCSUM; + if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) || virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6)) dev->features |= NETIF_F_GRO_HW; From patchwork Mon Jun 17 13:15:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Heng Qi X-Patchwork-Id: 13700595 X-Patchwork-Delegate: kuba@kernel.org Received: from out30-101.freemail.mail.aliyun.com (out30-101.freemail.mail.aliyun.com [115.124.30.101]) (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 BC32F1DFF4 for ; Mon, 17 Jun 2024 13:15:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.101 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718630133; cv=none; b=YM/V/4wt41J+Tn1SIQVIvRuRH+K6ONc2LlwCfGgo2vOo4JheUwGIZjMzskplj5Y6lNyfKgpSIm9NZSGkkOiMiQdJQlItsHy5t1w9Gvw9Tgna6apQGdOeLiSvuYDTnV9NGBYwWyuMzulzliDRKmIEqJt7un8bSIMI67C+mbNQvf0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718630133; c=relaxed/simple; bh=JC6+JAN40Zy6Tqcb+/Cxp+CoLq5SClRaE2RYAoFywOk=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=e9g7Zahg4m7eImffsjBF0EOn7y/iep5h+1/mhS5+/mjAIOIM+ODaUxiNSnKTj+L/7r1nTHRaTy4gqN4FuE4/NOxxzR2uqHDdUcNsv3TZZCvgb7rfQxc1Kgxmh9MbABEMS8pm2tifNkhD21x3jhQ1ilAF7KL6S916mhEgp17zfVw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=vdjBUJ4t; arc=none smtp.client-ip=115.124.30.101 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="vdjBUJ4t" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1718630128; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=dwhiCYViO2Sb5s0QwKB36F6bgPn0fPUH1OG3Wt9jiU0=; b=vdjBUJ4ty9GhC3pfSJ17GVELSyvHa3GyP6Dxo/roOkqGioZKUCwbfKLAUPtzvAy4DdwB3UEIEN+oZ9VVDtpSbA5TpMiw0174K82FAgn7+v/HLqpmhHSaHyGFpUYyQFxIVOGILs4M4O7KPXaQfwFKWMHDCipQui6+/ls6P1tc7pE= X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R311e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033037067112;MF=hengqi@linux.alibaba.com;NM=1;PH=DS;RN=15;SR=0;TI=SMTPD_---0W8enhha_1718630126; Received: from localhost(mailfrom:hengqi@linux.alibaba.com fp:SMTPD_---0W8enhha_1718630126) by smtp.aliyun-inc.com; Mon, 17 Jun 2024 21:15:27 +0800 From: Heng Qi To: netdev@vger.kernel.org, virtualization@lists.linux.dev Cc: Thomas Huth , Jason Wang , "Michael S. Tsirkin" , Xuan Zhuo , =?utf-8?q?Eugenio_P=C3=A9rez?= , Eric Dumazet , "David S. Miller" , Jakub Kicinski , Paolo Abeni , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend Subject: [PATCH 2/2] virtio_net: fixing XDP for fully checksummed packets handling Date: Mon, 17 Jun 2024 21:15:24 +0800 Message-Id: <20240617131524.63662-3-hengqi@linux.alibaba.com> X-Mailer: git-send-email 2.32.0.3.g01195cf9f In-Reply-To: <20240617131524.63662-1-hengqi@linux.alibaba.com> References: <20240617131524.63662-1-hengqi@linux.alibaba.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org The XDP program can't correctly handle partially checksummed packets, but works fine with fully checksummed packets. If the device has already validated fully checksummed packets, then the driver doesn't need to re-validate them, saving CPU resources. Additionally, the driver does not drop all partially checksummed packets when VIRTIO_NET_F_GUEST_CSUM is not negotiated. This is not a bug, as the driver has always done this. Fixes: 436c9453a1ac ("virtio-net: keep vnet header zeroed after processing XDP") Signed-off-by: Heng Qi --- drivers/net/virtio_net.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index aa70a7ed8072..ea10db9a09fa 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1360,6 +1360,10 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev, if (unlikely(hdr->hdr.gso_type)) goto err_xdp; + /* Partially checksummed packets must be dropped. */ + if (unlikely(hdr->hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) + goto err_xdp; + buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); @@ -1677,6 +1681,10 @@ static void *mergeable_xdp_get_buf(struct virtnet_info *vi, if (unlikely(hdr->hdr.gso_type)) return NULL; + /* Partially checksummed packets must be dropped. */ + if (unlikely(hdr->hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)) + return NULL; + /* Now XDP core assumes frag size is PAGE_SIZE, but buffers * with headroom may add hole in truesize, which * make their length exceed PAGE_SIZE. So we disabled the @@ -1943,6 +1951,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq, struct net_device *dev = vi->dev; struct sk_buff *skb; struct virtio_net_common_hdr *hdr; + u8 flags; if (unlikely(len < vi->hdr_len + ETH_HLEN)) { pr_debug("%s: short packet %i\n", dev->name, len); @@ -1951,6 +1960,15 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq, return; } + /* 1. Save the flags early, as the XDP program might overwrite them. + * These flags ensure packets marked as VIRTIO_NET_HDR_F_DATA_VALID + * stay valid after XDP processing. + * 2. XDP doesn't work with partially checksummed packets (refer to + * virtnet_xdp_set()), so packets marked as + * VIRTIO_NET_HDR_F_NEEDS_CSUM get dropped during XDP processing. + */ + flags = ((struct virtio_net_common_hdr *)buf)->hdr.flags; + if (vi->mergeable_rx_bufs) skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit, stats); @@ -1966,7 +1984,7 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq, if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) virtio_skb_set_hash(&hdr->hash_v1_hdr, skb); - if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID) + if (flags & VIRTIO_NET_HDR_F_DATA_VALID) skb->ip_summed = CHECKSUM_UNNECESSARY; if (virtio_net_hdr_to_skb(skb, &hdr->hdr,