From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 65089A00C5; Tue, 15 Feb 2022 21:09:08 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 370E1410F7; Tue, 15 Feb 2022 21:09:08 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 85587410F3 for ; Tue, 15 Feb 2022 21:09:06 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1644955746; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=D9CtuQoViApN88vyE67cXI8BMBELLr5R/AcZX06SWxk=; b=Ft1ly5lSmbIOL/mQGdxyCWrHsMKvHrCi+mibya6/o5aE1nFOSk8fByokjUFcO6Q1rFSzoj QRtn2y/LJ7fePY+XekXOWsCIWq00UhgUESzCHyEIcLvjz/yHb7opsAp9RKYGExbIWA6URP tbotcn8VHmM4AGbr0X2SbnO1AVRWndQ= Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-533-l1k0yBP1PQuqZUzjXcUihw-1; Tue, 15 Feb 2022 15:08:56 -0500 X-MC-Unique: l1k0yBP1PQuqZUzjXcUihw-1 Received: by mail-lf1-f70.google.com with SMTP id v13-20020ac2592d000000b004435f5315dbso1384719lfi.21 for ; Tue, 15 Feb 2022 12:08:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=D9CtuQoViApN88vyE67cXI8BMBELLr5R/AcZX06SWxk=; b=br5bfqxrwq97rViX+GoRD6vT8Njvh390D7F7IsGPzzagTWMt/usWn3nog2dsra9//3 JYqjADXkR073wDcN+WDspwhbwHDyD6EE+nMigBsMsBItFkZKGsQmUnoU6efJ7omQMK6r 1xZl5BXswGrVu88g1lctNxag8d4ilKjyjdye+5htftoA0E8dLLIdIGSmZz4JEGr1DEqV dOQ57EJt9IwOHHjk/HeF3e5vMixPskxeyCr/xhwHnocV8Vj1WoVDcMcYe40HhoSZpv6n vB61sbei5rskceR373Xdbm7MqBgV6Uqq7ljnp7+RafLR0RnLW6M+3tDTsO/XjZSqxtMu TyoA== X-Gm-Message-State: AOAM532RnzeCy7iO8ff/GIYZaV/0mj0BFzK30tJ0EF42Yp1avay313C+ NxPjNIWXtqj8lqBLYJfgJOKgJsGOslk0KEKlRqeehXMCv9HWTr14p4pTaUlsQKopyeJ3Pr0os2I usisvJZhVsMwY1cL2etw= X-Received: by 2002:a05:6512:3d14:: with SMTP id d20mr554988lfv.499.1644955735303; Tue, 15 Feb 2022 12:08:55 -0800 (PST) X-Google-Smtp-Source: ABdhPJzmbdeDsj/exOY2buCJNkrELkEfPmrE1fjU0Yq3RvC/gKRZsJS84ldCUSEYHbsaTQDZLei4EWEFgV9PzP1+Mv8= X-Received: by 2002:a05:6512:3d14:: with SMTP id d20mr554962lfv.499.1644955734741; Tue, 15 Feb 2022 12:08:54 -0800 (PST) MIME-Version: 1.0 References: <20220107115312.280036-1-baymaxhuang@gmail.com> In-Reply-To: <20220107115312.280036-1-baymaxhuang@gmail.com> From: David Marchand Date: Tue, 15 Feb 2022 21:08:43 +0100 Message-ID: Subject: Re: [PATCH] net/virtio: include ipv4 cksum to support cksum offload capability To: Harold Huang , Olivier Matz Cc: dev , Maxime Coquelin , Chenbo Xia Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Adding Olivier who, iirc, worked on those offloads a long time ago. On Fri, Jan 7, 2022 at 12:54 PM Harold Huang wrote: > > Device cksum offload capability usually include ipv4 cksum, tcp and udp > cksum offload capability. The application such as OVS usually negotiate > with the drive like this to enable cksum offload. OVS checksum offloading is still being worked on. By this, I mean neither that it is fully functional nor completely broken. But important to keep in mind that it is still a work in progress. > > Signed-off-by: Harold Huang > --- > drivers/net/virtio/virtio_ethdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virt= io_ethdev.c > index c2588369b2..65b03bf0e4 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct= rte_eth_dev_info *dev_info) > dev_info->rx_offload_capa |=3D RTE_ETH_RX_OFFLOAD_SCATTER= ; > if (host_features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) { > dev_info->rx_offload_capa |=3D > + RTE_ETH_RX_OFFLOAD_IPV4_CKSUM | > RTE_ETH_RX_OFFLOAD_TCP_CKSUM | > RTE_ETH_RX_OFFLOAD_UDP_CKSUM; > } Adding this capability alone probably has no effect visible to a dpdk application. For L3, virtio rx handlers never sets anything but RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN. https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtio_rxtx.c#n920 As a consequence, the application can't tell if the ip checksum is correct and the application must validate the checksum of packets on its own. Maybe we could add some RTE_MBUF_F_RX_IP_CKSUM_GOOD in case of TCP/UDP (?) https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtio_rxtx.c#n932 But simply reporting the capability looks wrong to me. > @@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct= rte_eth_dev_info *dev_info) > RTE_ETH_TX_OFFLOAD_VLAN_INSERT; > if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) { > dev_info->tx_offload_capa |=3D > + RTE_ETH_TX_OFFLOAD_IPV4_CKSUM | > RTE_ETH_TX_OFFLOAD_UDP_CKSUM | > RTE_ETH_TX_OFFLOAD_TCP_CKSUM; > } For the tx part, I am a bit unsure, but here is what I understand. With such a capability, the application may think it can send the packet with no checksum, neither ip, nor tcp. The virtio specification writes that the driver must make sure the ip checksum is correct if it wants to ask for L4 checksum. """ If the VIRTIO_NET_F_CSUM feature has been negotiated, the driver MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in flags, if so: the driver MUST validate the packet checksum at offset csum_offset from csum_start as well as all preceding offsets; the driver MUST set the packet checksum stored in the buffer to the TCP/UDP pseudo header; the driver MUST set csum_start and csum_offset such that calculating a ones=E2=80=99 complement checksum from csum_start up until the end of the packet and storing the result at offset csum_offset from csum_start will result in a fully checksummed packet; """ But, nothing in the virtio tx handlers deals with ip cheksum offloading. https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtqueue.h#n664 So here too, I think it is wrong to report such a capability. --=20 David Marchand