From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-x533.google.com (mail-pg1-x533.google.com [IPv6:2607:f8b0:4864:20::533]) by mail.toke.dk (Postfix) with ESMTPS id AAD669F75F6 for ; Wed, 22 Mar 2023 20:23:08 +0100 (CET) Authentication-Results: mail.toke.dk; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=sh0xLsYm Received: by mail-pg1-x533.google.com with SMTP id bn14so3775572pgb.11 for ; Wed, 22 Mar 2023 12:23:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1679512986; 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=A+XxDyG1Zb8n7rgGAcE315iKJNtu4d2B1g56wl3T6Ho=; b=sh0xLsYm7+j03IqhsmQvsORXPF9KFKuYhTvDijN5C0oFn6Utb4v7tjDLquZu/LWJD9 yeDhIY3vqdQiau/vcIJMVkYP/zUmmKi+J6iJYX+j5gR9C4MLqCTnYqlALq8u8Hx1hqcB pkQ9IxErHrpNI6E50R/NdhCp0tj0aBu8Sr+nTQCyHTGsPZ/M1qtZvh1NWWwVyyWys30R nAn2KxZMuVON1RKPPiu9oawz2o1aVCUJs9uPxYcKnCgLiwQ1Z209bAub1jgn15I0v5VI 9ncILNpTqgU6ZQX+ZrbTKjASTBmBe73FxT5F1zOawnC+W6GJn14+dmMVRCSqNiEQgB2b 6ivg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679512986; 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=A+XxDyG1Zb8n7rgGAcE315iKJNtu4d2B1g56wl3T6Ho=; b=WUIfzmH5V1PoViINz51FFzbFhmzHWkJ8ehz+6GHa0PdbUi9ytL2SLUeYSwgBscGqCU mrko3Z6T4ar5bjSUKUfPBQj0k7kxW1xOW3rG+lBMfI3no2O7J5y09BHQAHm4q/fp5mkZ KIX6zRxt2yvKmTNl2QBp/sujGp9K5desfzBrABJQN9DK9vJPaSzFMiJ7ybzScfwUcgub 3xcyjTyAsSDYVYIbo2dKndgONmE5PCKj2cZIpsv6mjrBi6jQyAN5f6w2w2yMIjDznJ2y 7AykXrI1NXexRdwpdqB4hO4aiJk9IiFiGfx6AubH67cDgXkeqSplGdWcQkCespd7R2st xvhw== X-Gm-Message-State: AO0yUKVWrYeMuywHcCBLQqGq8Yl3b9AlAFX1gD8EJj4VUmv3RYxfGpTn jgxd9X9FUnf+42cB2B1hcw/w3qfzZ3KtVaw9xqmvHA== X-Google-Smtp-Source: AK7set/E0fspDnF0EE3Ps3NBksqM1EMUhCA699yeWlh4OzFhsdKBUpjGKFwa/C4OQJOCsmUTQydO4jo+cjRMESk/t8U= X-Received: by 2002:a65:4801:0:b0:4fc:d6df:85a0 with SMTP id h1-20020a654801000000b004fcd6df85a0mr1066021pgs.1.1679512986079; Wed, 22 Mar 2023 12:23:06 -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: Stanislav Fomichev Date: Wed, 22 Mar 2023 12:22:54 -0700 Message-ID: To: Alexei Starovoitov Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 4BGMBBM4RCG3XBVZINBASCNCV6QBMIET X-Message-ID-Hash: 4BGMBBM4RCG3XBVZINBASCNCV6QBMIET X-MailFrom: sdf@google.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: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 log. > > > > >> > > > > >> Print hash value as a hex value, both AF_XDP userspace and bpf_p= rog, > > > > >> 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 as = 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", meta= ->rx_hash); > > > > >> + } else { > > > > >> + bpf_printk("rx_hash not-avail errno:%d", ret); > > > > >> meta->rx_hash =3D 0; /* Used by AF_XDP as not a= vail signal */ > > > > >> + } > > > > >> > > > > >> return bpf_redirect_map(&xsk, ctx->rx_queue_index, XDP_= PASS); > > > > >> } > > > > >> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/too= ls/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. Fu= nctional 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, let'= s > > > > > have a separate patch to enable tracing and periodically dump it = to > > > > > the console instead (as previously discussed). > > > > > > > > Cat'ing /sys/kernel/debug/tracing/trace_pipe work for me regardless= of > > > > setting in > > > > /sys/kernel/debug/tracing/events/bpf_trace/bpf_trace_printk/enable > > > > > > > > We likely need a followup patch that adds a BPF config switch that = can > > > > disable bpf_printk calls, because this adds overhead and thus affec= ts > > > > 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.