From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.warmcat.com (mail.warmcat.com [163.172.24.82]) by dpdk.org (Postfix) with ESMTP id 8098E2952 for ; Tue, 22 May 2018 03:29:50 +0200 (CEST) To: Bruce Richardson Cc: dev@dpdk.org, thomas@monjalon.net References: <152686781484.58694.14737673447518527445.stgit@localhost.localdomain> <152686807841.58694.11466927240402649305.stgit@localhost.localdomain> <20180521122719.GE22944@bricha3-MOBL.ger.corp.intel.com> From: Andy Green Message-ID: Date: Tue, 22 May 2018 09:29:45 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 In-Reply-To: <20180521122719.GE22944@bricha3-MOBL.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 5/8] rte_mbuf.h: avoid implicit demotion in 64-bit arith 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: , X-List-Received-Date: Tue, 22 May 2018 01:29:50 -0000 On 05/21/2018 08:27 PM, Bruce Richardson wrote: > On Mon, May 21, 2018 at 10:01:18AM +0800, Andy Green wrote: >> /projects/lagopus/src/dpdk/build/include/rte_mbuf.h: >> In function 'rte_validate_tx_offload': >> /projects/lagopus/src/dpdk/build/include/rte_mbuf.h: >> 2112:19: warning: conversion to 'uint64_t' >> {aka 'long unsigned int'} from 'int' may change the >> sign of the result [-Wsign-conversion] >> inner_l3_offset += m->outer_l2_len + m->outer_l3_len; >> ^~ >> >> uint64_t inner_l3_offset... >> >> /* fields for TX offloading of tunnels */ >> uint64_t outer_l3_len:9; /**< Outer L3 (IP) Hdr Length. */ >> uint64_t outer_l2_len:7; /**< Outer L2 (MAC) Hdr Length. */ >> >> We want to do the arithmetic entirely in uint64_t >> space, but there is an implicit demotion to int created by >> the +=. Remove the +=. >> >> Fixes: 4fb7e803eb ("ethdev: add Tx preparation") >> Signed-off-by: Andy Green >> --- >> lib/librte_mbuf/rte_mbuf.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> > > Fix looks ok, but given that it's non-obvious why += doesn't work, I think > it would be good to put a comment in explaining it. Otherwise I could see I commnented these and the related stanzas in the second patch you suggested the comment in v7. > someone changing this back in a later patch, because the problem doesn't > arise with regular DPDK compiles Yes the header apis are not actually used just building dpdk, so you don't see any problem. But the problem will come as soon as you try to actually build something else against dpdk + gcc8.1. -Andy > Acked-by: Bruce Richardson >