Re: [PATCH v22 bpf-next 18/23] libbpf: Add SEC name for xdp multi-frags programs

On Tue, Jan 18, 2022 at 2:31 PM Lorenzo Bianconi [off-list ref] wrote:

quoted

On Mon, Jan 17, 2022 at 06:28:30PM +0100, Lorenzo Bianconi wrote:

quoted

Introduce support for the following SEC entries for XDP multi-frags
property:
- SEC("xdp.frags")
- SEC("xdp.frags/devmap")
- SEC("xdp.frags/cpumap")

Acked-by: Toke Hoiland-Jorgensen <redacted>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 tools/lib/bpf/libbpf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fdb3536afa7d..611e81357fb6 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c

@@ -6562,6 +6562,9 @@ static int libbpf_preload_prog(struct bpf_program *prog,
    if (def & SEC_SLEEPABLE)
            opts->prog_flags |= BPF_F_SLEEPABLE;

+   if (prog->type == BPF_PROG_TYPE_XDP && strstr(prog->sec_name, ".frags"))
+           opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;

That's a bit sloppy.
Could you handle it similar to SEC_SLEEPABLE?

quoted

+
    if ((prog->type == BPF_PROG_TYPE_TRACING ||
         prog->type == BPF_PROG_TYPE_LSM ||
         prog->type == BPF_PROG_TYPE_EXT) && !prog->attach_btf_id) {

@@ -8600,8 +8603,11 @@ static const struct bpf_sec_def section_defs[] = {
    SEC_DEF("lsm.s/",               LSM, BPF_LSM_MAC, SEC_ATTACH_BTF | SEC_SLEEPABLE, attach_lsm),
    SEC_DEF("iter/",                TRACING, BPF_TRACE_ITER, SEC_ATTACH_BTF, attach_iter),
    SEC_DEF("syscall",              SYSCALL, 0, SEC_SLEEPABLE),
+   SEC_DEF("xdp.frags/devmap",     XDP, BPF_XDP_DEVMAP, SEC_NONE),
    SEC_DEF("xdp_devmap/",          XDP, BPF_XDP_DEVMAP, SEC_ATTACHABLE),
+   SEC_DEF("xdp.frags/cpumap",     XDP, BPF_XDP_CPUMAP, SEC_NONE),
    SEC_DEF("xdp_cpumap/",          XDP, BPF_XDP_CPUMAP, SEC_ATTACHABLE),
+   SEC_DEF("xdp.frags",            XDP, BPF_XDP, SEC_NONE),

It would be SEC_FRAGS here instead of SEC_NONE.

ack, I dropped SEC_FRAGS (SEC_XDP_MB before) from sec_def_flags because Andrii asked to remove
it but I am fine to add it back. Agree?

Andrii,
what was the motivation?
imo that's cleaner than strstr.