From mboxrd@z Thu Jan 1 00:00:00 1970 From: Song Liu Subject: Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload Date: Thu, 20 Sep 2018 05:48:26 +0000 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="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Alexei Starovoitov , "David S . Miller" , "daniel@iogearbox.net" , "peterz@infradead.org" , "acme@kernel.org" , "netdev@vger.kernel.org" , "Kernel Team" To: Alexei Starovoitov Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:41356 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726295AbeITLaw (ORCPT ); Thu, 20 Sep 2018 07:30:52 -0400 In-Reply-To: Content-Language: en-US Content-ID: <9409B7DD7EC21442BC6C13EC21164F2E@namprd15.prod.outlook.com> Sender: netdev-owner@vger.kernel.org List-ID: > On Sep 19, 2018, at 5:59 PM, Alexei Starovoitov wrote: >=20 > On 9/19/18 4:44 PM, Song Liu wrote: >>=20 >>=20 >>> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov wrote: >>>=20 >>> 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 w= as loaded. >>> Use empty program name as unload indication. >>>=20 >>> Signed-off-by: Alexei Starovoitov >>> --- >>> kernel/bpf/core.c | 22 ++++++++++++++++++++-- >>> 1 file changed, 20 insertions(+), 2 deletions(-) >>>=20 >>> 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 *pro= g, >>> *symbol_end =3D addr + hdr->pages * PAGE_SIZE; >>> } >>>=20 >>> -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 =3D sym + KSYM_NAME_LEN; >>>=20 >>> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_pro= g *prog, char *sym) >>> sym +=3D snprintf(sym, KSYM_NAME_LEN, "bpf_prog_"); >>> sym =3D bin2hex(sym, prog->tag, sizeof(prog->tag)); >>> if (prog->aux->name[0]) >>> - snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name); >>> + sym +=3D snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name); >>> else >>> *sym =3D 0; >>> + return sym; >>> } >>>=20 >>> static __always_inline unsigned long >>> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const st= ruct bpf_prog *fp) >>>=20 >>> 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; >>>=20 >>> + bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end); >>> + sym =3D 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++ =3D 0; >>=20 >> nit: This logic feels a little weird to me. How about we wrap the extra = logic >> in a separate function: >>=20 >> size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf= ) >=20 > probably bpf_get_prog_name_u64_padded() ? > would be cleaner indeed. bpf_get_prog_name_u64_padded does sound better.=20 > Will send v2 once Peter and Arnaldo provide their feedback. >=20 > Thanks for reviewing!