[RFC,RESEND,bpf-next,1/4] bpf: Rollback to text_poke when arch not supported ftrace direct call
Message ID | 20221220021319.1655871-2-pulehui@huaweicloud.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp2729527wrn; Mon, 19 Dec 2022 18:17:46 -0800 (PST) X-Google-Smtp-Source: AA0mqf4c4uAuv9TdxGydbvcKIG41YtGcedw5FW4xjeMCQAx40aQR7/Jl3cgpybnfcqIqmeSJhdzu X-Received: by 2002:a05:6402:450a:b0:45c:835b:ac67 with SMTP id ez10-20020a056402450a00b0045c835bac67mr38646232edb.34.1671502666025; Mon, 19 Dec 2022 18:17:46 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671502666; cv=none; d=google.com; s=arc-20160816; b=nXsko+M+pjfZ/7OpJ349pAsH1deiSAPh8UZihnH2JXeTpR7Yqi1KXMOrPsvrHd7zxW rqLnCdqqRHM4r5TsFYs+EKI/TkbiydIqFXYwlc0ARKTiLOKzULxGEhyJfMAOUQcL3Xy4 IzaRYf5xeCm9cdaXnM/K5EXlCvI4Z8UkJz3O8Q2hQf1iTtQL5Pg6pYd3rqC1+ZT5roZx shkeoKCs8/AOC1hVjAzc86PDU6Ky51OpKNVh49VMpK9YNn8tXpJTsLAfc6Z3EGBal/gg 2fiWQjl8Xas9GC/yM6zyx2vKHFG0ZaSHSvA+Id29DwZKOvmvkq5GAB1C39B3k+07SokX SjDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=DU2LJ1er6FgUDddmbRgbEdTAR3QJqVIOrPkCsnxCt6o=; b=xZxfzAtSteEGTea1pX5Krd9vL27Sz9lUjmwgeq181z1Lwnl7Xu/kXBePHvnBxsykRj 3DJ8FKYhjXpuC4PJFrHnFbhsbnJ1T0qU9tzSZ82KahGsybpHG5DsEsKzxgjpY76ndpMN qmj2umASt5061HmmBhYPeTpGPRiZWIFRhOe78ccof00yY5b0fDX0dhKEO6mTXydj5iIs V+7wl9tgjpeiX2BFsMvz4KlgFjjkPqlUTGaYuK6G3aiBGdsgjWuVE/PDQUxl6fncBx0T jsVzxvYIEBgtYBQHeNaZKTlf2KXZL09ZfOGd1jOWOGw6Ql0aGqvDZUQ3Cg5+V3U6wtBv 61EA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h8-20020a056402094800b00461d726439asi9025558edz.538.2022.12.19.18.17.22; Mon, 19 Dec 2022 18:17:46 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232913AbiLTCMX (ORCPT <rfc822;abdi.embedded@gmail.com> + 99 others); Mon, 19 Dec 2022 21:12:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55372 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232618AbiLTCMT (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 19 Dec 2022 21:12:19 -0500 Received: from dggsgout12.his.huawei.com (dggsgout12.his.huawei.com [45.249.212.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A84DA1B3; Mon, 19 Dec 2022 18:12:17 -0800 (PST) Received: from mail02.huawei.com (unknown [172.30.67.153]) by dggsgout12.his.huawei.com (SkyGuard) with ESMTP id 4Nbg7b1HjFz4f3kpC; Tue, 20 Dec 2022 10:12:11 +0800 (CST) Received: from localhost.localdomain (unknown [10.67.175.61]) by APP4 (Coremail) with SMTP id gCh0CgDnT7P6GaFju6m1AA--.8438S3; Tue, 20 Dec 2022 10:12:14 +0800 (CST) From: Pu Lehui <pulehui@huaweicloud.com> To: bpf@vger.kernel.org, linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Cc: =?utf-8?b?QmrDtnJuIFTDtnBlbA==?= <bjorn@kernel.org>, Luke Nelson <luke.r.nels@gmail.com>, Xi Wang <xi.wang@gmail.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>, Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Pu Lehui <pulehui@huawei.com>, Pu Lehui <pulehui@huaweicloud.com> Subject: [RFC PATCH RESEND bpf-next 1/4] bpf: Rollback to text_poke when arch not supported ftrace direct call Date: Tue, 20 Dec 2022 10:13:16 +0800 Message-Id: <20221220021319.1655871-2-pulehui@huaweicloud.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221220021319.1655871-1-pulehui@huaweicloud.com> References: <20221220021319.1655871-1-pulehui@huaweicloud.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: gCh0CgDnT7P6GaFju6m1AA--.8438S3 X-Coremail-Antispam: 1UD129KBjvJXoW7WF1UZw43uw17GF4rZrWUurg_yoW8JFykpF 43Gw13ua1jqFZrWF9rX3WkXrWYv3ykJ3yUGF47K34Fkan5Kr95tr4DZwn3XrWFkr9YvF1a yrWFqFZ0qa1UZa7anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUBE14x267AKxVWrJVCq3wAFc2x0x2IEx4CE42xK8VAvwI8IcIk0 rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2048vs2IY020E87I2jVAFwI0_Jr4l82xGYIkIc2 x26xkF7I0E14v26ryj6s0DM28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48ve4kI8wA2z4x0 Y4vE2Ix0cI8IcVAFwI0_tr0E3s1l84ACjcxK6xIIjxv20xvEc7CjxVAFwI0_Gr1j6F4UJw A2z4x0Y4vEx4A2jsIE14v26rxl6s0DM28EF7xvwVC2z280aVCY1x0267AKxVW0oVCq3wAS 0I0E0xvYzxvE52x082IY62kv0487Mc02F40EFcxC0VAKzVAqx4xG6I80ewAv7VC0I7IYx2 IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Jr0_Gr1lOx8S6xCaFVCjc4AY6r1j6r4UM4x0 Y48IcxkI7VAKI48JM4x0x7Aq67IIx4CEVc8vx2IErcIFxwACI402YVCY1x02628vn2kIc2 xKxwCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02F40E14v2 6r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_GFv_WrylIxkGc2 Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7CjxVAFwI0_ Gr0_Cr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r1j6r4UMI IF0xvEx4A2jsIEc7CjxVAFwI0_Gr0_Gr1UYxBIdaVFxhVjvjDU0xZFpf9x0JUHE__UUUUU = X-CM-SenderInfo: psxovxtxl6x35dzhxuhorxvhhfrp/ X-CFilter-Loop: Reflected X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE, SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1752697579745416834?= X-GMAIL-MSGID: =?utf-8?q?1752697579745416834?= |
Series |
Support bpf trampoline for RV64
|
|
Commit Message
Pu Lehui
Dec. 20, 2022, 2:13 a.m. UTC
From: Pu Lehui <pulehui@huawei.com> The current bpf trampoline attach to kernel functions via ftrace direct call API, while text_poke is applied for bpf2bpf attach and tail call optimization. For architectures that do not support ftrace direct call, text_poke is still able to attach bpf trampoline to kernel functions. Let's relax it by rollback to text_poke when architecture not supported. Signed-off-by: Pu Lehui <pulehui@huawei.com> --- kernel/bpf/trampoline.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
Comments
On 2022/12/20 10:32, Xu Kuohai wrote: > On 12/20/2022 10:13 AM, Pu Lehui wrote: >> From: Pu Lehui <pulehui@huawei.com> >> >> The current bpf trampoline attach to kernel functions via ftrace direct >> call API, while text_poke is applied for bpf2bpf attach and tail call >> optimization. For architectures that do not support ftrace direct call, >> text_poke is still able to attach bpf trampoline to kernel functions. >> Let's relax it by rollback to text_poke when architecture not supported. >> >> Signed-off-by: Pu Lehui <pulehui@huawei.com> >> --- >> kernel/bpf/trampoline.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c >> index d6395215b849..386197a7952c 100644 >> --- a/kernel/bpf/trampoline.c >> +++ b/kernel/bpf/trampoline.c >> @@ -228,15 +228,11 @@ static int modify_fentry(struct bpf_trampoline >> *tr, void *old_addr, void *new_ad >> static int register_fentry(struct bpf_trampoline *tr, void *new_addr) >> { >> void *ip = tr->func.addr; >> - unsigned long faddr; >> int ret; >> - faddr = ftrace_location((unsigned long)ip); >> - if (faddr) { >> - if (!tr->fops) >> - return -ENOTSUPP; >> + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && >> + !!ftrace_location((unsigned long)ip)) >> tr->func.ftrace_managed = true; >> - } >> > > After this patch, a kernel function with true trace_location will be > patched > by bpf when CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is disabled, which > means > that a kernel function may be patched by both bpf and ftrace in a mutually > unaware way. This will cause ftrace and bpf_arch_text_poke to fail in a > somewhat random way if the function to be patched was already patched > by the other. Thanks for your review. And yes, this is a backward compatible solution for architectures that not support DYNAMIC_FTRACE_WITH_DIRECT_CALLS. > >> if (bpf_trampoline_module_get(tr)) >> return -ENOENT;
Pu Lehui <pulehui@huaweicloud.com> writes: > On 2022/12/20 10:32, Xu Kuohai wrote: >> On 12/20/2022 10:13 AM, Pu Lehui wrote: >>> From: Pu Lehui <pulehui@huawei.com> >>> >>> The current bpf trampoline attach to kernel functions via ftrace direct >>> call API, while text_poke is applied for bpf2bpf attach and tail call >>> optimization. For architectures that do not support ftrace direct call, >>> text_poke is still able to attach bpf trampoline to kernel functions. >>> Let's relax it by rollback to text_poke when architecture not supported. >>> >>> Signed-off-by: Pu Lehui <pulehui@huawei.com> >>> --- >>> kernel/bpf/trampoline.c | 8 ++------ >>> 1 file changed, 2 insertions(+), 6 deletions(-) >>> >>> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c >>> index d6395215b849..386197a7952c 100644 >>> --- a/kernel/bpf/trampoline.c >>> +++ b/kernel/bpf/trampoline.c >>> @@ -228,15 +228,11 @@ static int modify_fentry(struct bpf_trampoline >>> *tr, void *old_addr, void *new_ad >>> static int register_fentry(struct bpf_trampoline *tr, void *new_addr) >>> { >>> void *ip = tr->func.addr; >>> - unsigned long faddr; >>> int ret; >>> - faddr = ftrace_location((unsigned long)ip); >>> - if (faddr) { >>> - if (!tr->fops) >>> - return -ENOTSUPP; >>> + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && >>> + !!ftrace_location((unsigned long)ip)) >>> tr->func.ftrace_managed = true; >>> - } >>> >> >> After this patch, a kernel function with true trace_location will be >> patched >> by bpf when CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is disabled, which >> means >> that a kernel function may be patched by both bpf and ftrace in a mutually >> unaware way. This will cause ftrace and bpf_arch_text_poke to fail in a >> somewhat random way if the function to be patched was already patched >> by the other. > > Thanks for your review. And yes, this is a backward compatible solution > for architectures that not support DYNAMIC_FTRACE_WITH_DIRECT_CALLS. It's not "backward compatible". Reiterating what Kuohai said; The BPF trampoline must be aware of ftrace's state -- with this patch, the trampoline can't blindly poke the text managed my ftrace. I'd recommend to remove this patch from the series. Björn
On 2023/1/3 20:05, Björn Töpel wrote: > Pu Lehui <pulehui@huaweicloud.com> writes: > >> On 2022/12/20 10:32, Xu Kuohai wrote: >>> On 12/20/2022 10:13 AM, Pu Lehui wrote: >>>> From: Pu Lehui <pulehui@huawei.com> >>>> >>>> The current bpf trampoline attach to kernel functions via ftrace direct >>>> call API, while text_poke is applied for bpf2bpf attach and tail call >>>> optimization. For architectures that do not support ftrace direct call, >>>> text_poke is still able to attach bpf trampoline to kernel functions. >>>> Let's relax it by rollback to text_poke when architecture not supported. >>>> >>>> Signed-off-by: Pu Lehui <pulehui@huawei.com> >>>> --- >>>> kernel/bpf/trampoline.c | 8 ++------ >>>> 1 file changed, 2 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c >>>> index d6395215b849..386197a7952c 100644 >>>> --- a/kernel/bpf/trampoline.c >>>> +++ b/kernel/bpf/trampoline.c >>>> @@ -228,15 +228,11 @@ static int modify_fentry(struct bpf_trampoline >>>> *tr, void *old_addr, void *new_ad >>>> static int register_fentry(struct bpf_trampoline *tr, void *new_addr) >>>> { >>>> void *ip = tr->func.addr; >>>> - unsigned long faddr; >>>> int ret; >>>> - faddr = ftrace_location((unsigned long)ip); >>>> - if (faddr) { >>>> - if (!tr->fops) >>>> - return -ENOTSUPP; >>>> + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && >>>> + !!ftrace_location((unsigned long)ip)) >>>> tr->func.ftrace_managed = true; >>>> - } >>>> >>> >>> After this patch, a kernel function with true trace_location will be >>> patched >>> by bpf when CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is disabled, which >>> means >>> that a kernel function may be patched by both bpf and ftrace in a mutually >>> unaware way. This will cause ftrace and bpf_arch_text_poke to fail in a >>> somewhat random way if the function to be patched was already patched >>> by the other. >> >> Thanks for your review. And yes, this is a backward compatible solution >> for architectures that not support DYNAMIC_FTRACE_WITH_DIRECT_CALLS. > > It's not "backward compatible". Reiterating what Kuohai said; The BPF > trampoline must be aware of ftrace's state -- with this patch, the > trampoline can't blindly poke the text managed my ftrace. > > I'd recommend to remove this patch from the series. > After deep consideration, Kuohai's catching is much more reasonable. Will remove it in the next. In the meantime, I found that song and guoren have worked on supporting riscv ftrace with direct call [0], so we can concentrate on making bpf_arch_text_poke specifically for the bpf context. However, riscv ftrace base framework will change because [0] uses t0 as the link register of traced function. We should consider the generality of riscv bpf trampoline for kernel function and bpf context. It's not clear if [0] will be upstreamed, so maybe we should wait for it? [0] https://lore.kernel.org/linux-riscv/20221208091244.203407-7-guoren@kernel.org Anyway, thanks both of you for the review. > > Björn
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c index d6395215b849..386197a7952c 100644 --- a/kernel/bpf/trampoline.c +++ b/kernel/bpf/trampoline.c @@ -228,15 +228,11 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad static int register_fentry(struct bpf_trampoline *tr, void *new_addr) { void *ip = tr->func.addr; - unsigned long faddr; int ret; - faddr = ftrace_location((unsigned long)ip); - if (faddr) { - if (!tr->fops) - return -ENOTSUPP; + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && + !!ftrace_location((unsigned long)ip)) tr->func.ftrace_managed = true; - } if (bpf_trampoline_module_get(tr)) return -ENOENT;