From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by mail.toke.dk (Postfix) with ESMTPS id EDDBE9B2E05 for ; Fri, 4 Nov 2022 04:25: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=gMWIfG1e Received: by mail-yb1-xb49.google.com with SMTP id b142-20020a253494000000b006ca86d5f40fso3824502yba.19 for ; Thu, 03 Nov 2022 20:25:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=PUk/KzpzScQpgR6M7qGYfZX/G9X9dItCofF/HHylYj4=; b=gMWIfG1emRxrVj3GyaVOH00tmqqnQXYIzwJzyGg+LotRtT7/13UdKQCitTcq1lddFF w7YoyreI/+mDOVzPc/bFF8SQE/T7Z/8qGxTc0Fdq0c5HmeYGP5WVmosxwEcW3VDg9VL/ gupQdFaR26PAjkrXLt0FYzyKaPNfJfTWqIUCDrbSiKJEHFao/OQfEgCYP29Uy6t5KaQN r0Rf+/FM0Lam4KlajBiPQwgeh1/5vK89s8U0CD6H5sYA2vz88FHawMAG7x8fpY6Q3W/i jhsbcRGsaS++U/GsdSLSbCoIdn+MzR15ZsorjSJNxkVZaC41STe9fVSsml7G7t7hU2oO SRUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=PUk/KzpzScQpgR6M7qGYfZX/G9X9dItCofF/HHylYj4=; b=RAQS1/jX8z1LscSyoooAKEmQ2+BMkRVnAgRWK5rbQDLg0it+RCzsx3HrEKLn3P+yow gCS39KO7zUWPlXIh8kgqDSwfWPvS77cgICXsPy3QtS4VYJzeWP2v7N7W41gN1jpea8Ls doe/y/VJEyJQgQ7q5VL4G4VzUgO7mWQJN6a4BOFSq+0mo4shIJX2prunUJAvFFejfHvD RvTJHOPoM63/f8kjeDMb381V/84boYIZY/XNnvy/70/2C9bNUxiqqwv5T5DEIorhexVX gmjiSUaSB7U1V7TBmRlVHptORJfG9oWseY5gzFmEpEYrQoMmw9UNEz3EvaYIocSpNuuS sxRA== X-Gm-Message-State: ACrzQf21885dM6BOWbUpyRcPRk2NdJvEMm2bAHOZCH11DWlgKaYy8ZkP fMs/cg7MQ0/0BFDxdj1vym9sQ/s= X-Google-Smtp-Source: AMsMyM7zc6prS8uR4b3KOrhY0adVhR9NmhhrHCa3CBp7mSaJS3isYh571BeYQWM6TiZfTQQ1/Yt3ojg= X-Received: from sdf.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5935]) (user=sdf job=sendgmr) by 2002:a0d:e647:0:b0:36f:f93f:7f7d with SMTP id p68-20020a0de647000000b0036ff93f7f7dmr33503616ywe.149.1667532334126; Thu, 03 Nov 2022 20:25:34 -0700 (PDT) Date: Thu, 3 Nov 2022 20:25:18 -0700 Mime-Version: 1.0 X-Mailer: git-send-email 2.38.1.431.g37b22c650d-goog Message-ID: <20221104032532.1615099-1-sdf@google.com> From: Stanislav Fomichev To: bpf@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Message-ID-Hash: FIAENIGXD5G6M5I6VFID4RQQZYIXIS2M X-Message-ID-Hash: FIAENIGXD5G6M5I6VFID4RQQZYIXIS2M X-MailFrom: 3LoZkYwMKCe0hSUVddVaT.RdbmSe-WXcihmSe-egdYTRi.cTi@flex--sdf.bounces.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: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org, David Ahern , Jakub Kicinski , Willem de Bruijn , Jesper Dangaard Brouer , Anatoly Burakov , Alexander Lobakin , Magnus Karlsson , Maryam Tahhan , xdp-hints@xdp-project.net, netdev@vger.kernel.org X-Mailman-Version: 3.3.5 Precedence: list Subject: [xdp-hints] [RFC bpf-next v2 00/14] xdp: hints via kfuncs List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Changes since the original RFC: - 'struct bpf_patch' to more easily manage insn arrays While working on longer unrolled functions I've bumped into verifier's insn_buf[16]. Instead of increasing it, I've added a simple abstraction that handles resizing. *insn++ = BPF_XXX_XXX(); *insn++ = BPF_YYY_YYY(); vs bpf_patch_append(patch, BPF_XXX_XXX(), BPF_YYY_YYY(), ); There are also some tricks where we resolve BPF_JMP_IMM(op, dst, imm, S16_MAX) to the real end of the program; mostly to make it easy to generate the following: if (some_condition) { if (some_other_condition) { // insns } } - Drop xdp_buff->priv in favor of container_of (Jakub) The drivers might need to maintain more data, so instead of constraining them to a single ->priv pointer in xdp_buff, use container_of and require the users to define the outer struct. xdp_buff should be the first member. Each driver now has two patches: the first one introduces new struct wrapper; the second one does the actual work. - bpf_xdp_metadata_export_to_skb (Toke) To solve the case where we're constructing skb from a redirected frame. bpf_xdp_metadata_export_to_skb is an unrolled kfunc that prepares 'struct xdp_to_skb_metadata' in the metadata. The layout is randomized and it has a randomized magic number to make sure userspace doesn't depend on it. I'm not sure how strict we should be here? Toke/Jesper seem to be ok with userspace depending on this but as long as they read the layout via BTF, so maybe having a fixed magic is ok/better her? Later, at skb_metadata_set time, we call into skb_metadata_import_from_xdp that tries to parse this fixed format and extract relevant metadata. Note that because we are unrolling bpf_xdp_metadata_export_to_skb, we have to constrain other kfuncs to R2-R5 only; I've added the reasoning to the comments. It might be better idea to do a real kfunc call (but we have to generate this kfunc anyway)? - helper to make it easier to call kernel code from unrolled kfuncs Since we're unrolling, we're running in a somewhat constrained environment. R6-R9 belong to the real callee, so we can't use them to stash our R1-R5. We also can't use stack in any way. Another constraint comes from bpf_xdp_metadata_export_to_skb which might call several metadata kfuncs and wants its R1 to be preserved. Thus, we add xdp_kfunc_call_preserving_r1, which generates the bytecode to preserve r1 somewhere in the user-supplied context. Again, we have to do this because we unroll bpf_xdp_metadata_export_to_skb. - mlx4/bnxt/ice examples (Jesper) ice is the only one where it seems feasible to unroll. The code is untested, but at least it shows that it's somewhat possible to get to the timestamp in our R2-R5-constrained environment. mlx4/bnxt do spinlocks/seqlocks, so I'm just calling into the kernel code. - Unroll default implementation to return false/0/NULL Original RFC left default kfuncs calls when the driver doesn't do the unrolling. Here, instead, we unroll to single 'return 0' instruction. - Drop prog_ifindex libbpf patch, use bpf_program__set_ifindex instead (Andrii) - Keep returning metadata by value instead of using a pointer I've tried using the pointer, it works currently, but it requires extra argument per commit eb1f7f71c126 ("bpf/verifier: allow kfunc to return an allocated mem"). The requirement can probably be lifted for our case, but not sure it's necessary for now. While adding rx_timestamp support for the drivers, it turns out we never really return the raw pointer to the descriptor field. We read the descriptor field, do shifts/multiplies, convert to kernel clock, etc/etc. So returning a value instead of a pointer seems more logical, at least for the rx timestamp case. For the other types of metadata, we might reconsider. - rename bpf_xdp_metadata_have_ to bpf_xdp_metadata__supported Spotted in one of Toke's emails. Seems like it better conveys the intent that it actually tests that the device supports the metadata, not that the particular packet has the metadata. The following is unchanged since the original RFC: This is an RFC for the alternative approach suggested by Martin and Jakub. I've tried to CC most of the people from the original discussion, feel free to add more if you think I've missed somebody. Summary: - add new BPF_F_XDP_HAS_METADATA program flag and abuse attr->prog_ifindex to pass target device ifindex at load time - at load time, find appropriate ndo_unroll_kfunc and call it to unroll/inline kfuncs; kfuncs have the default "safe" implementation if unrolling is not supported by a particular device - rewrite xskxceiver test to use C bpf program and extend it to export rx_timestamp (plus add rx timestamp to veth driver) I've intentionally kept it small and hacky to see whether the approach is workable or not. Pros: - we avoid BTF complexity; the BPF programs themselves are now responsible for agreeing on the metadata layout with the AF_XDP consumer - the metadata is free if not used - the metadata should, in theory, be cheap if used; kfuncs should be unrolled to the same code as if the metadata was pre-populated and passed with a BTF id - it's not all or nothing; users can use small subset of metadata which is more efficient than the BTF id approach where all metadata has to be exposed for every frame (and selectively consumed by the users) Cons: - forwarding has to be handled explicitly; the BPF programs have to agree on the metadata layout (IOW, the forwarding program has to be aware of the final AF_XDP consumer metadata layout) - TX picture is not clear; but it's not clear with BTF ids as well; I think we've agreed that just reusing whatever we have at RX won't fly at TX; seems like TX XDP program might be the answer here? (with a set of another tx kfuncs to "expose" bpf/af_xdp metatata back into the kernel) Cc: John Fastabend Cc: David Ahern Cc: Martin KaFai Lau Cc: Jakub Kicinski Cc: Willem de Bruijn Cc: Jesper Dangaard Brouer Cc: Anatoly Burakov Cc: Alexander Lobakin Cc: Magnus Karlsson Cc: Maryam Tahhan Cc: xdp-hints@xdp-project.net Cc: netdev@vger.kernel.org Stanislav Fomichev (14): bpf: Introduce bpf_patch bpf: Support inlined/unrolled kfuncs for xdp metadata veth: Introduce veth_xdp_buff wrapper for xdp_buff veth: Support rx timestamp metadata for xdp selftests/bpf: Verify xdp_metadata xdp->af_xdp path xdp: Carry over xdp metadata into skb context selftests/bpf: Verify xdp_metadata xdp->skb path bpf: Helper to simplify calling kernel routines from unrolled kfuncs ice: Introduce ice_xdp_buff wrapper for xdp_buff ice: Support rx timestamp metadata for xdp mlx4: Introduce mlx4_xdp_buff wrapper for xdp_buff mxl4: Support rx timestamp metadata for xdp bnxt: Introduce bnxt_xdp_buff wrapper for xdp_buff bnxt: Support rx timestamp metadata for xdp drivers/net/ethernet/broadcom/bnxt/bnxt.c | 73 +++- drivers/net/ethernet/intel/ice/ice.h | 5 + drivers/net/ethernet/intel/ice/ice_main.c | 1 + drivers/net/ethernet/intel/ice/ice_txrx.c | 105 ++++- .../net/ethernet/mellanox/mlx4/en_netdev.c | 1 + drivers/net/ethernet/mellanox/mlx4/en_rx.c | 66 ++- drivers/net/veth.c | 89 ++-- include/linux/bpf.h | 1 + include/linux/bpf_patch.h | 29 ++ include/linux/btf.h | 1 + include/linux/btf_ids.h | 4 + include/linux/mlx4/device.h | 7 + include/linux/netdevice.h | 5 + include/linux/skbuff.h | 4 + include/net/xdp.h | 41 ++ include/uapi/linux/bpf.h | 5 + kernel/bpf/Makefile | 2 +- kernel/bpf/bpf_patch.c | 81 ++++ kernel/bpf/syscall.c | 28 +- kernel/bpf/verifier.c | 85 ++++ net/core/dev.c | 7 + net/core/skbuff.c | 25 ++ net/core/xdp.c | 165 +++++++- tools/include/uapi/linux/bpf.h | 5 + tools/testing/selftests/bpf/Makefile | 2 +- .../selftests/bpf/prog_tests/xdp_metadata.c | 394 ++++++++++++++++++ .../selftests/bpf/progs/xdp_metadata.c | 78 ++++ 27 files changed, 1244 insertions(+), 65 deletions(-) create mode 100644 include/linux/bpf_patch.h create mode 100644 kernel/bpf/bpf_patch.c create mode 100644 tools/testing/selftests/bpf/prog_tests/xdp_metadata.c create mode 100644 tools/testing/selftests/bpf/progs/xdp_metadata.c -- 2.38.1.431.g37b22c650d-goog