From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olivier MATZ <olivier.matz@6wind.com>
Subject: Re: [PATCH v2 04/12] net: add function to calculate a
 checksum in a mbuf
Date: Tue, 11 Oct 2016 15:33:47 +0200
Message-ID: <57FCEA3B.3020601@6wind.com>
References: <1469088510-7552-1-git-send-email-olivier.matz@6wind.com>
 <1475485223-30566-1-git-send-email-olivier.matz@6wind.com>
 <1475485223-30566-5-git-send-email-olivier.matz@6wind.com>
 <b2d7d636-657d-b7da-80ce-97ca1300911e@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
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
To: Maxime Coquelin <maxime.coquelin@redhat.com>, dev@dpdk.org,
 yuanhan.liu@linux.intel.com
Return-path: <dev-bounces@dpdk.org>
Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com
 [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 1F68B6CBC
 for <dev@dpdk.org>; Tue, 11 Oct 2016 15:33:52 +0200 (CEST)
In-Reply-To: <b2d7d636-657d-b7da-80ce-97ca1300911e@redhat.com>
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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Maxime,

On 10/11/2016 03:25 PM, Maxime Coquelin wrote:
>>  /**
>> + * Compute the raw (non complemented) checksum of a packet.
>> + *
>> + * @param m
>> + *   The pointer to the mbuf.
>> + * @param off
>> + *   The offset in bytes to start the checksum.
>> + * @param len
>> + *   The length in bytes of the data to ckecksum.
>> + */
>> +static inline uint16_t
>> +rte_raw_cksum_mbuf(const struct rte_mbuf *m, uint32_t off, uint32_t len)
>> +{
>> +    const struct rte_mbuf *seg;
>> +    const char *buf;
>> +    uint32_t sum, tmp;
>> +    uint32_t seglen, done;
>> +
>> +    /* easy case: all data in the first segment */
>> +    if (off + len <= rte_pktmbuf_data_len(m))
>> +        return rte_raw_cksum(rte_pktmbuf_mtod_offset(m,
>> +                const char *, off), len);
>> +
>> +    if (off + len > rte_pktmbuf_pkt_len(m))
> unlikely?

Yes, will add it.

>> +        return 0; /* invalid params, return a dummy value */
> Couldn't be better to return an error, so that the caller has a chance
> to see it is passing wrong arguments?
> The csum would be passed as an arg.

Looks much better indeed. I'll change it for next revision.


Thanks,
Olivier