Received: by 2002:a05:7412:419a:b0:f3:1519:9f41 with SMTP id i26csp3381756rdh; Mon, 27 Nov 2023 12:45:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IFgDDcr4T8VOiNzks7aRpyLutLIfhroxvGc+ybNu4SbOiVq+utlqTLt3ulpn+lGC6hil5bA X-Received: by 2002:a17:902:ea11:b0:1cf:cbf4:6f7a with SMTP id s17-20020a170902ea1100b001cfcbf46f7amr6078440plg.6.1701117940481; Mon, 27 Nov 2023 12:45:40 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701117940; cv=none; d=google.com; s=arc-20160816; b=FBN3oGpViCmjaz8i0ems45+5O/Zh5pEs8fLyRqg+UzEpMHjTaDlcAlo/YKD3AZMT8l +o+2Jz0ZIPufI3+aRFVDWHE4dlCxCgbVrSP+Cn7yXIFYyLzKoyFN1P5BOY0IfZQZYl5J /8CZdeIS82ouH43YphqnnVQxP44dOd1hE2nhX7kmWOCfslAb5E3+xdxGr5k1cBv7017O JZzUEdNAfR5DSZg6sEUGLjJAxIZaEN9lFL49rbMAFkL3qen6tcA8LYaIj0wSrTynsSUw iqdhc1ZRWKVyVuY8iOpBs3kNgkAhbd61prwIpozJwNTh+4DqRqBoxymNeH5YdF1zTX3b ML/Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:feedback-id:dkim-signature:dkim-signature; bh=c6RIxCPZspuBtK8K2FUFUy9oe3rOA3IlnLJSMn0aJrY=; fh=7FYOkeFAH0oGBRwt5+KHxAjaBmVOD5DOXmHIrEP75hE=; b=ikswGZ6cCBYDLp4QqPMJIGZL5i9xD+ppqVmCuxtzH182A84w3YVRJ232RsbE3ULM9b xhF0BkbebpgfpMeCBrPE39Sw7GJzcQNuXjQuPdjSsoZG32+HO8kaP3PfKBrD35YXDp1g XQO2/pwQI6wFtk2qfPE7R/tYdaCd9TCRV2JKGk/qy1WsV8/MoZuBb0sTZnPO5bZlRc1b GHWMxkq+avvB8I+AYV/pqBZPvbjXFhbEczVTAUORkl62VQHLNWQuLeE5x+fxVb+QwUX+ O6kGYED7UG4o1lXLHIljXkaCh+d7ac47bMcmLPUcGiY47pnIgn5Zb14BlE5b8pgDqha5 o+/w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@dxuuu.xyz header.s=fm3 header.b=pgevG6dm; dkim=pass header.i=@messagingengine.com header.s=fm1 header.b=Rfq6nyQn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id d16-20020a170903231000b001cfd934bda1si1688445plh.232.2023.11.27.12.45.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Nov 2023 12:45:40 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@dxuuu.xyz header.s=fm3 header.b=pgevG6dm; dkim=pass header.i=@messagingengine.com header.s=fm1 header.b=Rfq6nyQn; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id B7E1680A9301; Mon, 27 Nov 2023 12:45:37 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233013AbjK0UpP (ORCPT + 99 others); Mon, 27 Nov 2023 15:45:15 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48570 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231601AbjK0UpN (ORCPT ); Mon, 27 Nov 2023 15:45:13 -0500 Received: from wnew3-smtp.messagingengine.com (wnew3-smtp.messagingengine.com [64.147.123.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7C22A1AA; Mon, 27 Nov 2023 12:45:19 -0800 (PST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailnew.west.internal (Postfix) with ESMTP id 4BBC32B0013D; Mon, 27 Nov 2023 15:45:15 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Mon, 27 Nov 2023 15:45:17 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dxuuu.xyz; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm3; t= 1701117914; x=1701125114; bh=c6RIxCPZspuBtK8K2FUFUy9oe3rOA3IlnLJ SMn0aJrY=; b=pgevG6dm8hI24w8peAUEuqrhrBxxBUDDFCa0Hm1UIUnb/T7rTjM qCpKbObWmxmmRGvLKpbJUQDavUV2Er1g3ipGG7DCnXPezvkHY65i21yrNKxtdzuk DR+PN7+i5JLdCXSmBY5xbAjfCJsdBFzf9wWI6ADakx3kMBu+wilTOKOiWs9TFpAc vBd8zruS+Gtr97UiqdlR9OG6hY+NSi7KYPE0l2P1R7JcUpqDCGmTsPyRcLXcZ/ox 5wmtS+5X4JdCWnCY881cxlgGzWkae7Afzbufdr+Mmn2ha5+/jqf6+89NQ99FeLlC x7/gYhb6UeZGD4zsq2Bdhb5KpuOW87GY+bw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1701117914; x=1701125114; bh=c6RIxCPZspuBtK8K2FUFUy9oe3rOA3IlnLJ SMn0aJrY=; b=Rfq6nyQnKnCi3gDOoi66hH0ZpfPk+E924Vdx/1BipZdSjeaqekH tdWqN63IevDJNwH9oSolNGavAKGHMUxjSTcHTUMACgQJO4au11SbcZHQZ5v3Rvgw ENZ9XeCF6VyNxNSLdT82qFEni/due1wxdKM7Ddj3ksHDPHpN/d5KpxpcJE+xvu3P f7F44b6mBTFFaSuslNTiM7bnBOgbwt2tB3/G3UvDjIOzB/X2+jggM80wPzmuD9AI N1ZVbM2WU53AQ3igGLR1fhVnUGtmtZzq6tXgYZzxQXUdc4f4SPPk4vVE4/vxyHpQ 2erVRMU6Ho3grJA25M9wZRMBMz6B/pXbGnw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvkedrudeiuddgudegvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enfghrlhcuvffnffculdejtddmnecujfgurhepfffhvfevuffkfhggtggugfgjsehtkefs tddttddunecuhfhrohhmpeffrghnihgvlhcuighuuceougiguhesugiguhhuuhdrgiihii eqnecuggftrfgrthhtvghrnhepudefiedtieehffeuffelffegheegjeekteekgfdtkeef jeehffejtdfgkeeiteelnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepugiguhesugiguhhuuhdrgiihii X-ME-Proxy: Feedback-ID: i6a694271:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 27 Nov 2023 15:45:12 -0500 (EST) Date: Mon, 27 Nov 2023 14:45:11 -0600 From: Daniel Xu To: Yonghong Song Cc: Eduard Zingerman , Alexei Starovoitov , Shuah Khan , Daniel Borkmann , Andrii Nakryiko , Alexei Starovoitov , Steffen Klassert , antony.antony@secunet.com, Mykola Lysenko , Martin KaFai Lau , Song Liu , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , bpf , "open list:KERNEL SELFTEST FRAMEWORK" , LKML , devel@linux-ipsec.org, Network Development Subject: Re: [PATCH ipsec-next v1 6/7] bpf: selftests: test_tunnel: Disable CO-RE relocations Message-ID: References: <391d524c496acc97a8801d8bea80976f58485810.1700676682.git.dxu@dxuuu.xyz> <0f210cef-c6e9-41c1-9ba8-225f046435e5@linux.dev> <3ec6c068-7f95-419a-a0ae-a901f95e4838@linux.dev> <18e43cdf65e7ba0d8f6912364fbc5b08a6928b35.camel@gmail.com> <0535eb913f1a0c2d3c291478fde07e0aa2b333f1.camel@gmail.com> <42f9bf0d-695a-412d-bea5-cb7036fa7418@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Mon, 27 Nov 2023 12:45:37 -0800 (PST) On Sun, Nov 26, 2023 at 09:53:04PM -0800, Yonghong Song wrote: > > On 11/27/23 12:44 AM, Yonghong Song wrote: > > > > On 11/26/23 8:52 PM, Eduard Zingerman wrote: > > > On Sun, 2023-11-26 at 18:04 -0600, Daniel Xu wrote: > > > [...] > > > > > Tbh I'm not sure. This test passes with preserve_static_offset > > > > > because it suppresses preserve_access_index. In general clang > > > > > translates bitfield access to a set of IR statements like: > > > > > > > > > > ?? C: > > > > > ???? struct foo { > > > > > ?????? unsigned _; > > > > > ?????? unsigned a:1; > > > > > ?????? ... > > > > > ???? }; > > > > > ???? ... foo->a ... > > > > > > > > > > ?? IR: > > > > > ???? %a = getelementptr inbounds %struct.foo, ptr %0, i32 0, i32 1 > > > > > ???? %bf.load = load i8, ptr %a, align 4 > > > > > ???? %bf.clear = and i8 %bf.load, 1 > > > > > ???? %bf.cast = zext i8 %bf.clear to i32 > > > > > > > > > > With preserve_static_offset the getelementptr+load are replaced by a > > > > > single statement which is preserved as-is till code generation, > > > > > thus load with align 4 is preserved. > > > > > > > > > > On the other hand, I'm not sure that clang guarantees that load or > > > > > stores used for bitfield access would be always aligned according to > > > > > verifier expectations. > > > > > > > > > > I think we should check if there are some clang knobs that prevent > > > > > generation of unaligned memory access. I'll take a look. > > > > Is there a reason to prefer fixing in compiler? I'm not opposed to it, > > > > but the downside to compiler fix is it takes years to propagate and > > > > sprinkles ifdefs into the code. > > > > > > > > Would it be possible to have an analogue of BPF_CORE_READ_BITFIELD()? > > > Well, the contraption below passes verification, tunnel selftest > > > appears to work. I might have messed up some shifts in the macro, > > > though. > > > > I didn't test it. But from high level it should work. > > > > > > > > Still, if clang would peek unlucky BYTE_{OFFSET,SIZE} for a particular > > > field access might be unaligned. > > > > clang should pick a sensible BYTE_SIZE/BYTE_OFFSET to meet > > alignment requirement. This is also required for BPF_CORE_READ_BITFIELD. > > > > > > > > --- > > > > > > diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c > > > b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c > > > index 3065a716544d..41cd913ac7ff 100644 > > > --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c > > > +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c > > > @@ -9,6 +9,7 @@ > > > ? #include "vmlinux.h" > > > ? #include > > > ? #include > > > +#include > > > ? #include "bpf_kfuncs.h" > > > ? #include "bpf_tracing_net.h" > > > ? @@ -144,6 +145,38 @@ int ip6gretap_get_tunnel(struct __sk_buff *skb) > > > ????? return TC_ACT_OK; > > > ? } > > > ? +#define BPF_CORE_WRITE_BITFIELD(s, field, new_val) ({??????????? \ > > > +??? void *p = (void *)s + __CORE_RELO(s, field, BYTE_OFFSET);??? \ > > > +??? unsigned byte_size = __CORE_RELO(s, field, BYTE_SIZE);??????? \ > > > +??? unsigned lshift = __CORE_RELO(s, field, LSHIFT_U64); \ > > > +??? unsigned rshift = __CORE_RELO(s, field, RSHIFT_U64); \ > > > +??? unsigned bit_size = (rshift - lshift);??????????????? \ > > > +??? unsigned long long nval, val, hi, lo;??????????????? \ > > > +??????????????????????????????????? \ > > > +??? asm volatile("" : "=r"(p) : "0"(p));??????????????? \ > > > > Use asm volatile("" : "+r"(p)) ? > > > > > +??????????????????????????????????? \ > > > +??? switch (byte_size) {??????????????????????? \ > > > +??? case 1: val = *(unsigned char *)p; break;??????????? \ > > > +??? case 2: val = *(unsigned short *)p; break;??????????? \ > > > +??? case 4: val = *(unsigned int *)p; break;??????????? \ > > > +??? case 8: val = *(unsigned long long *)p; break;??????????? \ > > > +??? }??????????????????????????????? \ > > > +??? hi = val >> (bit_size + rshift);??????????????? \ > > > +??? hi <<= bit_size + rshift;??????????????????? \ > > > +??? lo = val << (bit_size + lshift);??????????????? \ > > > +??? lo >>= bit_size + lshift;??????????????????? \ > > > +??? nval = new_val;??????????????????????????? \ > > > +??? nval <<= lshift;??????????????????????? \ > > > +??? nval >>= rshift;??????????????????????? \ > > > +??? val = hi | nval | lo;??????????????????????? \ > > > +??? switch (byte_size) {??????????????????????? \ > > > +??? case 1: *(unsigned char *)p????? = val; break;??????????? \ > > > +??? case 2: *(unsigned short *)p???? = val; break;??????????? \ > > > +??? case 4: *(unsigned int *)p?????? = val; break;??????????? \ > > > +??? case 8: *(unsigned long long *)p = val; break;??????????? \ > > > +??? }??????????????????????????????? \ > > > +}) > > > > I think this should be put in libbpf public header files but not sure > > where to put it. bpf_core_read.h although it is core write? > > > > But on the other hand, this is a uapi struct bitfield write, > > strictly speaking, CORE write is really unnecessary here. It > > would be great if we can relieve users from dealing with > > such unnecessary CORE writes. In that sense, for this particular > > case, I would prefer rewriting the code by using byte-level > > stores... > or preserve_static_offset to clearly mean to undo bitfield CORE ... Ok, I will do byte-level rewrite for next revision. Just wondering, though: will bpftool be able to generate the appropriate annotations for uapi structs? IIUC uapi structs look the same in BTF as any other struct. > > [...] > Thanks, Daniel