Thread (41 messages) flat view 41 messages, 5 authors, 2018-12-19

Re: [PATCH bpf-next 6/8] tools: bpftool: add probes for eBPF helper functions

From: Quentin Monnet <hidden>
Date: 2018-12-19 19:02:53

2018-12-18 01:42 UTC+0100 ~ Daniel Borkmann [off-list ref]
On 12/17/2018 11:18 AM, Quentin Monnet wrote:
quoted
2018-12-16 00:57 UTC+0100 ~ Daniel Borkmann [off-list ref]
quoted
On 12/15/2018 04:32 AM, Quentin Monnet wrote:
quoted
2018-12-15 01:08 UTC+0100 ~ Daniel Borkmann [off-list ref]
quoted
On 12/13/2018 01:19 PM, Quentin Monnet wrote:
quoted
Similarly to what was done for program types and map types, add a set of
probes to test the availability of the different eBPF helper functions
on the current system.

Sample output:

     # bpftool feature probe kernel
     ...
     Scanning eBPF helper functions...
     eBPF helper bpf_map_lookup_elem is available
     eBPF helper bpf_map_update_elem is available
     eBPF helper bpf_map_delete_elem is available
     ...

     # bpftool --json --pretty feature probe kernel
     {
         ...
         "helpers": {
             "have_bpf_map_lookup_elem_helper": true,
             "have_bpf_map_update_elem_helper": true,
             "have_bpf_map_delete_elem_helper": true,
             ...
         }
     }

     # bpftool feature probe kernel macros prefix BPFTOOL_
     ...
     /*** eBPF helper functions ***/
     #define BPFTOOL_BPF_MAP_LOOKUP_ELEM_HELPER
     #define BPFTOOL_BPF_MAP_UPDATE_ELEM_HELPER
     #define BPFTOOL_BPF_MAP_DELETE_ELEM_HELPER
Small nit: instead of BPFTOOL_ prefix, would it make sense
to generally use Have_ prefix (similar to json output). The
former seems somewhat bpftool related though it solely probes
the underlying kernel.
"BPFTOOL_" is provided on the command line ("prefix BPFTOOL_"), and it's
here just for the example. I initially had a non-configurable "HAVE_"
prefix instead, but after discussing it with Jakub we thought it best to
leave the choice of the prefix to the users, so they can choose what
works best for them, or adapt it and avoid any potential name conflict.
Ah, good point, makes sense then. As default you have no prefix? Perhaps
there it could be "HAVE_" prefix.
Yes, I can probably reinstate it as a default option if none was provided from the command line, for prog/map types and helpers.
quoted
quoted
quoted
quoted
Signed-off-by: Quentin Monnet <redacted>
Reviewed-by: Jakub Kicinski <redacted>
---
  .../bpftool/Documentation/bpftool-feature.rst |   4 +
  tools/bpf/bpftool/feature.c                   | 152 ++++++++++++++++++
  2 files changed, 156 insertions(+)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-feature.rst b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
index 23920a7490e9..083d30510cce 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-feature.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-feature.rst
@@ -39,6 +39,10 @@ DESCRIPTION
            names when including the output of this command as a header
            file.
  +          Note that when probed, some eBPF helpers (e.g.
+          **bpf_trace_printk**\ () or **bpf_probe_write_user**\ ()) may
+          print warnings to kernel logs.
+
      **bpftool feature help**
            Print short help message.
  diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 85928f172413..77221fff6ba9 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -24,6 +24,113 @@ enum probe_component {
      COMPONENT_KERNEL,
  };
  +#define MAX_HELPER_NAME_LEN 32
+struct helper_param {
+    enum bpf_prog_type progtype;
+    const char name[MAX_HELPER_NAME_LEN];
+};
+
+/* helper_progtype_and_name[index] associates to the BPF helper function of id
+ * "index" a name and a program type to run this helper with. In order to probe
+ * helper availability for programs offloaded to a network device, use
+ * offload-compatible types (e.g. XDP) everywhere we can. Caveats: helper
+ * probing may fail currently if only TC (but not XDP) is supported for
+ * offload.
+ */
+static const struct helper_param helper_progtype_and_name[] = {
+    { BPF_PROG_TYPE_XDP,        "no_helper_with_id_0" },
+    { BPF_PROG_TYPE_XDP,        "bpf_map_lookup_elem" },
+    { BPF_PROG_TYPE_XDP,        "bpf_map_update_elem" },
+    { BPF_PROG_TYPE_XDP,        "bpf_map_delete_elem" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_probe_read" },
+    { BPF_PROG_TYPE_XDP,        "bpf_ktime_get_ns" },
+    { BPF_PROG_TYPE_XDP,        "bpf_trace_printk" },
+    { BPF_PROG_TYPE_XDP,        "bpf_get_prandom_u32" },
+    { BPF_PROG_TYPE_XDP,        "bpf_get_smp_processor_id" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_store_bytes" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_l3_csum_replace" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_l4_csum_replace" },
+    { BPF_PROG_TYPE_XDP,        "bpf_tail_call" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_clone_redirect" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_current_pid_tgid" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_current_uid_gid" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_current_comm" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_get_cgroup_classid" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_vlan_push" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_vlan_pop" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_get_tunnel_key" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_set_tunnel_key" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_perf_event_read" },
+    { BPF_PROG_TYPE_XDP,        "bpf_redirect" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_get_route_realm" },
+    { BPF_PROG_TYPE_XDP,        "bpf_perf_event_output" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_load_bytes" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_stackid" },
+    { BPF_PROG_TYPE_XDP,        "bpf_csum_diff" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_get_tunnel_opt" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_set_tunnel_opt" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_change_proto" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_change_type" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_under_cgroup" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_get_hash_recalc" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_current_task" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_probe_write_user" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_current_task_under_cgroup" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_change_tail" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_pull_data" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_csum_update" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_set_hash_invalid" },
+    { BPF_PROG_TYPE_XDP,        "bpf_get_numa_node_id" },
+    { BPF_PROG_TYPE_SK_SKB,        "bpf_skb_change_head" },
+    { BPF_PROG_TYPE_XDP,        "bpf_xdp_adjust_head" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_probe_read_str" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_get_socket_cookie" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_get_socket_uid" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_set_hash" },
+    { BPF_PROG_TYPE_SOCK_OPS,    "bpf_setsockopt" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_adjust_room" },
+    { BPF_PROG_TYPE_XDP,        "bpf_redirect_map" },
+    { BPF_PROG_TYPE_SK_SKB,        "bpf_sk_redirect_map" },
+    { BPF_PROG_TYPE_SOCK_OPS,    "bpf_sock_map_update" },
+    { BPF_PROG_TYPE_XDP,        "bpf_xdp_adjust_meta" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_perf_event_read_value" },
+    { BPF_PROG_TYPE_PERF_EVENT,    "bpf_perf_prog_read_value" },
+    { BPF_PROG_TYPE_SOCK_OPS,    "bpf_getsockopt" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_override_return" },
+    { BPF_PROG_TYPE_SOCK_OPS,    "bpf_sock_ops_cb_flags_set" },
+    { BPF_PROG_TYPE_SK_MSG,        "bpf_msg_redirect_map" },
+    { BPF_PROG_TYPE_SK_MSG,        "bpf_msg_apply_bytes" },
+    { BPF_PROG_TYPE_SK_MSG,        "bpf_msg_cork_bytes" },
+    { BPF_PROG_TYPE_SK_MSG,        "bpf_msg_pull_data" },
+    { BPF_PROG_TYPE_CGROUP_SOCK_ADDR,    "bpf_bind" },
+    { BPF_PROG_TYPE_XDP,        "bpf_xdp_adjust_tail" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_get_xfrm_state" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_stack" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_load_bytes_relative" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_fib_lookup" },
+    { BPF_PROG_TYPE_SOCK_OPS,    "bpf_sock_hash_update" },
+    { BPF_PROG_TYPE_SK_MSG,        "bpf_msg_redirect_hash" },
+    { BPF_PROG_TYPE_SK_SKB,        "bpf_sk_redirect_hash" },
+    { BPF_PROG_TYPE_LWT_IN,        "bpf_lwt_push_encap" },
+    { BPF_PROG_TYPE_LWT_SEG6LOCAL,    "bpf_lwt_seg6_store_bytes" },
+    { BPF_PROG_TYPE_LWT_SEG6LOCAL,    "bpf_lwt_seg6_adjust_srh" },
+    { BPF_PROG_TYPE_LWT_SEG6LOCAL,    "bpf_lwt_seg6_action" },
+    { BPF_PROG_TYPE_LIRC_MODE2,    "bpf_rc_repeat" },
+    { BPF_PROG_TYPE_LIRC_MODE2,    "bpf_rc_keydown" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_cgroup_id" },
+    { BPF_PROG_TYPE_KPROBE,        "bpf_get_current_cgroup_id" },
+    { BPF_PROG_TYPE_CGROUP_SKB,    "bpf_get_local_storage" },
+    { BPF_PROG_TYPE_SK_REUSEPORT,    "bpf_sk_select_reuseport" },
+    { BPF_PROG_TYPE_SCHED_CLS,    "bpf_skb_ancestor_cgroup_id" },
+    { BPF_PROG_TYPE_SK_SKB,        "bpf_sk_lookup_tcp" },
+    { BPF_PROG_TYPE_SK_SKB,        "bpf_sk_lookup_udp" },
+    { BPF_PROG_TYPE_SK_SKB,        "bpf_sk_release" },
+    { BPF_PROG_TYPE_XDP,        "bpf_map_push_elem" },
+    { BPF_PROG_TYPE_XDP,        "bpf_map_pop_elem" },
+    { BPF_PROG_TYPE_XDP,        "bpf_map_peek_elem" },
+    { BPF_PROG_TYPE_SK_MSG,        "bpf_msg_push_data" },
Some of these helpers like bpf_perf_event_output are available on
multiple types; should they all be tested in case the one probed
prog type would not be available on the underlying kernel?

Thanks,
Daniel
That's a good point, but it will make things more complicated. We would
need a list of compatible program types for each helper, and then try
them all until one works. I can do that, but I fear this will get huge
and hard to maintain.
Hmm, true, one way to overcome this maintenance burden would be to try to
brute-force all helpers over all prog types but obviously from verifier
side it's more expensive. Perhaps worth testing whether this overhead would
be acceptable. On the upside you'd have a listing of all supported helpers
for each prog type.
Having the listing sounds nice. My concern is that the compatibility list for a couple of helpers has been modified in the past (e.g. csum_diff added to XDP), and I fear such changes will be easy to miss.

Brute-forcing all programs might be doable (programs are only 2-insn long, no jumps) but seems less clean. I can have a look at it. Is there any particular metric I should focus on?

Naive question, just in case: I don't suppose it is desirable to make the list of supported helpers for each program type accessible to user space? (I guess it would come down to implement those probes as a new bpf() command, which has been rejected before if I remember correctly?)
Yep, because what this basically is testing is verifier behavior in general
where the helper aspect is one specific case of accept / reject, but there can
be more subtle probes on verifier behavior that would go beyond just that like
the ones that Cilium provides where we test verifier for register's map id
marking, etc. Such kind of things are not feasible to expose in uapi in long
term as "capability" but should be probed instead as you do here.

Thanks,
Daniel
Thanks for the clarification.

I've been working on my set to add probing with all supported program
types for each helper. I'm considering printing a list of compatible
program types (as supported by the system) for each helper, like this:

	/*** eBPF helper functions ***/
	...
	#define BPF_SKB_CHANGE_HEAD_HELPER_COMPAT_LIST     ""      \
		"lwt_xmit "     \
		"sk_skb "
	#define BPF_XDP_ADJUST_HEAD_HELPER_COMPAT_LIST     ""      \
		"xdp "
	#define BPF_PROBE_READ_STR_HELPER_COMPAT_LIST      ""      \
		"kprobe "       \
		"tracepoint "   \
		"perf_event "   \
		"raw_tracepoint "
	...

or in JSON:

	{
	    ...
	    "helpers": {
	        ...
		"bpf_skb_change_head_compat_list": ["lwt_xmit","sk_skb"
		],
		"bpf_xdp_adjust_head_compat_list": ["xdp"
		],
		"bpf_probe_read_str_compat_list": \
	            ["kprobe","tracepoint","perf_event","raw_tracepoint"
		],
	    ...
	    }
	}

Would this be acceptable?

Thanks,
Quentin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help