link: use BPF links to attach Tracing and LSM prog types

[mmat11]

Fallback to sys.RawTracepointOpen() when not possible.

I've exported the Program's AttachType since it's needed when creating the link, the alternative would be to have separate functions for the different attach types, eg. AttachFEntry, AttachFExit, etc. but this would be a breaking change.

[mmat11]

ping @lmb this can be reviewed

[lmb]

Hey, I'm currently onboarding at Isovalent, so this might take me a while to get to. How urgent is this?

[mmat11]

Hey, I'm currently onboarding at Isovalent, so this might take me a while to get to. How urgent is this?

not urgent at all, take your time!

[ti-mo]

What is the reason the default attach type can't use bpf_link?

[mmat11]

AttachType needs to be specified in LinkCreateTracingAttr

[lmb]

https://elixir.bootlin.com/linux/v6.2.2/source/kernel/bpf/syscall.c#L4577

		ret = bpf_tracing_prog_attach(prog,
					      attr->link_create.target_fd,
					      attr->link_create.target_btf_id,
					      attr->link_create.tracing.cookie);

https://elixir.bootlin.com/linux/v6.2.2/source/kernel/bpf/syscall.c#L4595

			ret = bpf_tracing_prog_attach(prog,
						      attr->link_create.target_fd,
						      attr->link_create.target_btf_id,
						      attr->link_create.tracing.cookie);

https://elixir.bootlin.com/linux/v6.2.2/source/kernel/bpf/syscall.c#L3310

		return bpf_tracing_prog_attach(prog, 0, 0, 0);

• Both RAW_TRACEPOINT_OPEN and BPF_LINK_CREATE return a link fd (on newer kernels)
• But RAW_TRACEPOINT_OPEN doesn't allow specifying target or cookie

On new kernels, RAW_TRACEPOINT_OPEN == BPF_LINK_CREATE if target_fd, cookie, etc. are 0.

Thinking through the compatibility story here: seems like specifying AttachType on a kernel without LINK_CREATE will fail. So it's not possible to just always set AttachType if you want maximum compat, instead you should only set it if Cookie != 0. Can we do better?

[mmat11]

So, to recap:

• on new kernels it's always better to use LINK_CREATE as it behaves the same way if cookie etc. are 0
• on old kernels we can fail and fallback (fallthrough) to RAW_TRACEPOINT_OPEN

seems like specifying AttachType on a kernel without LINK_CREATE will fail

this should be handled by fallthrough, right?

So it's not possible to just always set AttachType if you want maximum compat, instead you should only set it if Cookie != 0

could you clarify this point? If I set opts.AttachType on a kernel that doesn't support LINK_CREATE, I expect it to fail and fallback to RAW_TRACEPOINT_OPEN

[lmb]

this should be handled by fallthrough, right?

yes, my bad, sorry!

• on new kernels it's always better to use LINK_CREATE as it behaves the same way if cookie etc. are 0

Nit: seems to me like LINK_CREATE vs RAW_TRACEPOINT_OPEN makes no difference if cookie is 0. They end up calling the same function?

Agreed that we should prefer LINK_CREATE.

[lmb]

Sorry it took me so long to review, please feel free to ping me earlier in the future!

[lmb]

https://elixir.bootlin.com/linux/v6.2.2/source/kernel/bpf/syscall.c#L4577

		ret = bpf_tracing_prog_attach(prog,
					      attr->link_create.target_fd,
					      attr->link_create.target_btf_id,
					      attr->link_create.tracing.cookie);

https://elixir.bootlin.com/linux/v6.2.2/source/kernel/bpf/syscall.c#L4595

			ret = bpf_tracing_prog_attach(prog,
						      attr->link_create.target_fd,
						      attr->link_create.target_btf_id,
						      attr->link_create.tracing.cookie);

https://elixir.bootlin.com/linux/v6.2.2/source/kernel/bpf/syscall.c#L3310

		return bpf_tracing_prog_attach(prog, 0, 0, 0);

• Both RAW_TRACEPOINT_OPEN and BPF_LINK_CREATE return a link fd (on newer kernels)
• But RAW_TRACEPOINT_OPEN doesn't allow specifying target or cookie

On new kernels, RAW_TRACEPOINT_OPEN == BPF_LINK_CREATE if target_fd, cookie, etc. are 0.

Thinking through the compatibility story here: seems like specifying AttachType on a kernel without LINK_CREATE will fail. So it's not possible to just always set AttachType if you want maximum compat, instead you should only set it if Cookie != 0. Can we do better?

[lmb]

LGTM! @ti-mo want to take another look or can I merge?

[lmb]

Timo is out, so going ahead. Thanks for your contribution as always! 🎉

[ti-mo]

Thanks!