From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A9D77A09D2; Wed, 11 Nov 2020 08:23:14 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EFCC5F64; Wed, 11 Nov 2020 08:23:12 +0100 (CET) Received: from smail.rz.tu-ilmenau.de (smail.rz.tu-ilmenau.de [141.24.186.67]) by dpdk.org (Postfix) with ESMTP id 8E33823D for ; Wed, 11 Nov 2020 08:23:10 +0100 (CET) Received: from [192.168.1.40] (unknown [91.137.63.238]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smail.rz.tu-ilmenau.de (Postfix) with ESMTPSA id 3286F58005E; Wed, 11 Nov 2020 08:23:10 +0100 (CET) Message-ID: <23c6f497eb31840f0a2260bb3b5521fba8c7ec3b.camel@tu-ilmenau.de> From: Michael Pfeiffer To: Ferruh Yigit , Keith Wiles Cc: dev@dpdk.org Date: Wed, 11 Nov 2020 08:23:09 +0100 In-Reply-To: References: <20201109142217.115918-1-michael.pfeiffer@tu-ilmenau.de> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] net/tap: Allow all-zero checksum for UDP over IPv4 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, On Tue, 2020-11-10 at 15:59 +0000, Ferruh Yigit wrote: > On 11/9/2020 2:22 PM, Michael Pfeiffer wrote: > > Unlike TCP, UDP checksums are optional and may be zero to indicate "not > > set" [RFC 768] (except for IPv6, where this prohibited [RFC 8200]). Add > > this special case to the checksum offload emulation in net/tap. > > > > Signed-off-by: Michael Pfeiffer > > --- > >   drivers/net/tap/rte_eth_tap.c | 13 +++++++++++-- > >   1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > > index 2f8abb12c..e486b41c5 100644 > > --- a/drivers/net/tap/rte_eth_tap.c > > +++ b/drivers/net/tap/rte_eth_tap.c > > @@ -303,6 +303,7 @@ tap_verify_csum(struct rte_mbuf *mbuf) > >         uint16_t cksum = 0; > >         void *l3_hdr; > >         void *l4_hdr; > > +       struct rte_udp_hdr *udp_hdr; > >   > >         if (l2 == RTE_PTYPE_L2_ETHER_VLAN) > >                 l2_len += 4; > > @@ -349,10 +350,18 @@ tap_verify_csum(struct rte_mbuf *mbuf) > >                 /* Don't verify checksum for multi-segment packets. */ > >                 if (mbuf->nb_segs > 1) > >                         return; > > -               if (l3 == RTE_PTYPE_L3_IPV4) > > +               if (l3 == RTE_PTYPE_L3_IPV4) { > > +                       if (l4 == RTE_PTYPE_L4_UDP) { > > +                               udp_hdr = (struct rte_udp_hdr *)l4_hdr; > > +                               if (udp_hdr->dgram_cksum == 0) { > > Overall patch looks good to me, but can you please add a comment on top of > above > check to describe why checksum can be zero, as done in the commit log. Sure, I will update the patch. I am also not completely sure whether PKT_RX_L4_CKSUM_NONE is the right flag for this case (rather than _UNKNOWN). >From rte_core_mbuf.h: * - PKT_RX_L4_CKSUM_UNKNOWN: no information about the RX L4 checksum * - PKT_RX_L4_CKSUM_NONE: the L4 checksum is not correct in the packet * data, but the integrity of the L4 data is verified. The second part after the "but" is not really the case here. I don't know how relevant the distinction is, as most application side code will probably only do something like if ((mbuf->ol_flags & PKT_RX_L4_CKSUM_MASK) == PKT_RX_L4_CKSUM_BAD) rte_pktmbuf_free(mbuf); anyway. Do you have any opinions on that? > > +                                       mbuf->ol_flags |= > > PKT_RX_L4_CKSUM_NONE; > > +                                       return; > > +                               } > > +                       } > >                         cksum = ~rte_ipv4_udptcp_cksum(l3_hdr, l4_hdr); > > -               else if (l3 == RTE_PTYPE_L3_IPV6) > > +               } else if (l3 == RTE_PTYPE_L3_IPV6) { > >                         cksum = ~rte_ipv6_udptcp_cksum(l3_hdr, l4_hdr); > > +               } > >                 mbuf->ol_flags |= cksum ? > >                         PKT_RX_L4_CKSUM_BAD : > >                         PKT_RX_L4_CKSUM_GOOD; > > > Regards Michael