From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com
[62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 5AE6F2B9E
for <dev@dpdk.org>; Tue, 11 Oct 2016 16:50:02 +0200 (CEST)
Received: from [10.16.0.195] (unknown [10.16.0.195])
by proxy.6wind.com (Postfix) with ESMTP id B480924A78;
Tue, 11 Oct 2016 16:50:01 +0200 (CEST)
To: Maxime Coquelin <maxime.coquelin@redhat.com>, 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>
<65d18f11-9a51-a9d8-a649-6285fcdd0b2b@redhat.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: Olivier MATZ <olivier.matz@6wind.com>
Message-ID: <57FCFC15.6090305@6wind.com>
Date: Tue, 11 Oct 2016 16:49:57 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
Icedove/38.6.0
MIME-Version: 1.0
In-Reply-To: <65d18f11-9a51-a9d8-a649-6285fcdd0b2b@redhat.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 11 Oct 2016 14:50:02 -0000
On 10/11/2016 04:36 PM, Maxime Coquelin wrote:
>
>
> 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?
Yep, good idea.
Thanks!
Olivier