From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-eopbgr60047.outbound.protection.outlook.com [40.107.6.47]) by dpdk.org (Postfix) with ESMTP id 2B2E77CB0 for ; Thu, 1 Nov 2018 19:24:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector1-arm-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4aGj9hlIoV1Ic+DdZVRzHajdXgRShfplfL53Xka4G7M=; b=c+s70rneCH0G/TxKSrCPH7OR7c9gXd9fLuI4V3nHemngi8xfX77PkAavTDtUCKiir992BDSMJlQQ+f7sqPEvbp888MsAZzBa/aK6/t0DdYAZ5PtouDyiwvo4evL5NxvwDLCGg6NLZ5+YwcmBXfQ9JJO9i+oVQ+F0Ikm/DEcqi1c= Received: from DB6PR0801MB1864.eurprd08.prod.outlook.com (10.168.85.10) by DB6PR0801MB1813.eurprd08.prod.outlook.com (10.169.227.135) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1294.20; Thu, 1 Nov 2018 18:24:33 +0000 Received: from DB6PR0801MB1864.eurprd08.prod.outlook.com ([fe80::acb6:b973:48a9:792]) by DB6PR0801MB1864.eurprd08.prod.outlook.com ([fe80::acb6:b973:48a9:792%2]) with mapi id 15.20.1273.030; Thu, 1 Nov 2018 18:24:33 +0000 From: Malvika Gupta To: "Ananyev, Konstantin" CC: "dev@dpdk.org" , "Gavin Hu (Arm Technology China)" , Honnappa Nagarahalli , nd Thread-Topic: [PATCH] test/bpf: use hton instead of __builtin_bswap Thread-Index: AQHUPxBK9GM4qmAjxku+s3XgHfeEQqTgMAWAgAIP03CACssPIIA5wlCAgAmquSCAA4rSAIAHneIg Date: Thu, 1 Nov 2018 18:24:33 +0000 Message-ID: References: <20180828204620.1862-1-Malvika.Gupta@arm.com> <2601191342CEEE43887BDE71AB977258EA9511E7@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580102FE9DA4@IRSMSX106.ger.corp.intel.com> <2601191342CEEE43887BDE71AB97725801030618AD@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB97725801030618AD@irsmsx105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Malvika.Gupta@arm.com; x-originating-ip: [217.140.111.135] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB6PR0801MB1813; 6:5z1Oz/3qVbZPhGEZJAF3RKW9AXu6j6guVwyq6UkhMbJuqIZjgqLbs52JxWVP4eFrsLETN6ONyIEQC21v1s7SDBNSc6FJ1c79oGEwNK/EelfZrgIvfcUQGy5VaCavXevmD0MqP+buJsWw1xum4U8g4R1ukAYMeHLQfGuoCvi56BsCqBMSKemc+gKpTFUp1agPzI64ORJNj+d3L44b9lCoOQ3MU32UyRgF9GyeV3vhv8hvU3hxHXMtYrdyiArvHGUjaM5CvKAqx7ebHqLAregLtleJI3hx/DpWiA3yXzv5FyDFLm52kQHxzLcwd/F9R1nuHNvWT62esy31fHnIUvvpABmQybZ20E0gCRHy2hD7sFVP/GkJ7qYPnd11hAH5H1e2zyiuVAFJpvFegfWhE4x/m0gnRPIhCkcHrh+cWr/P90/jIFzp8E9JQMat/L/GneJB7JDBM9t99fIs9Me6ygNaoQ==; 5:fN/oe+lB+Ftb8Y/mxBsgUb4NIYSMcSlq00YP1UyU2o9CpMB65oJ3ePFfCRvZsGyFnz6+VmmIFVLXbkjg81iuvixXD8HD2U733mhjrX0KpKK7rEDC5Z9LcK3xl8eBl14+3Ejs9UoP6O1lAHUmFbA+jP5xv2+LBnSCCTVIYGsWB44=; 7:sD1Th3rHXrSskNG7kabE4SN5I+9vr243XDgNCQ7WOOmbxZD26qit/X1o7PGcsJG4aFwg8tZLyL61+dGqUVPG8lNsCZgLuecG4cKCrcAzFFq+hjKUK6zLUxZW8zfVyYW257gOO/Sy292Nsj4n5Dg2DQ== x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR; x-ms-office365-filtering-correlation-id: 78238678-09da-4c0e-67db-08d6402745b0 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:DB6PR0801MB1813; x-ms-traffictypediagnostic: DB6PR0801MB1813: nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(228905959029699)(180628864354917); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3002001)(3231382)(944501410)(52105095)(6055026)(148016)(149066)(150057)(6041310)(20161123558120)(20161123562045)(20161123564045)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:DB6PR0801MB1813; BCL:0; PCL:0; RULEID:; SRVR:DB6PR0801MB1813; x-forefront-prvs: 0843C17679 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(396003)(376002)(136003)(346002)(39860400002)(13464003)(199004)(189003)(97736004)(68736007)(2900100001)(54906003)(5250100002)(316002)(6116002)(3846002)(186003)(6246003)(5660300001)(305945005)(2906002)(256004)(14444005)(74316002)(6506007)(99286004)(8936002)(26005)(102836004)(7696005)(76176011)(7736002)(93886005)(53546011)(6916009)(8676002)(14454004)(11346002)(25786009)(229853002)(4326008)(4744004)(478600001)(72206003)(86362001)(486006)(9686003)(476003)(71200400001)(446003)(33656002)(6436002)(71190400001)(53936002)(106356001)(105586002)(81156014)(55016002)(81166006)(66066001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB6PR0801MB1813; H:DB6PR0801MB1864.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: 5CC7qJADAL3udcdP1ewAM1n2/Cu8lu/i+tWlMc51UG2BJNB3V7VVSA1fgK2sX4TKsGfa0nLNJP8cdlwuy5b9Cw9fUGOBU1fvcjpIwRD32BrWKhaI/FsojiScMHTe5ehogQ95xbI78x+aKiFhnqeYsw2ouVId2yjI82tsDopo/2xI2IvsirT9qAtRQcAJsoMXz+s2z+9pfrrHKy/YL38wFjVSK1NmilGHeSB82dLU0ieeM/0cX9xJbYhBDk2MowjWlsswGAa9+bDBuYFlnweLm9ybKts6u7UGeTLW3nrUs6A8SMHtdIy2XEmAqZUyNgat9K/nQVDE7NdrJrDXhKagOz3ne5urSQ6NkfFT+JQhhSM= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: 78238678-09da-4c0e-67db-08d6402745b0 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Nov 2018 18:24:33.4873 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0801MB1813 Subject: Re: [dpdk-dev] [PATCH] test/bpf: use hton instead of __builtin_bswap 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: Thu, 01 Nov 2018 18:24:35 -0000 > > Hi Konstantin, > > > > At time of upstreaming the patch, I had compiled it on Cortex-A72 and i= t > had worked with the -I parameter. >=20 > Just curious what is memory ordering on your box (BE/LE)? =20 The memory ordering on my machine was little endian > > But when tried building it on > > x86-64, I encountered the error that you were facing. From my > > understanding of this problem, this is a portability issue. As you poin= ted > out, gcc and clang implement __builtin_constant_p in a different way on x= 86 > but that may not be true for ARM. > > > > In such a case, to enable compilation on x86, we may want to write > > specific hton and ntoh functions. But this would become a tedious task > > because, in future, to enable compilation on other architectures beside= s > x86 and ARM, we may have to repeat this process of writing analogs for ea= ch > architecture. >=20 > Don't see why we need it done for each arch. > Obviously all is needed is BE and LE version. >=20 > > I think the best solution in the current scenario would be to use the > > -U parameter to enable compilation on > > x86 and later decide if we want to write a new patch with analogs for > builtin hton() and ntoh(). >=20 > I suppose -U__GNUC__ is ok. > If you decide to go that way, please update appropriate comments in *.c. I will update the comments in the .c files and submit the patch again. Thanks, Malvika=20 =20 > Konstantin >=20 > > > > Best, > > Malvika > > > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Friday, October 19, 2018 7:17 AM > > To: Malvika Gupta > > Cc: dev@dpdk.org; Gavin Hu (Arm Technology China) > ; > > Honnappa Nagarahalli ; nd > > > Subject: RE: [PATCH] test/bpf: use hton instead of __builtin_bswap > > > > > > Hi Malvika, > > > > > > > > Hi Konstantin, > > > > > > I installed the clang version 4.0.1 to check for the issue you were > > > facing with -O2 compilation. I was able to compile with -O2 and -O0 > > > optimization without any errors. Please see the exact command I > > used and the following output for your reference: > > > > > > $ clang -O2 -target bpf -I /usr/include/aarch64-linux-gnu/ -c t1.c $ > > > clang -O0 -target bpf -I /usr/include/aarch64-linux-gnu/ -c t1.c $ > > > clang -v clang version 4.0.1-10 (tags/RELEASE_401/final) > > > Target: aarch64-unknown-linux-gnu > > > Thread model: posix > > > InstalledDir: /usr/bin > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/6 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/6.4.0 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/7 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/7.3.0 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/8 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/8.0.1 > > > Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/6 > > > Found candidate GCC installation: > > > /usr/lib/gcc/aarch64-linux-gnu/6.4.0 > > > Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/7 > > > Found candidate GCC installation: > > > /usr/lib/gcc/aarch64-linux-gnu/7.3.0 > > > Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/8 > > > Found candidate GCC installation: > > > /usr/lib/gcc/aarch64-linux-gnu/8.0.1 > > > Selected GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/7.3.0 > > > Candidate multilib: .;@m64 > > > Selected multilib: .;@m64 > > > > > > I hope this was helpful to you. > > > > Sorry for delay. > > I tried with clang 6.0, but still seeing same issue. > > Digging a bit more, I think the culprit is here: > > > > #ifdef __GNUC__ > > # if __GNUC__ >=3D 2 > > # define __bswap_16(x) \ > > (__extension__ = \ > > ({ unsigned short int __v, __x =3D (unsigned short int) (x); = \ > > if (__builtin_constant_p (__x)) = \ <------ evaluates > to FALSE !!! > > __v =3D __bswap_constant_16 (__x); = \ > > else = \ > > __asm__ ("rorw $8, %w0" = \ > > : "=3Dr" (__v) = \ > > : "0" (__x) = \ > > : "cc"); = \ > > __v; })) > > # else > > /* This is better than nothing. */ > > # define __bswap_16(x) \ > > (__extension__ = \ > > ({ unsigned short int __x =3D (unsigned short int) (x); = \ > > __bswap_constant_16 (__x); })) # endif #else static __inline > > unsigned short int > > __bswap_16 (unsigned short int __bsx) > > { > > return __bswap_constant_16 (__bsx); > > } > > #endif > > > > Seems that gcc and clang implements __builtin_constant_p in a different > way. > > Let say, the following function: > > int bbb1(void) {uint16_t x=3D0; return__bswap_constant_16(x);} when > compiled with gcc would return 1, while when compiled with clang - 0. > > Adding 'const' to definition of 'x' cures the problem for clang. > > I don't know why there is no 'const' in original __bswap_16() macro. > > To overcome that problem, I used: > > clang -O2 -U__GNUC__ -target bpf -c t1.c > > > > Another possible option - define our own analogs for buitin hton*(). > > Konstantin > > > > > > > Best Regards, > > > Malvika Gupta > > > > > > > > > -----Original Message----- > > > From: Malvika Gupta > > > Sent: Wednesday, September 5, 2018 4:43 PM > > > To: Ananyev, Konstantin > > > Cc: dev@dpdk.org; Gavin Hu (Arm Technology China) > > > ; Honnappa Nagarahalli > > > ; Brian Brooks > ; > > > nd > > > Subject: RE: [PATCH] test/bpf: use hton instead of __builtin_bswap > > > > > > Hi Ananyev, > > > > > > I used clang version 6.0.0. Please see the following output for your > reference. > > > > > > $ clang -v > > > clang version 6.0.0-1ubuntu2 (tags/RELEASE_600/final) > > > Target: aarch64-unknown-linux-gnu > > > Thread model: posix > > > InstalledDir: /usr/bin > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/6 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/6.4.0 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/7 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/7.3.0 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/8 > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/8.0.1 > > > Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/6 > > > Found candidate GCC installation: > > > /usr/lib/gcc/aarch64-linux-gnu/6.4.0 > > > Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/7 > > > Found candidate GCC installation: > > > /usr/lib/gcc/aarch64-linux-gnu/7.3.0 > > > Found candidate GCC installation: /usr/lib/gcc/aarch64-linux-gnu/8 > > > Found candidate GCC installation: > > > /usr/lib/gcc/aarch64-linux-gnu/8.0.1 > > > Selected GCC installation: > > > /usr/bin/../lib/gcc/aarch64-linux-gnu/7.3.0 > > > Candidate multilib: .;@m64 > > > Selected multilib: .;@m64 > > > > > > Also, the code compiles with both -O2 and -O0 for me. > > > > > > I hope this was helpful > > > Best, > > > Malvika > > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Tuesday, September 4, 2018 8:56 AM > > > To: Malvika Gupta > > > Cc: dev@dpdk.org; Gavin Hu (Arm Technology China) > > > ; Honnappa Nagarahalli > > > ; Brian Brooks > ; > > > nd > > > Subject: RE: [PATCH] test/bpf: use hton instead of __builtin_bswap > > > > > > Hi, > > > > > > > > > > > Convert host machine endianness to networking endianness for > > > > comparison of incoming packets with BPF filter > > > > > > > > > > > > Signed-off-by: Malvika Gupta > > > > Reviewed-by: Gavin Hu > > > > Reviewed-by: Brian Brooks > > > > Suggested-by: Brian Brooks > > > > --- > > > > test/bpf/t1.c | 7 ++++--- > > > > test/bpf/t3.c | 3 ++- > > > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/test/bpf/t1.c b/test/bpf/t1.c index > > > > 60f9434ab..7943fcf34 > > > > 100644 > > > > --- a/test/bpf/t1.c > > > > +++ b/test/bpf/t1.c > > > > @@ -28,24 +28,25 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > uint64_t > > > > entry(void *pkt) > > > > { > > > > struct ether_header *ether_header =3D (void *)pkt; > > > > > > > > - if (ether_header->ether_type !=3D __builtin_bswap16(0x0800)) > > > > + if (ether_header->ether_type !=3D htons(0x0800)) > > > > > > Which version of clang do you use? > > > With my one I get: > > > $ clang -O2 -target bpf -c t1.c > > > t1.c:37:34: error: couldn't allocate output register for constraint '= r' > > > if (ether_header->ether_type !=3D ntohs(0x0800)) > > > ^ > > > /usr/include/netinet/in.h:402:21: note: expanded from macro 'ntohs' > > > # define ntohs(x) __bswap_16 (x) > > > ^ > > > /usr/include/bits/byteswap-16.h:31:14: note: expanded from macro > '__bswap_16' > > > __asm__ ("rorw $8, %w0" > > > > > > With '-O0' it compiles ok. > > > > > > $ clang -v > > > clang version 4.0.1 (tags/RELEASE_401/final) > > > Target: x86_64-unknown-linux-gnu > > > Thread model: posix > > > InstalledDir: /usr/bin > > > Found candidate GCC installation: > > > /usr/bin/../lib/gcc/x86_64-redhat-linux/7 > > > Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/7 > > > Selected GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/7 > > > Candidate multilib: .;@m64 > > > Candidate multilib: 32;@m32 > > > Selected multilib: .;@m64 > > > > > > Konstantin > > > > > > > return 0; > > > > > > > > struct iphdr *iphdr =3D (void *)(ether_header + 1); > > > > if (iphdr->protocol !=3D 17 || (iphdr->frag_off & 0x1ffff) !=3D 0= || > > > > - iphdr->daddr !=3D __builtin_bswap32(0x1020304)) > > > > + iphdr->daddr !=3D htonl(0x1020304)) > > > > return 0; > > > > > > > > int hlen =3D iphdr->ihl * 4; > > > > struct udphdr *udphdr =3D (void *)iphdr + hlen; > > > > > > > > - if (udphdr->dest !=3D __builtin_bswap16(5000)) > > > > + if (udphdr->dest !=3D htons(5000)) > > > > return 0; > > > > > > > > return 1; > > > > diff --git a/test/bpf/t3.c b/test/bpf/t3.c index > > > > 531b9cb8c..24298b7c7 > > > > 100644 > > > > --- a/test/bpf/t3.c > > > > +++ b/test/bpf/t3.c > > > > @@ -17,6 +17,7 @@ > > > > #include > > > > #include > > > > #include "mbuf.h" > > > > +#include > > > > > > > > extern void rte_pktmbuf_dump(FILE *, const struct rte_mbuf *, > > > > unsigned int); > > > > > > > > @@ -29,7 +30,7 @@ entry(const void *pkt) > > > > mb =3D pkt; > > > > eth =3D rte_pktmbuf_mtod(mb, const struct ether_header *); > > > > > > > > - if (eth->ether_type =3D=3D __builtin_bswap16(ETHERTYPE_ARP)) > > > > + if (eth->ether_type =3D=3D htons(ETHERTYPE_ARP)) > > > > rte_pktmbuf_dump(stdout, mb, 64); > > > > > > > > return 1; > > > > -- > > > > 2.17.1