From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexei Starovoitov Subject: Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload Date: Wed, 19 Sep 2018 17:59:33 -0700 Message-ID: References: <20180919223935.999270-1-ast@kernel.org> <20180919223935.999270-3-ast@kernel.org> <3FB6EC86-700C-4C80-A467-55B6753B07FE@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Cc: "David S . Miller" , "daniel@iogearbox.net" , "peterz@infradead.org" , "acme@kernel.org" , "netdev@vger.kernel.org" , Kernel Team To: Song Liu , Alexei Starovoitov Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:49916 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725904AbeITGl1 (ORCPT ); Thu, 20 Sep 2018 02:41:27 -0400 In-Reply-To: <3FB6EC86-700C-4C80-A467-55B6753B07FE@fb.com> Sender: netdev-owner@vger.kernel.org List-ID: On 9/19/18 4:44 PM, Song Liu wrote: > > >> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov wrote: >> >> use perf_event_mmap_bpf_prog() helper to notify user space >> about JITed bpf programs. >> Use RECORD_MMAP perf event to tell user space where JITed bpf program was loaded. >> Use empty program name as unload indication. >> >> Signed-off-by: Alexei Starovoitov >> --- >> kernel/bpf/core.c | 22 ++++++++++++++++++++-- >> 1 file changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c >> index 3f5bf1af0826..ddf11fdafd36 100644 >> --- a/kernel/bpf/core.c >> +++ b/kernel/bpf/core.c >> @@ -384,7 +384,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog, >> *symbol_end = addr + hdr->pages * PAGE_SIZE; >> } >> >> -static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym) >> +static char *bpf_get_prog_name(const struct bpf_prog *prog, char *sym) >> { >> const char *end = sym + KSYM_NAME_LEN; >> >> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym) >> sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_"); >> sym = bin2hex(sym, prog->tag, sizeof(prog->tag)); >> if (prog->aux->name[0]) >> - snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name); >> + sym += snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name); >> else >> *sym = 0; >> + return sym; >> } >> >> static __always_inline unsigned long >> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp) >> >> void bpf_prog_kallsyms_add(struct bpf_prog *fp) >> { >> + unsigned long symbol_start, symbol_end; >> + char buf[KSYM_NAME_LEN], *sym; >> + >> if (!bpf_prog_kallsyms_candidate(fp) || >> !capable(CAP_SYS_ADMIN)) >> return; >> >> + bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end); >> + sym = bpf_get_prog_name(fp, buf); >> + sym++; /* sym - buf is the length of the name including trailing 0 */ >> + while (!IS_ALIGNED(sym - buf, sizeof(u64))) >> + *sym++ = 0; > > nit: This logic feels a little weird to me. How about we wrap the extra logic > in a separate function: > > size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf) probably bpf_get_prog_name_u64_padded() ? would be cleaner indeed. Will send v2 once Peter and Arnaldo provide their feedback. Thanks for reviewing!