From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-x1031.google.com (mail-pj1-x1031.google.com [IPv6:2607:f8b0:4864:20::1031]) by mail.toke.dk (Postfix) with ESMTPS id ECC3C9F7623 for ; Wed, 22 Mar 2023 20:33:36 +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=NEt3kWU3 Received: by mail-pj1-x1031.google.com with SMTP id o6-20020a17090a9f8600b0023f32869993so20668424pjp.1 for ; Wed, 22 Mar 2023 12:33:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; t=1679513614; 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=d+yqqsftMo/G3nMdP87H9eSSXuM3ivJL8CNlu7fY/OI=; b=NEt3kWU3db2Pks8j9Xq0f/3Gg2nN6/zbzH1eHceZJasP2v37IlFJ1KevfQqfvU0vrh ic3qhbvC4j5ilbP31NUyhYoXwE0K2giaeEz54BIEdo68mGRSm5IkUrh0Ch9qvokAUER4 4dW2MQVaLSzjK4ED++GjEJjfAiyzEjeFQJ9El8dLJ6/PZRH3fmNwimWwcTgtrmQ/tcZB F114eR4A40rp8znRHpAQOfrsHDSRegZvtoW25HPZ82D1T1/EzevfXj0B5eFmGchuGyLW eSdElYMeAVgkjpYb0RIAF/5ssBeok6KndQoyoEm+LbeFK0VovW9czaOo7j3ZBuEgeoOR Iopg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679513614; 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=d+yqqsftMo/G3nMdP87H9eSSXuM3ivJL8CNlu7fY/OI=; b=yiUzSdjw0ODCyKGv6vxhmzwdd1UFlZ7UjyaAH5wEwUKWCIA8SGkkEtWsU1QYMlrj6b J7jZ6CjKC7JW3yNwaqRe+QTrIKU84nvwB4di739uRIYczaCSaqs2i4QXf8TrrXEl9XHF 5Wm5nTacxZiZZgL/mweaBrhEy6MU8fl02PkzJXvjOsNzenaioqiOijRi5x2GkHw8d5bv +QkZH2W5mOe9/IW6JCtjvieJEF1bDLR5GM2XjEZ29JKy5w1mxTa4FHurmKUh+83qJrl1 +ixGIKJAfurKJtNMJpHSQpZCaBOn0wzt2yPHu4pLf4GapgkkW8nRbNuFo5P5TFT5ihZs BDCw== X-Gm-Message-State: AO0yUKWfU8Ab9aiitGMwOSGhe6aGb+TyMDat9IYmCKJyg83TrqUqq74I vlAgwDU05m18XXTe6w0GIyz5QitgGJz/g51g+BJJEg== X-Google-Smtp-Source: AK7set/ypIrUA5qMrYjv6h+zUCLqRU+8WvxjgbyIxdvmItI2pMSceBZC/hQmlHEP6AeqH1Mo+V8r0ywLc7A8XdUOoYM= X-Received: by 2002:a17:902:708b:b0:1a0:6001:2e0e with SMTP id z11-20020a170902708b00b001a060012e0emr1445303plk.8.1679513614220; Wed, 22 Mar 2023 12:33:34 -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:33:22 -0700 Message-ID: To: Alexei Starovoitov Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: SBQSWUIG5WER4V7MO5HMDMPSB4IJC7A3 X-Message-ID-Hash: SBQSWUIG5WER4V7MO5HMDMPSB4IJC7A3 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:30=E2=80=AFPM Alexei Starovoitov wrote: > > 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 Broue= r > > > > > > > wrote: > > > > > > >> > > > > > > >> When driver developers add XDP-hints kfuncs for RX hash it i= s > > > > > > >> practical to print the return code in bpf_printk trace pipe = log. > > > > > > >> > > > > > > >> Print hash value as a hex value, both AF_XDP userspace and b= pf_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_metada= ta.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 n= ot avail signal */ > > > > > > >> + } > > > > > > >> > > > > > > >> return bpf_redirect_map(&xsk, ctx->rx_queue_index, = XDP_PASS); > > > > > > >> } > > > > > > >> diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b= /tools/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, = 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 regard= less of > > > > > > setting in > > > > > > /sys/kernel/debug/tracing/events/bpf_trace/bpf_trace_printk/ena= ble > > > > > > > > > > > > We likely need a followup patch that adds a BPF config switch t= hat can > > > > > > disable bpf_printk calls, because this adds overhead and thus a= ffects > > > > > > 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 mlx= 4) > > > > > > 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. =F0=9F=91=8D makes sense, ty! hopefully we won't have to add a separate cha= nnel for those and can (ab)use the metadata area.