From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [bpf-next PATCH 3/3] bpf: add sample program to trace map events Date: Fri, 20 Apr 2018 11:47:11 +0200 Message-ID: <20180420114711.1a06fb26@redhat.com> References: <152406544226.3465.948692097697975172.stgit@localhost.localdomain> <152406545918.3465.14253635905960610284.stgit@localhost.localdomain> <20180420002735.ytmnhzs73rwkwewm@ast-mbp> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Sebastiano Miano , netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net, mingo@redhat.com, rostedt@goodmis.org, fulvio.risso@polito.it, "David S. Miller" , brouer@redhat.com To: Alexei Starovoitov Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:56398 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754170AbeDTJrR (ORCPT ); Fri, 20 Apr 2018 05:47:17 -0400 In-Reply-To: <20180420002735.ytmnhzs73rwkwewm@ast-mbp> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 19 Apr 2018 17:27:37 -0700 Alexei Starovoitov wrote: > On Wed, Apr 18, 2018 at 05:30:59PM +0200, Sebastiano Miano wrote: > > This patch adds a sample program, called trace_map_events, > > that shows how to capture map events and filter them based on > > the map id. > ... > > +struct bpf_map_keyval_ctx { > > + u64 pad; // First 8 bytes are not accessible by bpf code > > + u32 type; // offset:8; size:4; signed:0; > > + u32 key_len; // offset:12; size:4; signed:0; > > + u32 key; // offset:16; size:4; signed:0; > > + bool key_trunc; // offset:20; size:1; signed:0; > > + u32 val_len; // offset:24; size:4; signed:0; > > + u32 val; // offset:28; size:4; signed:0; > > + bool val_trunc; // offset:32; size:1; signed:0; > > + int ufd; // offset:36; size:4; signed:1; > > + u32 id; // offset:40; size:4; signed:0; > > +}; > > + > > +SEC("tracepoint/bpf/bpf_map_lookup_elem") > > +int trace_bpf_map_lookup_elem(struct bpf_map_keyval_ctx *ctx) > > +{ > > + struct map_event_data data; > > + int cpu = bpf_get_smp_processor_id(); > > + bool *filter; > > + u32 key = 0, map_id = ctx->id; > > + > > + filter = bpf_map_lookup_elem(&filter_events, &key); > > + if (!filter) > > + return 1; > > + > > + if (!*filter) > > + goto send_event; > > + > > + /* > > + * If the map_id is not in the list of filtered > > + * ids we immediately return > > + */ > > + if (!bpf_map_lookup_elem(&filtered_ids, &map_id)) > > + return 0; > > + > > +send_event: > > + data.map_id = map_id; > > + data.evnt_type = MAP_LOOKUP; > > + data.map_type = ctx->type; > > + > > + bpf_perf_event_output(ctx, &map_event_trace, cpu, &data, sizeof(data)); > > + return 0; > > +} > > looks like the purpose of the series is to create map notify mechanism > so some external user space daemon can snoop all bpf map operations > that all other processes and bpf programs are doing. > I think it would be way better to create a proper mechanism for that > with permissions. > > tracepoints in the bpf core were introduced as introspection mechanism > for debugging. Right now we have better ways to do introspection > with ids, queries, etc that bpftool is using, so original purpose of > those tracepoints is gone and they actually rot. > Let's not repurpose them into this map notify logic. > I don't want tracepoints in the bpf core to become a stable interface > it will stiffen the development. Well, I need it exactly for introspection and debugging, and just need the missing ID (as it was introduced later). Can we just drop the sample program then? I would likely not use the sample program, because it is missing the PID+comm-name. For my use, I can simply use perf record to debug what programs are changing the map ID: perf record -e bpf:bpf_map_* -a --filter 'id == 2' This should be a fairly common troubleshooting step. I want to figure-out if anybody else (another userspace program) is changing my map. This can easily be caused by two userspace control programs running at the same time. Sysadm/scripts made a mistake and started two instances. Without the map ID, I cannot know what map perf is talking about... -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer