Re: [PATCH v7 06/11] tools headers: Grab copy of io_uring.h

From: Breno Leitao <leitao@debian.org>
To: Gabriel Krisman Bertazi <krisman@suse.de>,
	martin.lau@linux.dev, sdf@google.com
Cc: sdf@google.com, axboe@kernel.dk, asml.silence@gmail.com,
	willemdebruijn.kernel@gmail.com, kuba@kernel.org,
	pabeni@redhat.com, martin.lau@linux.dev, bpf@vger.kernel.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	io-uring@vger.kernel.org,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Stefan Metzmacher <metze@samba.org>,
	Josh Triplett <josh@joshtriplett.org>
Subject: Re: [PATCH v7 06/11] tools headers: Grab copy of io_uring.h
Date: Tue, 17 Oct 2023 05:34:46 -0700	[thread overview]
Message-ID: <ZS5/Zk60X0+ZX+hf@gmail.com> (raw)
In-Reply-To: <652d877c.250a0220.b0af2.3a66SMTPIN_ADDED_BROKEN@mx.google.com>

Hello Gabriel,

On Mon, Oct 16, 2023 at 02:56:55PM -0400, Gabriel Krisman Bertazi wrote:
> Breno Leitao <leitao@debian.org> writes:
> 
> > This file will be used by mini_uring.h and allow tests to run without
> > the need of installing liburing to run the tests.
> >
> > This is needed to run io_uring tests in BPF, such as
> > (tools/testing/selftests/bpf/prog_tests/sockopt.c).
> >
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> Can't mini_uring rely on the kernel header like
> selftests/net/io_uring_zerocopy_tx.c does?

Before this patch, io_uring_zerocopy_tx was not relying on "make
headers" headers, as far as I know. I think it was not a problem because
there was no CI running the test, and whoever was running the test was
relying on local io_uring headers.

My patch is, in fact,  adding the following flag, which relies on the
headers now on:

	+$(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/

> I ask because this will be the third copy of these
> definitions that we're gonna need to keep in sync (kernel, liburing and
> here). Given this is only used for selftests, we better avoid the
> duplication.

Right, I don't know why this was the suggested way, but, that is how
people are using it.

I can definitely get rid of the copy and do the same mechanism as
io_uring_zerocopy_tx. This is what I've tested, and it worked fine.

---

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 4225f975fce3..9f79a392acc1 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -383,6 +383,8 @@ BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN) \
 CLANG_CFLAGS = $(CLANG_SYS_INCLUDES) \
               -Wno-compare-distinct-pointer-types

+HEADER_CFLAGS = -I$(abspath $(OUTPUT)/../../../../usr/include)
+
 $(OUTPUT)/test_l4lb_noinline.o: BPF_CFLAGS += -fno-inline
 $(OUTPUT)/test_xdp_noinline.o: BPF_CFLAGS += -fno-inline

@@ -551,7 +553,7 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o:                   \
                      $(TRUNNER_BPF_SKELS_LINKED)                       \
                      $$(BPFOBJ) | $(TRUNNER_OUTPUT)
        $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
-       $(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
+       $(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) $$(HEADER_CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)

