From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Subject: Re: [bpf-next PATCH 3/3] bpf: add sample program to trace map events
Date: Thu, 19 Apr 2018 17:27:37 -0700
Message-ID: <20180420002735.ytmnhzs73rwkwewm@ast-mbp>
References: <152406544226.3465.948692097697975172.stgit@localhost.localdomain>
 <152406545918.3465.14253635905960610284.stgit@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
        mingo@redhat.com, rostedt@goodmis.org, brouer@redhat.com,
        fulvio.risso@polito.it, "David S. Miller" <davem@davemloft.net>
To: Sebastiano Miano <sebastiano.miano@polito.it>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pl0-f67.google.com ([209.85.160.67]:40648 "EHLO
        mail-pl0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753761AbeDTA1l (ORCPT
        <rfc822;netdev@vger.kernel.org>); Thu, 19 Apr 2018 20:27:41 -0400
Received: by mail-pl0-f67.google.com with SMTP id t22-v6so4214776plo.7
        for <netdev@vger.kernel.org>; Thu, 19 Apr 2018 17:27:41 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <152406545918.3465.14253635905960610284.stgit@localhost.localdomain>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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.