From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 6DBA06936 for ; Tue, 11 Oct 2016 16:36:17 +0200 (CEST) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9CC95A0C45; Tue, 11 Oct 2016 14:36:16 +0000 (UTC) Received: from [10.36.6.57] (vpn1-6-57.ams2.redhat.com [10.36.6.57]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9BEaAi8030327 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 11 Oct 2016 10:36:12 -0400 To: Olivier MATZ , dev@dpdk.org, yuanhan.liu@linux.intel.com References: <1469088510-7552-1-git-send-email-olivier.matz@6wind.com> <1475485223-30566-1-git-send-email-olivier.matz@6wind.com> <1475485223-30566-10-git-send-email-olivier.matz@6wind.com> <9a1ef865-7bff-33cd-5d6e-01ff9e4b1b5a@redhat.com> <57FCF75E.3090205@6wind.com> Cc: konstantin.ananyev@intel.com, sugesh.chandran@intel.com, bruce.richardson@intel.com, jianfeng.tan@intel.com, helin.zhang@intel.com, adrien.mazarguil@6wind.com, stephen@networkplumber.org, dprovan@bivio.net, xiao.w.wang@intel.com From: Maxime Coquelin Message-ID: <65d18f11-9a51-a9d8-a649-6285fcdd0b2b@redhat.com> Date: Tue, 11 Oct 2016 16:36:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <57FCF75E.3090205@6wind.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 11 Oct 2016 14:36:16 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 09/12] virtio: add Rx checksum offload support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Oct 2016 14:36:17 -0000 On 10/11/2016 04:29 PM, Olivier MATZ wrote: > > > On 10/11/2016 04:04 PM, Maxime Coquelin wrote: >>> +/* Optionally fill offload information in structure */ >>> +static int >>> +virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr) >>> +{ >>> + struct rte_net_hdr_lens hdr_lens; >>> + uint32_t hdrlen, ptype; >>> + int l4_supported = 0; >>> + >>> + /* nothing to do */ >>> + if (hdr->flags == 0 && hdr->gso_type == VIRTIO_NET_HDR_GSO_NONE) >>> + return 0; >>> + >>> + m->ol_flags |= PKT_RX_IP_CKSUM_UNKNOWN; >>> + >>> + ptype = rte_net_get_ptype(m, &hdr_lens, RTE_PTYPE_ALL_MASK); >>> + m->packet_type = ptype; >>> + if ((ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP || >>> + (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_UDP || >>> + (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_SCTP) >>> + l4_supported = 1; >>> + >>> + if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) { >>> + hdrlen = hdr_lens.l2_len + hdr_lens.l3_len + hdr_lens.l4_len; >>> + if (hdr->csum_start <= hdrlen && l4_supported) { >>> + m->ol_flags |= PKT_RX_L4_CKSUM_NONE; >>> + } else { >>> + /* Unknown proto or tunnel, do sw cksum. We can assume >>> + * the cksum field is in the first segment since the >>> + * buffers we provided to the host are large enough. >>> + * In case of SCTP, this will be wrong since it's a CRC >>> + * but there's nothing we can do. >>> + */ >>> + uint16_t csum, off; >>> + >>> + csum = rte_raw_cksum_mbuf(m, hdr->csum_start, >>> + rte_pktmbuf_pkt_len(m) - hdr->csum_start); >>> + if (csum != 0xffff) >> Why don't we do the 1-complement if 0xffff? > > This was modified after a comment from Xiao. > > In checksum arithmetic (ones' complement), there are 2 equivalent ways > to say the checksum is 0: 0xffff (0-), and 0x0000 (0+). > Some protocols like UDP use this to differentiate between 0xffff (packet > checksum is 0) and 0x0000 (packet checksum is not calculated). > > Here, we want to avoid to set a checksum to 0, in case it would mean no > checksum for UDP packets. Instead, it is set to 0xffff, which is also a > valid checksum for this packet. Ha ok, I wasn't aware of this. Thanks for the explanation! Maybe not a big deal, but we could add likely around the test? Maxime > > Regards, > Olivier