bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Ignatov <rdna@fb.com>
To: <bpf@vger.kernel.org>
Cc: Andrey Ignatov <rdna@fb.com>, <ast@kernel.org>,
	<daniel@iogearbox.net>, <kernel-team@fb.com>
Subject: [PATCH bpf 1/2] libbpf: Fix loading cgroup_skb/egress with ret in [2, 3]
Date: Fri, 10 Apr 2020 12:54:00 -0700	[thread overview]
Message-ID: <daa546903aa74cf844452b7f80788d67c15b42ea.1586547735.git.rdna@fb.com> (raw)
In-Reply-To: <cover.1586547735.git.rdna@fb.com>

Initially BPF_CGROUP_INET_EGRESS hook didn't require specifying
expected_attach_type at loading time, but commit

  5cf1e9145630 ("bpf: cgroup inet skb programs can return 0 to 3")

changed it so that expected_attach_type must be specified if program can
return either 2 or 3 (before it was either 0 or 1) to communicate
congestion notification to caller.

At the same time loading w/o expected_attach_type is still supported for
backward compatibility if program retval is in tnum_range(0, 1).

Though libbpf currently supports guessing prog/attach/expected_attach
types only for "old" mode (retval in [0; 1]). And if cgroup_skb egress
program stars returning e.g. 2 (corresponds to NET_XMIT_CN), then
guessing breaks and, e.g. bpftool can't load an object with such a
program anymore:

  # bpftool prog loadall tools/testing/selftests/bpf/test_skb.o /sys/fs/bpf/test_skb
  libbpf: load bpf program failed: Invalid argument
  libbpf: -- BEGIN DUMP LOG ---
  libbpf:
  ; return tc_prog(skb) == TC_ACT_OK ? 1 : 2 /* NET_XMIT_CN */;
  0: (85) call pc+5

   ... skip ...

  from 87 to 1: R0_w=invP2 R10=fp0
  ; return tc_prog(skb) == TC_ACT_OK ? 1 : 2 /* NET_XMIT_CN */;
  1: (bc) w1 = w0
  2: (b4) w0 = 1
  3: (16) if w1 == 0x0 goto pc+1
  4: (b4) w0 = 2
  ; return tc_prog(skb) == TC_ACT_OK ? 1 : 2 /* NET_XMIT_CN */;
  5: (95) exit
  At program exit the register R0 has value (0x2; 0x0) should have been in (0x0; 0x1)
  processed 96 insns (limit 1000000) max_states_per_insn 1 total_states 10 peak_states 10 mark_read 2

  libbpf: -- END LOG --
  libbpf: failed to load program 'cgroup_skb/egress'
  libbpf: failed to load object 'tools/testing/selftests/bpf/test_skb.o'
  Error: failed to load object file

Fix it by introducing another entry in libbpf section_defs that makes the load
happens with expected_attach_type: cgroup_skb/egress/expected

That name may not be ideal, but I don't have a better option.

Strictly speaking this is not a fix but rather a missing feature, that's
why there is no Fixes tag. But it still seems to be a good idea to merge
it to stable tree to fix loading programs that use a feature available
for almost a year.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/lib/bpf/libbpf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ff9174282a8c..c909352f894d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6330,6 +6330,8 @@ static const struct bpf_sec_def section_defs[] = {
 	BPF_PROG_SEC("lwt_seg6local",		BPF_PROG_TYPE_LWT_SEG6LOCAL),
 	BPF_APROG_SEC("cgroup_skb/ingress",	BPF_PROG_TYPE_CGROUP_SKB,
 						BPF_CGROUP_INET_INGRESS),
+	BPF_EAPROG_SEC("cgroup_skb/egress/expected", BPF_PROG_TYPE_CGROUP_SKB,
+						BPF_CGROUP_INET_EGRESS),
 	BPF_APROG_SEC("cgroup_skb/egress",	BPF_PROG_TYPE_CGROUP_SKB,
 						BPF_CGROUP_INET_EGRESS),
 	BPF_APROG_COMPAT("cgroup/skb",		BPF_PROG_TYPE_CGROUP_SKB),
-- 
2.24.1


  reply	other threads:[~2020-04-10 19:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-10 19:53 [PATCH bpf 0/2] libbpf: Fix loading cgroup_skb/egress with ret in [2, 3] Andrey Ignatov
2020-04-10 19:54 ` Andrey Ignatov [this message]
2020-04-10 20:39   ` [PATCH bpf 1/2] " Andrii Nakryiko
2020-04-10 21:14     ` Alexei Starovoitov
2020-04-10 21:52       ` Andrii Nakryiko
2020-04-10 23:02         ` Andrey Ignatov
2020-04-10 22:57     ` Andrey Ignatov
2020-04-11  0:09       ` Andrii Nakryiko
2020-04-12  6:01         ` Andrii Nakryiko
2020-04-13 20:24           ` Andrey Ignatov
2020-04-11 22:42       ` Alexei Starovoitov
2020-04-12  0:36         ` Andrey Ignatov
2020-04-10 19:54 ` [PATCH bpf 2/2] selftests/bpf: Test cgroup_skb/egress/expected section name Andrey Ignatov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=daa546903aa74cf844452b7f80788d67c15b42ea.1586547735.git.rdna@fb.com \
    --to=rdna@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).