From mboxrd@z Thu Jan  1 00:00:00 1970
From: Olivier Matz <olivier.matz@6wind.com>
Subject: Re: librte_net/rte_ip.h checksum functions question
Date: Thu, 20 Jul 2017 11:09:17 +0200
Message-ID: <20170720110917.4ec188eb@platinum>
References: <4E60939B-45FA-45C6-B6C7-1643CB6285D2@cisco.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: "konstantin.ananyev@intel.com" <konstantin.ananyev@intel.com>, "Hanoch
 Haim (hhaim)" <hhaim@cisco.com>, "dev@dpdk.org" <dev@dpdk.org>
To: "Ido Barnea (ibarnea)" <ibarnea@cisco.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wr0-f169.google.com (mail-wr0-f169.google.com
 [209.85.128.169]) by dpdk.org (Postfix) with ESMTP id 1001E25A1
 for <dev@dpdk.org>; Thu, 20 Jul 2017 11:09:20 +0200 (CEST)
Received: by mail-wr0-f169.google.com with SMTP id 12so68156893wrb.1
 for <dev@dpdk.org>; Thu, 20 Jul 2017 02:09:20 -0700 (PDT)
In-Reply-To: <4E60939B-45FA-45C6-B6C7-1643CB6285D2@cisco.com>
List-Id: DPDK patches and discussions <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 Ido,

On Tue, 18 Jul 2017 12:18:20 +0000, "Ido Barnea (ibarnea)" <ibarnea@cisco.com> wrote:
> Hi,
> Is it intentional that rte_ipv4_phdr_cksum and others in this file assume that ipv4 header is sizeof(struct ipv4_hdr), actually assuming that there are no IPv4 options?
> If not, I would like to submit a patch changing that.
> Something in the line of:
> sizeof(struct ipv4_hdr) changes to ipv4_hdr->version_ihl & IPV4_HDR_IHL_MASK) * IPV4_IHL_MULTIPLIER)
> 

I agree we should have other functions that take the real ip header
len in account. What about the following:

static inline uint16_t
rte_ipv4ext_cksum(const struct ipv4_hdr *ipv4_hdr, size_t hdrlen)

static inline uint16_t
rte_ipv4ext_phdr_cksum(const struct ipv4_hdr *ipv4_hdr, size_t hdrlen,
	uint64_t ol_flags)
...

The current function rte_ipv4_cksum() can call rte_ipv4ext_cksum(.., 5)
without additional cost, since 5 will be a constant, resolved at
compilation time.

I also noticed that some functions are not properly documented (it should
be announced that options are not supported). Also, the api comment
of rte_ipv6_udptcp_cksum() talks about ipv4...


Thanks,
Olivier