From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mail.toke.dk; spf=pass (mailfrom) smtp.mailfrom=linux.dev (client-ip=188.165.223.204; helo=out2.migadu.com; envelope-from=martin.lau@linux.dev; receiver=) Authentication-Results: mail.toke.dk; dkim=pass (1024-bit key; unprotected) header.d=linux.dev header.i=@linux.dev header.a=rsa-sha256 header.s=key1 header.b=LnsGOz/a Received: from out2.migadu.com (out2.migadu.com [188.165.223.204]) by mail.toke.dk (Postfix) with ESMTPS id B9B6B9CF916 for ; Wed, 14 Dec 2022 02:54:04 +0100 (CET) Message-ID: <74e48fc9-8f5d-4183-9f39-c4587c74a74e@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1670982843; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=GNg4/FrMFF9sXTLeG093TYFyexXEX026wSRyByZmxZg=; b=LnsGOz/aHRiS0h47bpq4i8GzveVplZ2215QzabqvW9i7hEucZQK4C0LdzxA9PBTrGjl2Jv bTZYQZO1UgC/xu9W485A2mtau4vTxm3OMYFv2lNdnKRAjYMRDIssJUF8F4KSe5Gz+cjS1+ 7Hy/40pP7FYC03nDfxoiG7qvtWz8vII= Date: Tue, 13 Dec 2022 17:53:58 -0800 MIME-Version: 1.0 Content-Language: en-US To: Stanislav Fomichev References: <20221213023605.737383-1-sdf@google.com> <20221213023605.737383-6-sdf@google.com> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Martin KaFai Lau In-Reply-To: <20221213023605.737383-6-sdf@google.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT Message-ID-Hash: PG65FDYWE6VT55JEA3UARLMKIBJG5ZLJ X-Message-ID-Hash: PG65FDYWE6VT55JEA3UARLMKIBJG5ZLJ X-MailFrom: martin.lau@linux.dev 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, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org, 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, bpf@vger.kernel.org X-Mailman-Version: 3.3.7 Precedence: list Subject: [xdp-hints] Re: [PATCH bpf-next v4 05/15] bpf: XDP metadata RX kfuncs List-Id: XDP hardware hints design discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 12/12/22 6:35 PM, Stanislav Fomichev wrote: > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index ca22e8b8bd82..de6279725f41 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -2477,6 +2477,8 @@ void bpf_offload_dev_netdev_unregister(struct bpf_offload_dev *offdev, > struct net_device *netdev); > bool bpf_offload_dev_match(struct bpf_prog *prog, struct net_device *netdev); > > +void *bpf_dev_bound_resolve_kfunc(struct bpf_prog *prog, u32 func_id); > + This probably requires an inline version for !CONFIG_NET. > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index d434a994ee04..c3e501e3e39c 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -2097,6 +2097,13 @@ bool bpf_prog_map_compatible(struct bpf_map *map, > if (fp->kprobe_override) > return false; > > + /* When tail-calling from a non-dev-bound program to a dev-bound one, > + * XDP metadata helpers should be disabled. Until it's implemented, > + * prohibit adding dev-bound programs to tail-call maps. > + */ > + if (bpf_prog_is_dev_bound(fp->aux)) > + return false; > + > spin_lock(&map->owner.lock); > if (!map->owner.type) { > /* There's no owner yet where we could check for > diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c > index f714c941f8ea..3b6c9023f24d 100644 > --- a/kernel/bpf/offload.c > +++ b/kernel/bpf/offload.c > @@ -757,6 +757,29 @@ void bpf_dev_bound_netdev_unregister(struct net_device *dev) > up_write(&bpf_devs_lock); > } > > +void *bpf_dev_bound_resolve_kfunc(struct bpf_prog *prog, u32 func_id) > +{ > + const struct xdp_metadata_ops *ops; > + void *p = NULL; > + > + down_read(&bpf_devs_lock); > + if (!prog->aux->offload || !prog->aux->offload->netdev) This happens when netdev is unregistered in the middle of bpf_prog_load and the bpf_offload_dev_match() will eventually fail during dev_xdp_attach()? A comment will be useful. > + goto out; > + > + ops = prog->aux->offload->netdev->xdp_metadata_ops; > + if (!ops) > + goto out; > + > + if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_TIMESTAMP)) > + p = ops->xmo_rx_timestamp; > + else if (func_id == xdp_metadata_kfunc_id(XDP_METADATA_KFUNC_RX_HASH)) > + p = ops->xmo_rx_hash; > +out: > + up_read(&bpf_devs_lock); > + > + return p; > +} > + > static int __init bpf_offload_init(void) > { > int err; > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 203d8cfeda70..e61fe0472b9b 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -15479,12 +15479,35 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, > struct bpf_insn *insn_buf, int insn_idx, int *cnt) > { > const struct bpf_kfunc_desc *desc; > + void *xdp_kfunc; > > if (!insn->imm) { > verbose(env, "invalid kernel function call not eliminated in verifier pass\n"); > return -EINVAL; > } > > + *cnt = 0; > + > + if (xdp_is_metadata_kfunc_id(insn->imm)) { > + if (!bpf_prog_is_dev_bound(env->prog->aux)) { The "xdp_is_metadata_kfunc_id() && (!bpf_prog_is_dev_bound() || bpf_prog_is_offloaded())" test should have been done much earlier in add_kfunc_call(). Then the later stage of the verifier does not have to keep worrying about it like here. nit. may be rename xdp_is_metadata_kfunc_id() to bpf_dev_bound_kfunc_id() and hide the "!bpf_prog_is_dev_bound() || bpf_prog_is_offloaded()" test into bpf_dev_bound_kfunc_check(&env->log, env->prog). The change in fixup_kfunc_call could then become: if (bpf_dev_bound_kfunc_id(insn->imm)) { xdp_kfunc = bpf_dev_bound_resolve_kfunc(env->prog, insn->imm); /* ... */ } > + verbose(env, "metadata kfuncs require device-bound program\n"); > + return -EINVAL; > + } > + > + if (bpf_prog_is_offloaded(env->prog->aux)) { > + verbose(env, "metadata kfuncs can't be offloaded\n"); > + return -EINVAL; > + } > + > + xdp_kfunc = bpf_dev_bound_resolve_kfunc(env->prog, insn->imm); > + if (xdp_kfunc) { > + insn->imm = BPF_CALL_IMM(xdp_kfunc); > + return 0; > + } > + > + /* fallback to default kfunc when not supported by netdev */ > + } > +