Message ID | 1417666564-19950-1-git-send-email-michael.qiu@intel.com (mailing list archive) |
---|---|
State | Accepted, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id D536D803A; Thu, 4 Dec 2014 14:34:30 +0100 (CET) Received: from mail-pd0-f173.google.com (mail-pd0-f173.google.com [209.85.192.173]) by dpdk.org (Postfix) with ESMTP id 8105E7EB0 for <dev@dpdk.org>; Thu, 4 Dec 2014 14:34:28 +0100 (CET) Received: by mail-pd0-f173.google.com with SMTP id ft15so17635012pdb.18 for <dev@dpdk.org>; Thu, 04 Dec 2014 05:34:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=ry0p/exYhRaywZRk5DS6ZwKEoe2Ew+BfFR1XND30CZA=; b=ehUcyg5gvPUWlvBE3hCBUtg2zOoCl7H7842Xet5tNBXZHKkfuNllkMbGB4B/mUndFp nY39X+tttTtkJ8BnQkl4e8wp8dF2rHxnPLX2PwzrEwtRVsG+HHHUMasMX5czkgoMQOuZ 4d5dr/2oaelId6+ghcbe8OTIsEdP/3wd0mn/4TULgxamhhajieghwguGlgIR9EGtmivC gwviHIvoY4E4ZUdjqRuJHGyYObUIJGOGeu22gTR7HjbT1oRJ+f7Kc6Va9EUdm0y9HrQh JT/MujGNFenIonNf1ZoQ9qFMWGENI0Jcy8/HKHUcFDtnkqu3/i70SI9ISjV3aFpdry8b WYYg== X-Received: by 10.66.142.137 with SMTP id rw9mr18786417pab.124.1417700067788; Thu, 04 Dec 2014 05:34:27 -0800 (PST) Received: from localhost.localdomain.localdomain ([180.99.189.223]) by mx.google.com with ESMTPSA id ds16sm7391608pdb.65.2014.12.04.05.34.24 for <multiple recipients> (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 04 Dec 2014 05:34:27 -0800 (PST) From: Michael Qiu <qdy220091330@gmail.com> X-Google-Original-From: Michael Qiu <michael.qiu@intel.com> To: dev@dpdk.org Date: Thu, 4 Dec 2014 12:16:04 +0800 Message-Id: <1417666564-19950-1-git-send-email-michael.qiu@intel.com> X-Mailer: git-send-email 1.9.3 In-Reply-To: <1417663711-19576-1-git-send-email-michael.qiu@intel.com> References: <1417663711-19576-1-git-send-email-michael.qiu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v3] test-pmd: Fix pointer aliasing error 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> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Michael Qiu
Dec. 4, 2014, 4:16 a.m. UTC
app/test-pmd/csumonly.c: In function ‘get_psd_sum’:
build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’
does break strict-aliasing rules
build/include/rte_ip.h:157: note: initialized from here
...
The root cause is that, compile enable strict aliasing by default,
while in function rte_raw_cksum() try to convert 'const char *'
to 'const uint16_t *'.
This patch is one workaround fix.
Signed-off-by: Michael Qiu <michael.qiu@intel.com>
---
v3 --> v2:
use uintptr_t instead of unsigned long to
save pointer.
v2 --> v1:
Workaround solution instead of shut off the
gcc params.
lib/librte_net/rte_ip.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
Any comments about this version? a new workaround solution :) Thanks, Michael On 12/4/2014 9:35 PM, Michael Qiu wrote: > app/test-pmd/csumonly.c: In function ‘get_psd_sum’: > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’ > does break strict-aliasing rules > build/include/rte_ip.h:157: note: initialized from here > ... > > The root cause is that, compile enable strict aliasing by default, > while in function rte_raw_cksum() try to convert 'const char *' > to 'const uint16_t *'. > > This patch is one workaround fix. > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> > --- > v3 --> v2: > use uintptr_t instead of unsigned long to > save pointer. > > v2 --> v1: > Workaround solution instead of shut off the > gcc params. > > lib/librte_net/rte_ip.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h > index 61e4457..cda3436 100644 > --- a/lib/librte_net/rte_ip.h > +++ b/lib/librte_net/rte_ip.h > @@ -154,7 +154,8 @@ struct ipv4_hdr { > static inline uint16_t > rte_raw_cksum(const char *buf, size_t len) > { > - const uint16_t *u16 = (const uint16_t *)buf; > + uintptr_t ptr = (uintptr_t)buf; > + const uint16_t *u16 = (const uint16_t *)ptr; > uint32_t sum = 0; > > while (len >= (sizeof(*u16) * 4)) {
2014-12-05 05:34, Qiu, Michael: > Any comments about this version? a new workaround solution :) Yes, one comment: I think it's ugly :) These aliasing errors are not reliable so I think we can disable it (like Linux does). But in case you don't want to disable the warning, please add a comment to your workaround to explain it is caused by GCC strict-aliasing check. > > - const uint16_t *u16 = (const uint16_t *)buf; > > + uintptr_t ptr = (uintptr_t)buf; > > + const uint16_t *u16 = (const uint16_t *)ptr; Thanks
On 12/5/2014 5:26 PM, Thomas Monjalon wrote: > 2014-12-05 05:34, Qiu, Michael: >> Any comments about this version? a new workaround solution :) > Yes, one comment: I think it's ugly :) > These aliasing errors are not reliable so I think we can disable it (like > Linux does). > But in case you don't want to disable the warning, please add a comment to your > workaround to explain it is caused by GCC strict-aliasing check. Yes, really ugly .... But lots of reviewer voted against with disable it as my first version is disable. I could not think out a better solution for all reviewers can accept. I will add the comment with the thread of v3 patch. Thanks, Michael >>> - const uint16_t *u16 = (const uint16_t *)buf; >>> + uintptr_t ptr = (uintptr_t)buf; >>> + const uint16_t *u16 = (const uint16_t *)ptr; > Thanks
On 12/4/2014 9:35 PM, Michael Qiu wrote: > app/test-pmd/csumonly.c: In function ‘get_psd_sum’: > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’ > does break strict-aliasing rules > build/include/rte_ip.h:157: note: initialized from here > ... > > The root cause is that, compile enable strict aliasing by default, > while in function rte_raw_cksum() try to convert 'const char *' > to 'const uint16_t *'. > > This patch is one workaround fix. > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> > --- > v3 --> v2: > use uintptr_t instead of unsigned long to > save pointer. > > v2 --> v1: > Workaround solution instead of shut off the > gcc params. > > lib/librte_net/rte_ip.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h > index 61e4457..cda3436 100644 > --- a/lib/librte_net/rte_ip.h > +++ b/lib/librte_net/rte_ip.h > @@ -154,7 +154,8 @@ struct ipv4_hdr { > static inline uint16_t > rte_raw_cksum(const char *buf, size_t len) > { > - const uint16_t *u16 = (const uint16_t *)buf; > + uintptr_t ptr = (uintptr_t)buf; > + const uint16_t *u16 = (const uint16_t *)ptr; > uint32_t sum = 0; > > while (len >= (sizeof(*u16) * 4)) { This workaround is to solve the compile issue of GCC strict-aliasing(Two different type pointers should not be point to the same memory address). For GCC 4.4.7 it will definitely occurs if flags "-fstrict-aliasing" and "-Wall" used. Thanks, Michael
Hi Thomas, What's going on with this patch? I really do not have other better solution. Thanks, Michael On 12/8/2014 9:30 AM, Qiu, Michael wrote: > On 12/4/2014 9:35 PM, Michael Qiu wrote: >> app/test-pmd/csumonly.c: In function ‘get_psd_sum’: >> build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’ >> does break strict-aliasing rules >> build/include/rte_ip.h:157: note: initialized from here >> ... >> >> The root cause is that, compile enable strict aliasing by default, >> while in function rte_raw_cksum() try to convert 'const char *' >> to 'const uint16_t *'. >> >> This patch is one workaround fix. >> >> Signed-off-by: Michael Qiu <michael.qiu@intel.com> >> --- >> v3 --> v2: >> use uintptr_t instead of unsigned long to >> save pointer. >> >> v2 --> v1: >> Workaround solution instead of shut off the >> gcc params. >> >> lib/librte_net/rte_ip.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h >> index 61e4457..cda3436 100644 >> --- a/lib/librte_net/rte_ip.h >> +++ b/lib/librte_net/rte_ip.h >> @@ -154,7 +154,8 @@ struct ipv4_hdr { >> static inline uint16_t >> rte_raw_cksum(const char *buf, size_t len) >> { >> - const uint16_t *u16 = (const uint16_t *)buf; >> + uintptr_t ptr = (uintptr_t)buf; >> + const uint16_t *u16 = (const uint16_t *)ptr; >> uint32_t sum = 0; >> >> while (len >= (sizeof(*u16) * 4)) { > This workaround is to solve the compile issue of GCC strict-aliasing(Two > different type pointers should not be point to the same memory address). > > For GCC 4.4.7 it will definitely occurs if flags "-fstrict-aliasing" > and "-Wall" used. > > Thanks, > Michael >
> > app/test-pmd/csumonly.c: In function ‘get_psd_sum’: > > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’ > > does break strict-aliasing rules > > build/include/rte_ip.h:157: note: initialized from here > > ... > > > > The root cause is that, compile enable strict aliasing by default, > > while in function rte_raw_cksum() try to convert 'const char *' > > to 'const uint16_t *'. > > > > This patch is one workaround fix. > > > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> > > --- > > v3 --> v2: > > use uintptr_t instead of unsigned long to > > save pointer. > > > > v2 --> v1: > > Workaround solution instead of shut off the > > gcc params. > > This workaround is to solve the compile issue of GCC strict-aliasing(Two > different type pointers should not be point to the same memory address). > > For GCC 4.4.7 it will definitely occurs if flags "-fstrict-aliasing" > and "-Wall" used. Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>> Applied with a comment in the code. Thanks
Thomas, Michael, Wouldn't it cause unaligned memory access (new changes as well as the previous code)? Wondering if get_unaligned/put_unaligned macros similar to the ones used in kernel be ported to user-space? Thanks, Ravi On Wed, Dec 10, 2014 at 4:54 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote: > > > > app/test-pmd/csumonly.c: In function 'get_psd_sum': > > > build/include/rte_ip.h:161: error: dereferencing pointer 'u16' > > > does break strict-aliasing rules > > > build/include/rte_ip.h:157: note: initialized from here > > > ... > > > > > > The root cause is that, compile enable strict aliasing by default, > > > while in function rte_raw_cksum() try to convert 'const char *' > > > to 'const uint16_t *'. > > > > > > This patch is one workaround fix. > > > > > > Signed-off-by: Michael Qiu <michael.qiu@intel.com> > > > --- > > > v3 --> v2: > > > use uintptr_t instead of unsigned long to > > > save pointer. > > > > > > v2 --> v1: > > > Workaround solution instead of shut off the > > > gcc params. > > > > This workaround is to solve the compile issue of GCC strict-aliasing(Two > > different type pointers should not be point to the same memory address). > > > > For GCC 4.4.7 it will definitely occurs if flags "-fstrict-aliasing" > > and "-Wall" used. > > Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>> > > Applied with a comment in the code. > > Thanks > -- > Thomas >
On 2014/12/12 1:51, r k wrote: Thomas, Michael, Wouldn't it cause unaligned memory access (new changes as well as the previous code)? Wondering if get_unaligned/put_unaligned macros similar to the ones used in kernel be ported to user-space? I think it will not, as all buf point to are struct udp_hdr/struct tcp_hdr/struct ipv6_psd_header/struct ipv4_psd_header, they are all aligned with uint16_t. Thanks Michael Thanks, Ravi On Wed, Dec 10, 2014 at 4:54 PM, Thomas Monjalon <thomas.monjalon@6wind.com<mailto:thomas.monjalon@6wind.com>> wrote: > > app/test-pmd/csumonly.c: In function ‘get_psd_sum’: > > build/include/rte_ip.h:161: error: dereferencing pointer ‘u16’ > > does break strict-aliasing rules > > build/include/rte_ip.h:157: note: initialized from here > > ... > > > > The root cause is that, compile enable strict aliasing by default, > > while in function rte_raw_cksum() try to convert 'const char *' > > to 'const uint16_t *'. > > > > This patch is one workaround fix. > > > > Signed-off-by: Michael Qiu <michael.qiu@intel.com<mailto:michael.qiu@intel.com>> > > --- > > v3 --> v2: > > use uintptr_t instead of unsigned long to > > save pointer. > > > > v2 --> v1: > > Workaround solution instead of shut off the > > gcc params. > > This workaround is to solve the compile issue of GCC strict-aliasing(Two > different type pointers should not be point to the same memory address). > > For GCC 4.4.7 it will definitely occurs if flags "-fstrict-aliasing" > and "-Wall" used. Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com<mailto:thomas.monjalon@6wind.com>>> Applied with a comment in the code. Thanks -- Thomas
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h index 61e4457..cda3436 100644 --- a/lib/librte_net/rte_ip.h +++ b/lib/librte_net/rte_ip.h @@ -154,7 +154,8 @@ struct ipv4_hdr { static inline uint16_t rte_raw_cksum(const char *buf, size_t len) { - const uint16_t *u16 = (const uint16_t *)buf; + uintptr_t ptr = (uintptr_t)buf; + const uint16_t *u16 = (const uint16_t *)ptr; uint32_t sum = 0; while (len >= (sizeof(*u16) * 4)) {