[PATCH bpf-next v2 0/2] bpftool/libbpf: Add probe for large INSN limit

[PATCH bpf-next v2 0/2] bpftool/libbpf: Add probe for large INSN limit

[Michal Rostecki]

This series implements a new BPF feature probe which checks for the
commit c04c0d2b968a ("bpf: increase complexity limit and maximum program
size"), which increases the maximum program size to 1M. It's based on
the similar check in Cilium, although Cilium is already aiming to use
bpftool checks and eventually drop all its custom checks.

Examples of outputs:

# bpftool feature probe
[...]
Scanning miscellaneous eBPF features...
Large complexity limit and maximum program size (1M) is available

# bpftool feature probe macros
[...]
/*** eBPF misc features ***/
#define HAVE_HAVE_LARGE_INSN_LIMIT

# bpftool feature probe -j | jq '.["misc"]'
{
  "have_large_insn_limit": true
}

v1 -> v2:
- Test for 'BPF_MAXINSNS + 1' number of total insns.
- Remove info about the current 1M limit from probe's description.

Michal Rostecki (2):
  libbpf: Add probe for large INSN limit
  bpftool: Add misc secion and probe for large INSN limit

 tools/bpf/bpftool/feature.c   | 18 ++++++++++++++++++
 tools/lib/bpf/libbpf.h        |  1 +
 tools/lib/bpf/libbpf.map      |  1 +
 tools/lib/bpf/libbpf_probes.c | 21 +++++++++++++++++++++
 4 files changed, 41 insertions(+)

-- 
2.16.4

[PATCH bpf-next v2 1/2] libbpf: Add probe for large INSN limit

[Michal Rostecki]

Introduce a new probe which checks whether kernel has large maximum
program size (1M) which was increased in the following commit:

c04c0d2b968a ("bpf: increase complexity limit and maximum program size")

Based on the similar check in Cilium[0], authored by Daniel Borkmann.

[0] https://github.com/cilium/cilium/commit/657d0f585afd26232cfa5d4e70b6f64d2ea91596

Co-authored-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
 tools/lib/bpf/libbpf.h        |  1 +
 tools/lib/bpf/libbpf.map      |  1 +
 tools/lib/bpf/libbpf_probes.c | 21 +++++++++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index fe592ef48f1b..26bf539f1b3c 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -521,6 +521,7 @@ LIBBPF_API bool bpf_probe_prog_type(enum bpf_prog_type prog_type,
 LIBBPF_API bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex);
 LIBBPF_API bool bpf_probe_helper(enum bpf_func_id id,
 				 enum bpf_prog_type prog_type, __u32 ifindex);
+LIBBPF_API bool bpf_probe_large_insn_limit(__u32 ifindex);
 
 /*
  * Get bpf_prog_info in continuous memory
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index e9713a574243..b300d74c921a 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -219,6 +219,7 @@ LIBBPF_0.0.7 {
 		bpf_object__detach_skeleton;
 		bpf_object__load_skeleton;
 		bpf_object__open_skeleton;
+		bpf_probe_large_insn_limit;
 		bpf_prog_attach_xattr;
 		bpf_program__attach;
 		bpf_program__name;
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index a9eb8b322671..221e6ad97012 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -321,3 +321,24 @@ bool bpf_probe_helper(enum bpf_func_id id, enum bpf_prog_type prog_type,
 
 	return res;
 }
+
+/*
+ * Probe for availability of kernel commit (5.3):
+ *
+ * c04c0d2b968a ("bpf: increase complexity limit and maximum program size")
+ */
+bool bpf_probe_large_insn_limit(__u32 ifindex)
+{
+	struct bpf_insn insns[BPF_MAXINSNS + 1];
+	int i;
+
+	for (i = 0; i < BPF_MAXINSNS; i++)
+		insns[i] = BPF_MOV64_IMM(BPF_REG_0, 1);
+	insns[BPF_MAXINSNS] = BPF_EXIT_INSN();
+
+	errno = 0;
+	probe_load(BPF_PROG_TYPE_SCHED_CLS, insns, ARRAY_SIZE(insns), NULL, 0,
+		   ifindex);
+
+	return errno != E2BIG && errno != EINVAL;
+}
-- 
2.16.4

[PATCH bpf-next v2 2/2] bpftool: Add misc secion and probe for large INSN limit

[Michal Rostecki]

Introduce a new probe section (misc) for probes not related to concrete
map types, program types, functions or kernel configuration. Introduce a
probe for large INSN limit as the first one in that section.

Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
---
 tools/bpf/bpftool/feature.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 03bdc5b3ac49..d8ce93092c45 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -572,6 +572,18 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
 		printf("\n");
 }
 
+static void
+probe_large_insn_limit(const char *define_prefix, __u32 ifindex)
+{
+	bool res;
+
+	res = bpf_probe_large_insn_limit(ifindex);
+	print_bool_feature("have_large_insn_limit",
+			   "Large complexity and program size limit",
+			   "HAVE_LARGE_INSN_LIMIT",
+			   res, define_prefix);
+}
+
 static int do_probe(int argc, char **argv)
 {
 	enum probe_component target = COMPONENT_UNSPEC;
@@ -724,6 +736,12 @@ static int do_probe(int argc, char **argv)
 		probe_helpers_for_progtype(i, supported_types[i],
 					   define_prefix, ifindex);
 
+	print_end_then_start_section("misc",
+				     "Scanning miscellaneous eBPF features...",
+				     "/*** eBPF misc features ***/",
+				     define_prefix);
+	probe_large_insn_limit(define_prefix, ifindex);
+
 exit_close_json:
 	if (json_output) {
 		/* End current "section" of probes */
-- 
2.16.4

Re: [PATCH bpf-next v2 2/2] bpftool: Add misc secion and probe for large INSN limit

[Quentin Monnet]

Nit: typo in subject ("secion").

2020-01-07 14:03 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
> Introduce a new probe section (misc) for probes not related to concrete
> map types, program types, functions or kernel configuration. Introduce a
> probe for large INSN limit as the first one in that section.
> 
> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
> ---
>  tools/bpf/bpftool/feature.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index 03bdc5b3ac49..d8ce93092c45 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -572,6 +572,18 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
>  		printf("\n");
>  }
>  
> +static void
> +probe_large_insn_limit(const char *define_prefix, __u32 ifindex)
> +{
> +	bool res;
> +
> +	res = bpf_probe_large_insn_limit(ifindex);
> +	print_bool_feature("have_large_insn_limit",
> +			   "Large complexity and program size limit",

I am not sure we should mention "complexity" here. Although it is
related to program size in the kernel commit you describe, the probe
that is run is only on instruction number. This can make a difference
for offloaded programs: When you probe a device, if kernel has commit
c04c0d2b968a and supports up to 1M instructions, but hardware supports
no more than 4k instructions, you may still benefit from the new value
for BPF_COMPLEXITY_LIMIT_INSNS for complexity, but not for the total
number of available instructions. In that case the probe will fail, and
the message on complexity would not be accurate.

Looks good otherwise, thanks Michal!

Quentin

Re: [PATCH bpf-next v2 2/2] bpftool: Add misc secion and probe for large INSN limit

[Michal Rostecki]

On Tue, Jan 07, 2020 at 02:36:15PM +0000, Quentin Monnet wrote:
> Nit: typo in subject ("secion").
> 
> 2020-01-07 14:03 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
> > Introduce a new probe section (misc) for probes not related to concrete
> > map types, program types, functions or kernel configuration. Introduce a
> > probe for large INSN limit as the first one in that section.
> > 
> > Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
> > ---
> >  tools/bpf/bpftool/feature.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> > index 03bdc5b3ac49..d8ce93092c45 100644
> > --- a/tools/bpf/bpftool/feature.c
> > +++ b/tools/bpf/bpftool/feature.c
> > @@ -572,6 +572,18 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
> >  		printf("\n");
> >  }
> >  
> > +static void
> > +probe_large_insn_limit(const char *define_prefix, __u32 ifindex)
> > +{
> > +	bool res;
> > +
> > +	res = bpf_probe_large_insn_limit(ifindex);
> > +	print_bool_feature("have_large_insn_limit",
> > +			   "Large complexity and program size limit",
> 
> I am not sure we should mention "complexity" here. Although it is
> related to program size in the kernel commit you describe, the probe
> that is run is only on instruction number. This can make a difference
> for offloaded programs: When you probe a device, if kernel has commit
> c04c0d2b968a and supports up to 1M instructions, but hardware supports
> no more than 4k instructions, you may still benefit from the new value
> for BPF_COMPLEXITY_LIMIT_INSNS for complexity, but not for the total
> number of available instructions. In that case the probe will fail, and
> the message on complexity would not be accurate.
> 
> Looks good otherwise, thanks Michal!
> 
> Quentin

Thanks for review! Should I change the description just to "Large
program size limit" or "Large instruction limit"?

Michal

Re: [PATCH bpf-next v2 2/2] bpftool: Add misc secion and probe for large INSN limit

[Quentin Monnet]

2020-01-08 13:41 UTC+0000 ~ Michal Rostecki <mrostecki@opensuse.org>
> On Tue, Jan 07, 2020 at 02:36:15PM +0000, Quentin Monnet wrote:
>> Nit: typo in subject ("secion").
>>
>> 2020-01-07 14:03 UTC+0100 ~ Michal Rostecki <mrostecki@opensuse.org>
>>> Introduce a new probe section (misc) for probes not related to concrete
>>> map types, program types, functions or kernel configuration. Introduce a
>>> probe for large INSN limit as the first one in that section.
>>>
>>> Signed-off-by: Michal Rostecki <mrostecki@opensuse.org>
>>> ---
>>>  tools/bpf/bpftool/feature.c | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
>>> index 03bdc5b3ac49..d8ce93092c45 100644
>>> --- a/tools/bpf/bpftool/feature.c
>>> +++ b/tools/bpf/bpftool/feature.c
>>> @@ -572,6 +572,18 @@ probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type,
>>>  		printf("\n");
>>>  }
>>>  
>>> +static void
>>> +probe_large_insn_limit(const char *define_prefix, __u32 ifindex)
>>> +{
>>> +	bool res;
>>> +
>>> +	res = bpf_probe_large_insn_limit(ifindex);
>>> +	print_bool_feature("have_large_insn_limit",
>>> +			   "Large complexity and program size limit",
>>
>> I am not sure we should mention "complexity" here. Although it is
>> related to program size in the kernel commit you describe, the probe
>> that is run is only on instruction number. This can make a difference
>> for offloaded programs: When you probe a device, if kernel has commit
>> c04c0d2b968a and supports up to 1M instructions, but hardware supports
>> no more than 4k instructions, you may still benefit from the new value
>> for BPF_COMPLEXITY_LIMIT_INSNS for complexity, but not for the total
>> number of available instructions. In that case the probe will fail, and
>> the message on complexity would not be accurate.
>>
>> Looks good otherwise, thanks Michal!
>>
>> Quentin
> 
> Thanks for review! Should I change the description just to "Large
> program size limit" or "Large instruction limit"?
> 
> Michal
> 

I don't really have a preference here, let's keep "program size"?
Quentin