bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sedat Dilek <sedat.dilek@gmail.com>
To: Yonghong Song <yhs@fb.com>
Cc: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
	dwarves@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Bill Wendling <morbo@google.com>,
	bpf@vger.kernel.org, David Blaikie <dblaikie@gmail.com>,
	kernel-team@fb.com, Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly
Date: Sun, 4 Apr 2021 14:46:01 +0200	[thread overview]
Message-ID: <CA+icZUWh6YOkCKG72SndqUbQNwG+iottO4=cPyRRVjaHD2=0qw@mail.gmail.com> (raw)
In-Reply-To: <CA+icZUV4fw5GNXFnyOjvajkVFdPhkOrhr3rn5OrAKGujpSrmgQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 7456 bytes --]

> This shows a new build-error:
>
> clang  -g -D__TARGET_ARCH_x86 -mlittle-endian
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/include
> -I/home/dileks/src/linux-kernel/git/tools/t
> esting/selftests/bpf
> -I/home/dileks/src/linux-kernel/git/tools/include/uapi
> -I/home/dileks/src/linux-kernel/git/tools/testing/selftests/usr/include
> -idirafter /usr/loc
> al/include -idirafter /opt/llvm-toolchain/lib/clang/12.0.0/include
> -idirafter /usr/include/x86_64-linux-gnu -idirafter /usr/include
> -Wno-compare-distinct-pointer-type
> s -DENABLE_ATOMICS_TESTS -O2 -target bpf -c
> progs/test_sk_storage_tracing.c -o
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o-mcpu=v3
> progs/test_sk_storage_tracing.c:38:18: error: use of undeclared
> identifier 'BPF_TCP_CLOSE'
>        if (newstate == BPF_TCP_CLOSE)
>                        ^
> 1 error generated.
> make: *** [Makefile:414:
> /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_sk_storage_tracing.o]
> Error 1
>

I was able to fix this by adding appropriate enums from <linux/bpf.h>.

$ git diff
diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
index 8e94e5c080aa..3c7508f48ce0 100644
--- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
@@ -6,6 +6,28 @@
#include <bpf/bpf_core_read.h>
#include <bpf/bpf_helpers.h>

+/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
+ * changes between the TCP and BPF versions. Ideally this should never happen.
+ * If it does, we need to add code to convert them before calling
+ * the BPF sock_ops function.
+ */
+enum {
+       BPF_TCP_ESTABLISHED = 1,
+       BPF_TCP_SYN_SENT,
+       BPF_TCP_SYN_RECV,
+       BPF_TCP_FIN_WAIT1,
+       BPF_TCP_FIN_WAIT2,
+       BPF_TCP_TIME_WAIT,
+       BPF_TCP_CLOSE,
+       BPF_TCP_CLOSE_WAIT,
+       BPF_TCP_LAST_ACK,
+       BPF_TCP_LISTEN,
+       BPF_TCP_CLOSING,        /* Now a valid state */
+       BPF_TCP_NEW_SYN_RECV,
+
+       BPF_TCP_MAX_STATES      /* Leave at the end! */
+};
+
struct sk_stg {
       __u32 pid;
       __u32 last_notclose_state;

NOTE: Attached as a diff as Gmail might truncate it.

[ Q ] Should these enums be in vmlinux.h - if so why are they missing?

Next build-error:

g++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/b
pf/tools/include -I/home/dileks/src/linux-kernel/git/include/generated
-I/home/dileks/src/linux-kernel/git/tools/lib
-I/home/dileks/src/linux-kernel/git/tools/include
-I/home/dileks/src/linux-kernel/git/tools/include/uapi
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-Dbpf_prog_load=bpf_prog_test_load
-Dbpf_load_program=bpf_test_load_program test_cpp.cpp
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
-lcap -lelf -lz -lrt -lpthread -o
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
/usr/bin/ld: /home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a(libbpf-in.o):
relocation R_X86_64_32 against `.rodata.str1.1' ca
n not be used when making a PIE object; recompile with -fPIE
collect2: error: ld returned 1 exit status
make: *** [Makefile:455:
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
Error 1
make: Leaving directory
'/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf'

LOL, I was not aware that there is usage of *** CXX*** in tools
directory (see g++ line and /usr/bin/ld ?).

So, I changed my $MAKE_OPTS to use "CXX=clang++".

$ echo $PATH
/opt/llvm-toolchain/bin:/opt/proxychains-ng/bin:/home/dileks/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games

$ echo $MAKE $MAKE_OPTS
make V=1 HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
CXX=clang++ LD=ld.lld LLVM=1 LLVM_IAS=1 PAHOLE=/opt/pahole/bin/pahole

$ clang --version
dileks clang version 12.0.0 (https://github.com/llvm/llvm-project.git
04ba60cfe598e41084fb848daae47e0ed910fa7d)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/llvm-toolchain/bin
$ ld.lld --version
LLD 12.0.0 (https://github.com/llvm/llvm-project.git
04ba60cfe598e41084fb848daae47e0ed910fa7d) (compatible with GNU
linkers)

$ LC_ALL=C $MAKE $MAKE_OPTS -C tools/testing/selftests/bpf/

This breaks like this:

clang++ -g -rdynamic -Wall -O2 -DHAVE_GENHDR
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-I/home/dileks/src/linux-kernel/git/tools/testing/selftes
ts/bpf/tools/include
-I/home/dileks/src/linux-kernel/git/include/generated
-I/home/dileks/src/linux-kernel/git/tools/lib
-I/home/dileks/src/linux-kernel/git/tools/incl
ude -I/home/dileks/src/linux-kernel/git/tools/include/uapi
-I/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf
-Dbpf_prog_load=bpf_prog_test_load -Dbpf_loa
d_program=bpf_test_load_program test_cpp.cpp
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_core_extern.skel.h
/home/dileks/src/linux-kernel/git/to
ols/testing/selftests/bpf/tools/build/libbpf/libbpf.a
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_stub.o
-lcap -lelf -lz -lrt -lpthread -o /home
/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp
clang-12: warning: treating 'c-header' input as 'c++-header' when in
C++ mode, this behavior is deprecated [-Wdeprecated]
clang-12: error: cannot specify -o when generating multiple output files
make: *** [Makefile:455:
/home/dileks/src/linux-kernel/git/tools/testing/selftests/bpf/test_cpp]
Error 1

OK, I see in bpf-next includes several fixes like:

commit a0964f526df6facd4e12a4c416185013026eecf9
"selftests/bpf: Add multi-file statically linked BPF object file test"

...and to "selftests: xsk".

Finally, I was able to build by suppressing the build of "test_cpp"
and "xdpxceiver":

$ git diff tools/testing/selftests/bpf/Makefile
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d9b19524b2d4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
# Compile but not part of 'make run_tests'
TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
       flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
-       test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
-       xdpxceiver
+       test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko
+       # test_cpp xdpxceiver

TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read

This diff is also attached before Gmail eats it.

Yonghong Song as you described your build-environment and checking
requirements for clang-13 in bpf-next (see [1]), I am unsure if I want
to upgrade LLVM toolchain to v13-git and use bpf-next as the new
kernel base.
Lemme see if I get LLVM/Clang v13-git from Debian/experimental and/or
<apt.llvm.org>.

- Sedat -

[1] https://git.kernel.org/bpf/bpf-next/c/2ba4badca9977b64c966b0177920daadbd5501fe
[2] https://git.kernel.org/bpf/bpf-next/c/a0964f526df6facd4e12a4c416185013026eecf9

[-- Attachment #2: selftests-bpf-Makefile.diff --]
[-- Type: text/x-patch, Size: 677 bytes --]

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d9b19524b2d4 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -77,8 +77,8 @@ TEST_PROGS_EXTENDED := with_addr.sh \
 # Compile but not part of 'make run_tests'
 TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
 	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
-	test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
-	xdpxceiver
+	test_lirc_mode2_user xdping runqslower bench bpf_testmod.ko
+	# test_cpp xdpxceiver
 
 TEST_CUSTOM_PROGS = $(OUTPUT)/urandom_read
 

[-- Attachment #3: selftests-bpf-progs-test_sk_storage_tracing_c.diff --]
[-- Type: text/x-patch, Size: 1040 bytes --]

diff --git a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
index 8e94e5c080aa..3c7508f48ce0 100644
--- a/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
+++ b/tools/testing/selftests/bpf/progs/test_sk_storage_tracing.c
@@ -6,6 +6,28 @@
 #include <bpf/bpf_core_read.h>
 #include <bpf/bpf_helpers.h>
 
+/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
+ * changes between the TCP and BPF versions. Ideally this should never happen.
+ * If it does, we need to add code to convert them before calling
+ * the BPF sock_ops function.
+ */
+enum {
+	BPF_TCP_ESTABLISHED = 1,
+	BPF_TCP_SYN_SENT,
+	BPF_TCP_SYN_RECV,
+	BPF_TCP_FIN_WAIT1,
+	BPF_TCP_FIN_WAIT2,
+	BPF_TCP_TIME_WAIT,
+	BPF_TCP_CLOSE,
+	BPF_TCP_CLOSE_WAIT,
+	BPF_TCP_LAST_ACK,
+	BPF_TCP_LISTEN,
+	BPF_TCP_CLOSING,	/* Now a valid state */
+	BPF_TCP_NEW_SYN_RECV,
+
+	BPF_TCP_MAX_STATES	/* Leave at the end! */
+};
+
 struct sk_stg {
 	__u32 pid;
 	__u32 last_notclose_state;

[-- Attachment #4: make-log_tools-testing-selftests-bpf_5.12.0-rc5-12-amd64-clang12-lto_test_sk_storage_tracing_c-CXX-test_cpp.txt.gz --]
[-- Type: application/gzip, Size: 39210 bytes --]

  parent reply	other threads:[~2021-04-04 12:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-03 18:41 [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
2021-04-03 18:52 ` David Blaikie
2021-04-03 20:20   ` Yonghong Song
2021-04-03 23:31     ` David Blaikie
     [not found] ` <CA+icZUWLf4W_1u_p4-Rx1OD7h_ydP4Xzv12tMA2HZqj9CCOH0Q@mail.gmail.com>
2021-04-03 23:13   ` Yonghong Song
2021-04-03 23:27   ` Yonghong Song
     [not found]     ` <CA+icZUV4fw5GNXFnyOjvajkVFdPhkOrhr3rn5OrAKGujpSrmgQ@mail.gmail.com>
2021-04-04 12:46       ` Sedat Dilek [this message]
2021-04-04 16:39         ` Yonghong Song
2021-04-04 17:25           ` Sedat Dilek
2021-04-05  2:24             ` Yonghong Song
     [not found]               ` <CA+icZUVcQ+vQjc0VavetA3s6jzNhC20dU4Sa9ApBLNXbY=w5wA@mail.gmail.com>
2021-04-05 11:04                 ` Sedat Dilek
2021-04-05 11:46                   ` Sedat Dilek
2021-04-05 16:17                 ` Yonghong Song
2021-04-05 18:32                   ` Sedat Dilek
2021-04-05 18:56                     ` Yonghong Song
2021-04-05 20:42                       ` Sedat Dilek
     [not found]             ` <CA+icZUVp3UTPUS-ZjCOnHbNXxaA7DN=4x_08jc8BExFe4Nf2ZQ@mail.gmail.com>
2021-04-05  2:30               ` Yonghong Song
     [not found]                 ` <CA+icZUVtzXNxuVtEUwfULa7nivV0VFfJznsRnSZtEh+V=C=RPg@mail.gmail.com>
2021-04-05  6:56                   ` Sedat Dilek
2021-04-04 14:59 ` Usage of CXX in tools directory Sedat Dilek
2021-04-04 15:19   ` Sedat Dilek
2021-04-06 18:39   ` Nick Desaulniers
2021-04-04 16:45 ` [PATCH dwarves] dwarf_loader: handle DWARF5 DW_OP_addrx properly Yonghong Song
2021-04-04 17:29   ` Sedat Dilek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+icZUWh6YOkCKG72SndqUbQNwG+iottO4=cPyRRVjaHD2=0qw@mail.gmail.com' \
    --to=sedat.dilek@gmail.com \
    --cc=arnaldo.melo@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dblaikie@gmail.com \
    --cc=dwarves@vger.kernel.org \
    --cc=kernel-team@fb.com \
    --cc=morbo@google.com \
    --cc=ndesaulniers@google.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).