[{"id":1771373,"web_url":"http://patchwork.ozlabs.org/comment/1771373/","msgid":"<59C180D8.1050900@iogearbox.net>","list_archive_url":null,"date":"2017-09-19T20:40:56","subject":"Re: [PATCH net-next v4 3/4] bpf: add helper bpf_perf_prog_read_value","submitter":{"id":65705,"url":"http://patchwork.ozlabs.org/api/people/65705/","name":"Daniel Borkmann","email":"daniel@iogearbox.net"},"content":"On 09/19/2017 09:04 AM, Yonghong Song wrote:\n[...]\n> #ifdef CONFIG_CGROUP_PERF\n> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h\n> index 2c68b9e..ba77022 100644\n> --- a/include/uapi/linux/bpf.h\n> +++ b/include/uapi/linux/bpf.h\n> @@ -590,6 +590,13 @@ union bpf_attr {\n> * @buf: buf to fill\n> * @buf_size: size of the buf\n> * Return: 0 on success or negative error code\n> + *\n> + * int bpf_perf_prog_read_value(ctx, buf, buf_size)\n> + * read perf prog attached perf event counter and enabled/running time\n> + * @ctx: pointer to ctx\n> + * @buf: buf to fill\n> + * @buf_size: size of the buf\n> + * Return : 0 on success or negative error code\n> */\n> #define __BPF_FUNC_MAPPER(FN)\t\t\\\n> \tFN(unspec),\t\t\t\\\n> @@ -647,6 +654,7 @@ union bpf_attr {\n> \tFN(sk_redirect_map),\t\t\\\n> \tFN(sock_map_update),\t\t\\\n> \tFN(perf_event_read_value),\t\t\\\n> +\tFN(perf_prog_read_value),\t\t\\\n\n(Same here.)\n\n>\n> /* integer value in 'imm' field of BPF_CALL instruction selects which helper\n> * function eBPF program intends to call\n> diff --git a/kernel/events/core.c b/kernel/events/core.c\n> index 2d5bbe5..d039086 100644\n> --- a/kernel/events/core.c\n> +++ b/kernel/events/core.c\n> @@ -8081,6 +8081,7 @@ static void bpf_overflow_handler(struct perf_event *event,\n> \tstruct bpf_perf_event_data_kern ctx = {\n> \t\t.data = data,\n> \t\t.regs = regs,\n> +\t\t.event = event,\n> \t};\n> \tint ret = 0;\n>\n> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c\n> index 39ce5d9..596b5c9 100644\n> --- a/kernel/trace/bpf_trace.c\n> +++ b/kernel/trace/bpf_trace.c\n> @@ -603,6 +603,18 @@ BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,\n> \t\t\t flags, 0, 0);\n> }\n>\n> +BPF_CALL_3(bpf_perf_prog_read_value_tp, void *, ctx, struct bpf_perf_event_value *,\n> +\tbuf, u32, size)\n\nNit: indent\n\n> +{\n> +\tstruct bpf_perf_event_data_kern *kctx = (struct bpf_perf_event_data_kern *)ctx;\n\nWhy having the arg as void * and have this detour instead of having\nstruct bpf_perf_event_data_kern * right in the helper signature as\nargument?\n\n> +\tif (size != sizeof(struct bpf_perf_event_value))\n\nunlikely()\n\n> +\t\treturn -EINVAL;\n> +\n> +\treturn perf_event_read_local(kctx->event, &buf->counter, &buf->enabled,\n> +\t\t\t\t &buf->running);\n> +}\n\nbpf_perf_prog_read_value_proto_tp would go right underneath here,\nand bpf_get_stackid_proto_tp below the previous helper above.\n\n> static const struct bpf_func_proto bpf_get_stackid_proto_tp = {\n> \t.func\t\t= bpf_get_stackid_tp,\n> \t.gpl_only\t= true,\n> @@ -612,6 +624,15 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {\n> \t.arg3_type\t= ARG_ANYTHING,\n> };\n>\n> +static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = {\n> + .func = bpf_perf_prog_read_value_tp,\n> + .gpl_only = true,\n> + .ret_type = RET_INTEGER,\n> + .arg1_type = ARG_PTR_TO_CTX,\n> + .arg2_type = ARG_PTR_TO_UNINIT_MEM,\n\nSame on error path.\n\n> + .arg3_type = ARG_CONST_SIZE,\n> +};\n> +\n> static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)\n> {\n> \tswitch (func_id) {\n> @@ -619,6 +640,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)\n> \t\treturn &bpf_perf_event_output_proto_tp;\n> \tcase BPF_FUNC_get_stackid:\n> \t\treturn &bpf_get_stackid_proto_tp;\n> +\tcase BPF_FUNC_perf_prog_read_value:\n> +\t\treturn &bpf_perf_prog_read_value_proto_tp;\n> \tdefault:\n> \t\treturn tracing_func_proto(func_id);\n> \t}\n>","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xxZXL2GrRz9s81\n\tfor ;\n\tWed, 20 Sep 2017 06:41:06 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751653AbdISUlD (ORCPT );\n\tTue, 19 Sep 2017 16:41:03 -0400","from www62.your-server.de ([213.133.104.62]:56363 \"EHLO\n\twww62.your-server.de\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751583AbdISUlD (ORCPT\n\t); Tue, 19 Sep 2017 16:41:03 -0400","from [85.7.161.218] (helo=localhost.localdomain)\n\tby www62.your-server.de with esmtpsa (TLSv1.2:DHE-RSA-AES256-SHA:256)\n\t(Exim 4.85_2) (envelope-from )\n\tid 1duPKC-0002qJ-Ua; Tue, 19 Sep 2017 22:40:57 +0200"],"Message-ID":"<59C180D8.1050900@iogearbox.net>","Date":"Tue, 19 Sep 2017 22:40:56 +0200","From":"Daniel Borkmann ","User-Agent":"Mozilla/5.0 (X11; Linux x86_64;\n\trv:31.0) Gecko/20100101 Thunderbird/31.7.0","MIME-Version":"1.0","To":"Yonghong Song , peterz@infradead.org,\n\trostedt@goodmis.org, ast@fb.com, netdev@vger.kernel.org","CC":"kernel-team@fb.com","Subject":"Re: [PATCH net-next v4 3/4] bpf: add helper bpf_perf_prog_read_value","References":"<20170919070413.3838201-1-yhs@fb.com>\n\t<20170919070413.3838201-4-yhs@fb.com>","In-Reply-To":"<20170919070413.3838201-4-yhs@fb.com>","Content-Type":"text/plain; charset=windows-1252; format=flowed","Content-Transfer-Encoding":"7bit","X-Authenticated-Sender":"daniel@iogearbox.net","X-Virus-Scanned":"Clear (ClamAV 0.99.2/23853/Tue Sep 19 14:42:49 2017)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]