From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.9 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F1941C4727C for ; Tue, 29 Sep 2020 23:53:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7BE43206D8 for ; Tue, 29 Sep 2020 23:53:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="St8FUH5Z" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729548AbgI2XxF (ORCPT ); Tue, 29 Sep 2020 19:53:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39580 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728192AbgI2XxE (ORCPT ); Tue, 29 Sep 2020 19:53:04 -0400 Received: from mail-ej1-x642.google.com (mail-ej1-x642.google.com [IPv6:2a00:1450:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5F0EBC061755 for ; Tue, 29 Sep 2020 16:53:04 -0700 (PDT) Received: by mail-ej1-x642.google.com with SMTP id r7so64129ejs.11 for ; Tue, 29 Sep 2020 16:53:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+zkNZ3rHDtIXpRMfemrb4Rnw6S05cCl4Cwqw5YJtKgE=; b=St8FUH5ZkpD514Lc1ACTepBhIVInJ2OFFojurFuFNigfD0qEhWUZYz5uGi9mEEldWV cYinhyzT2ciGRMh7BVJ3zSiVRlwxCEB65TUcwghJm+p6kVSQ+iNqnh6SFBQSeLLjfZWG pW9eOx01yFqdSx+ngBIq6Iuyu1EKZI5JxUsqzVhYLeNaUBRB+dwmjfTk+Z8hH/uM6Mq5 x63nxX9LDHliN7Oppxeln0tnvfYs5k5qZXt3j+6n8zKMAfUA67a9qFZMr5bgN0wV+Ndw aB74tWJ5/ahZJfsmk1EfNNj4vzz7sKpGeNdLyxl6RBlROY/MP46drqS98vl+8ajG81Kr V79Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+zkNZ3rHDtIXpRMfemrb4Rnw6S05cCl4Cwqw5YJtKgE=; b=TMQMFjDat/AbE+yN9VlAjZNPgVd4cD8AB9SgwyXeI8c1nso7Y+u/u9lylXgCQkkoph VebUkS8ozAxJMARZHXQaPhDdah4J6WmPKyZFC28aSCRYKXuIqpwBs4JKwzsvMLXU07tl lDpl5NlfuFdiniXr35AtwM1jfLvZregRKeSrUhJXfT2keCPN54YzODsw1JiSCtATjiXG 0LNBPsAybRFb3QMttk8+4C7ED1AfSN8QZKswGY5skqFT/SJG74V2DEy26+6l/9EHnQkU TFomOeBHiYyJN+H7OnVf4UyJtqp5fTT15IoZuB0S5HVqvN8NgZvmpv+j+516slDsCsF+ U7og== X-Gm-Message-State: AOAM5339cHVywX7x1CSFMTOWT84gVQ2+XADg/wbwMFJU/ZwJKPYqjMLx CkGPhw/0IvQ2ubRuaOORHMwQPxHhFqhW46WQNVcgfA== X-Google-Smtp-Source: ABdhPJwykJ2TBuIhAw1k5+bwvnp7SG868xuuOD6eKYx67r9ek9EorQK1sczyBF4wXcpaiattQ7jEqHrYerczcuTM62w= X-Received: by 2002:a17:906:7d52:: with SMTP id l18mr119950ejp.220.1601423582833; Tue, 29 Sep 2020 16:53:02 -0700 (PDT) MIME-Version: 1.0 References: <20200916223512.2885524-1-haoluo@google.com> <20200916223512.2885524-5-haoluo@google.com> In-Reply-To: From: Hao Luo Date: Tue, 29 Sep 2020 16:52:51 -0700 Message-ID: Subject: Re: [PATCH bpf-next v3 4/6] bpf: Introduce bpf_per_cpu_ptr() To: Andrii Nakryiko Cc: Networking , bpf , open list , "open list:KERNEL SELFTEST FRAMEWORK" , Shuah Khan , Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Quentin Monnet , Steven Rostedt , Ingo Molnar Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Andrii, Thanks for taking a look. Sorry for the late reply. Spent some time on rebasing and fixing a build issue in my development environment that started happening in v5.9. On Mon, Sep 21, 2020 at 11:09 AM Andrii Nakryiko wrote: > > On Wed, Sep 16, 2020 at 3:39 PM Hao Luo wrote: > > > > Add bpf_per_cpu_ptr() to help bpf programs access percpu vars. > > bpf_per_cpu_ptr() has the same semantic as per_cpu_ptr() in the kernel > > except that it may return NULL. This happens when the cpu parameter is > > out of range. So the caller must check the returned value. > > > > Acked-by: Andrii Nakryiko > > Signed-off-by: Hao Luo > > --- > > include/linux/bpf.h | 4 +++ > > include/linux/btf.h | 11 ++++++ > > include/uapi/linux/bpf.h | 18 ++++++++++ > > kernel/bpf/btf.c | 10 ------ > > kernel/bpf/helpers.c | 18 ++++++++++ > > kernel/bpf/verifier.c | 64 ++++++++++++++++++++++++++++++++-- > > kernel/trace/bpf_trace.c | 2 ++ > > tools/include/uapi/linux/bpf.h | 18 ++++++++++ > > 8 files changed, 132 insertions(+), 13 deletions(-) > > > > I already acked this, but see my concern about O(N) look up for > .data..percpu. Feel free to follow up on this with a separate patch. > Thanks! > > [...] > > > @@ -4003,6 +4008,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, > > if (type != expected_type) > > goto err_type; > > } > > + } else if (arg_type == ARG_PTR_TO_PERCPU_BTF_ID) { > > + expected_type = PTR_TO_PERCPU_BTF_ID; > > + if (type != expected_type) > > + goto err_type; > > + if (!reg->btf_id) { > > + verbose(env, "Helper has invalid btf_id in R%d\n", regno); > > + return -EACCES; > > + } > > + meta->ret_btf_id = reg->btf_id; > > FYI, this will conflict with Lorenz's refactoring, so you might need > to rebase and solve the conflicts if his patch set lands first. > Indeed. Do hit this while rebasing but managed to resolve it. Please take a look and let me know if you have comments there in v4 > > @@ -7413,6 +7451,7 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn) > > dst_reg->mem_size = aux->btf_var.mem_size; > > break; > > case PTR_TO_BTF_ID: > > + case PTR_TO_PERCPU_BTF_ID: > > dst_reg->btf_id = aux->btf_var.btf_id; > > break; > > default: > > @@ -9313,10 +9352,14 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env, > > struct bpf_insn *insn, > > struct bpf_insn_aux_data *aux) > > { > > - u32 type, id = insn->imm; > > + u32 datasec_id, type, id = insn->imm; > > + const struct btf_var_secinfo *vsi; > > + const struct btf_type *datasec; > > const struct btf_type *t; > > const char *sym_name; > > + bool percpu = false; > > u64 addr; > > + int i; > > > > if (!btf_vmlinux) { > > verbose(env, "kernel is missing BTF, make sure CONFIG_DEBUG_INFO_BTF=y is specified in Kconfig.\n"); > > @@ -9348,12 +9391,27 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env, > > return -ENOENT; > > } > > > > + datasec_id = btf_find_by_name_kind(btf_vmlinux, ".data..percpu", > > + BTF_KIND_DATASEC); > > this is a relatively expensive O(N) operation, it probably makes sense > to cache it (there are about 80'000 types now in BTF for my typical > kernel config, so iterating that much for every single ldimm64 for > ksym is kind of expensive. > ACK. This currently works. I can do it in another patch. Hao