From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by mail.toke.dk (Postfix) with ESMTPS id 50A209F760F for ; Wed, 22 Mar 2023 20:30:49 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=F5qfAd61 Received: by mail-ed1-x52d.google.com with SMTP id cy23so77286090edb.12 for ; Wed, 22 Mar 2023 12:30:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679513449; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=TrQlaEec5B0ZGmryHZC8OLS4jt5SwKguRICHgHnf1bI=; b=F5qfAd61GgyM+ravQ2cpL0YIlB6d/Jxc/GQJXNn8B+WLlAmss/Ykt4wOHyfdVRt7p9 747Evf56A/huI57j4TISAid9yUv2x0VQL3o7mqEq0WtXW+R2OQOnGpC3QkRqkRh1LuDT cp/7gCKbpROBt8r4KIZQkp6glIy0UHdQdx/o/EwyVddaMUgQNZ0jkgrVI35EzWEfkp4u He6kL5dNrA7fGQob9/Vffh3JxBuEl5nALyJkL1J4M81uv+eJLujXYUXB3MUgTpqPFUVH IyIQhkmBCuah6C5W8r/9r/HNBZGimx4iTfg7TxOkeW6ouKgrlOMyyYi3pQUk4Px53Lxg kqrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679513449; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TrQlaEec5B0ZGmryHZC8OLS4jt5SwKguRICHgHnf1bI=; b=JpYcvDy7XV5cFqOuDGkq7wVZ7ioMcFmgaFrhlqKRAM2BwyaAfa6D3b7HuRHohSqCxu 4tXhgYUzPrOGU9wyRwG/LUCSU8mbxum5comv00PhlgsiqgVBOWA0yUtC1jUgdxL2jWmm Nd0ogjsSTq/xXdLl9VusWJgyIa7MP52cIuyr44R4UaGnsHDOkhm7agDIVV6Tlbtkxz9C w0d0r1yh8ncdnceKKVXs+mVETjoqjUp1ZpjxtGGpws+N59W4tWYBNydjdCK8YHu15CJQ qdBhQ93+FmM5Lma50wmP+1GN4HZYfjF8mCeTabC+r697UWxcR6XuyT09eI+4lsAOJnnF 27uA== X-Gm-Message-State: AO0yUKXkhKNjukVmczll7o4j5C1C70BdOfzZUiBp3kff+xyUSLGsZ5Qg xDx41gW5jZyZ14cJDEyImvyTrlD8yvIBvu7JqL8= X-Google-Smtp-Source: AK7set9q1qs+/E5dlv0SRf2iQTk9JRKsI0pGAVe6/06WLrNMVKgHleemIyGTp6evMQg4J1kF4xkhTZcQvsY1onABpdU= X-Received: by 2002:a17:906:a288:b0:931:3a19:d835 with SMTP id i8-20020a170906a28800b009313a19d835mr3755928ejz.3.1679513448996; Wed, 22 Mar 2023 12:30:48 -0700 (PDT) MIME-Version: 1.0 References: <167940634187.2718137.10209374282891218398.stgit@firesoul> <167940643669.2718137.4624187727245854475.stgit@firesoul> <080640fc-5835-26f1-2b20-ff079bd59182@redhat.com> In-Reply-To: From: Alexei Starovoitov Date: Wed, 22 Mar 2023 12:30:37 -0700 Message-ID: To: Stanislav Fomichev Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: QDQ4DUTEXFX3JJZPM4FJ5VL3ZOFMN7NX X-Message-ID-Hash: QDQ4DUTEXFX3JJZPM4FJ5VL3ZOFMN7NX X-MailFrom: alexei.starovoitov@gmail.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Jesper Dangaard Brouer , Jesper Dangaard Brouer , bpf , Network Development , Martin KaFai Lau , Alexei Starovoitov , Daniel Borkmann , Alexander Lobakin , Larysa Zaremba , xdp-hints@xdp-project.net, anthony.l.nguyen@intel.com, "Song, Yoong Siang" , "Ong, Boon Leong" , intel-wired-lan , Paolo Abeni , Jesse Brandeburg , Jakub Kicinski , Eric Dumazet , John Fastabend , Jesper Dangaard Brouer , "David S. Miller" X-Mailman-Version: 3.3.8 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next V2 3/6] selftests/bpf: xdp_hw_metadata RX hash return code info List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, Mar 22, 2023 at 12:23=E2=80=AFPM Stanislav Fomichev wrote: > > On Wed, Mar 22, 2023 at 12:17=E2=80=AFPM Alexei Starovoitov > wrote: > > > > On Wed, Mar 22, 2023 at 12:00=E2=80=AFPM Stanislav Fomichev wrote: > > > > > > On Wed, Mar 22, 2023 at 9:07=E2=80=AFAM Alexei Starovoitov > > > wrote: > > > > > > > > On Wed, Mar 22, 2023 at 9:05=E2=80=AFAM Jesper Dangaard Brouer > > > > wrote: > > > > > > > > > > > > > > > > > > > > On 21/03/2023 19.47, Stanislav Fomichev wrote: > > > > > > On Tue, Mar 21, 2023 at 6:47=E2=80=AFAM Jesper Dangaard Brouer > > > > > > wrote: > > > > > >> > > > > > >> When driver developers add XDP-hints kfuncs for RX hash it is > > > > > >> practical to print the return code in bpf_printk trace pipe lo= g. > > > > > >> > > > > > >> Print hash value as a hex value, both AF_XDP userspace and bpf= _prog, > > > > > >> as this makes it easier to spot poor quality hashes. > > > > > >> > > > > > >> Signed-off-by: Jesper Dangaard Brouer > > > > > >> --- > > > > > >> .../testing/selftests/bpf/progs/xdp_hw_metadata.c | 9 ++= ++++--- > > > > > >> tools/testing/selftests/bpf/xdp_hw_metadata.c | 5 ++= ++- > > > > > >> 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > >> > > > > > >> diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata= .c b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > > > > > >> index 40c17adbf483..ce07010e4d48 100644 > > > > > >> --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > > > > > >> +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c > > > > > >> @@ -77,10 +77,13 @@ int rx(struct xdp_md *ctx) > > > > > >> meta->rx_timestamp =3D 0; /* Used by AF_XDP a= s not avail signal */ > > > > > >> } > > > > > >> > > > > > >> - if (!bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash)) > > > > > >> - bpf_printk("populated rx_hash with %u", meta->= rx_hash); > > > > > >> - else > > > > > >> + ret =3D bpf_xdp_metadata_rx_hash(ctx, &meta->rx_hash); > > > > > >> + if (ret >=3D 0) { > > > > > >> + bpf_printk("populated rx_hash with 0x%08X", me= ta->rx_hash); > > > > > >> + } else { > > > > > >> + bpf_printk("rx_hash not-avail errno:%d", ret); > > > > > >> meta->rx_hash =3D 0; /* Used by AF_XDP as not= avail signal */ > > > > > >> + } > > > > > >> > > > > > >> return bpf_redirect_map(&xsk, ctx->rx_queue_index, XD= P_PASS); > > > > > >> } > > > > > >> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/t= ools/testing/selftests/bpf/xdp_hw_metadata.c > > > > > >> index 400bfe19abfe..f3ec07ccdc95 100644 > > > > > >> --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c > > > > > >> +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c > > > > > >> @@ -3,6 +3,9 @@ > > > > > >> /* Reference program for verifying XDP metadata on real HW. = Functional test > > > > > >> * only, doesn't test the performance. > > > > > >> * > > > > > >> + * BPF-prog bpf_printk info outout can be access via > > > > > >> + * /sys/kernel/debug/tracing/trace_pipe > > > > > > > > > > > > s/outout/output/ > > > > > > > > > > > > > > > > Fixed in V3 > > > > > > > > > > > But let's maybe drop it? If you want to make it more usable, le= t's > > > > > > have a separate patch to enable tracing and periodically dump i= t to > > > > > > the console instead (as previously discussed). > > > > > > > > > > Cat'ing /sys/kernel/debug/tracing/trace_pipe work for me regardle= ss of > > > > > setting in > > > > > /sys/kernel/debug/tracing/events/bpf_trace/bpf_trace_printk/enabl= e > > > > > > > > > > We likely need a followup patch that adds a BPF config switch tha= t can > > > > > disable bpf_printk calls, because this adds overhead and thus aff= ects > > > > > the timestamps. > > > > > > > > No. This is by design. > > > > Do not use bpf_printk* in production. > > > > > > But that's not for the production? xdp_hw_metadata is a small tool to > > > verify that the metadata being dumped is correct (during the > > > development). > > > We have a proper (less verbose) selftest in > > > {progs,prog_tests}/xdp_metadata.c (over veth). > > > This xdp_hw_metadata was supposed to be used for running it against > > > the real hardware, so having as much debugging at hand as possible > > > seems helpful? (at least it was helpful to me when playing with mlx4) > > > > The only use of bpf_printk is for debugging of bpf progs themselves. > > It should not be used in any tool. > > Hmm, good point. I guess it also means we won't have to mess with > enabling/dumping ftrace (and don't need this comment about cat'ing the > file). > Jesper, maybe we can instead pass the status of those > bpf_xdp_metadata_xxx kfuncs via 'struct xdp_meta'? And dump this info > from the userspace if needed. There are so many other ways for bpf prog to communicate with user space. Use ringbuf, perf_event buffer, global vars, maps, etc. trace_pipe is debug only because it's global and will conflict with all other debug sessions.