Add function SetKprobeForSection to change section name in module

[shang-wang]

Currently gobpf uses the name defined in SEC() header of each function to bind to the right eBPF API. This creates some inflexibility when we need two implementations of a certain API. For example:

In kernels later than v4.1.0, tcp_sendmsg is defined as

int tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)

In kernels earlier than v4.1.0, the definition is

int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg, size_t size)

With the change in this pull request we could override the names in any mappings that are stored in module. This means that we can have a custom name in SEC() header but still map the implementation to an API that we want. For instance, SEC("kprobe/tcp_sendmsg_v2") can map to kprobe/tcp_sendmsg if we call SetKprobeForSection("tcp_sendmsg_v2", "tcp_sendmsg").

When we enable the kprobe later on, just do

m.EnableKprobe("kprobe/tcp_sendmsg_v2", maxActive)

and underlying probe would bind to tcp_sendmsg API correctly.

[p-lambert]

I think what we want to test is the side effect of this, not the Name field.

So basically we want to have a Kprobe handler under an arbitrary name SEC("foobar"), attach that handler to Kprobe by doing something like

m.UpdateKprobeSecName("foobar,"kprobe/whatever")
m.EnableKprobe("foobar")

and finally verify that your handler was executed when whatever ran.

[shang-wang]

I was thinking about the same thing, but the direct result of calling EnableKprobe() is not easy to check(requires mount of debug, etc), that's why the overall tests in elf is lacking.

[leeavital]

How about adding a new function EnableKProbeForFunction(secName string, funcName string, isRetProbe bool, string maxactive) that doesn't tie the function to the name of the entry in b.probes ? You could implement EnableKProbe in terms of EnableKProbeForFunction.

[shang-wang]

@leeavital there are other ways that EnableKprobe is used(https://github.com/DataDog/gobpf/blob/master/elf/module.go#L373), if you don't change the actual kprobe name, you might ended up with surprises for the inconsistencies. Does that make sense?

[leeavital]

Won't EnableKrpobes break anyway? If there is a kprobe named kprobe/udp_sendmsg/rhel, the it will try to attach that kprobe to a function named udp_sendmsg/rhel which won't exist.

[shang-wang]

It won't break if you call "update" first, but it's hard to call both EnableKprobe and EnableKProbeForFunction in a for loop in EnableKprobes if you want to enable certain kprobe differently.

[p-lambert]

Looking at your other PR I'm starting to think that a cleaner way to do this would be not changing the API of the package at all and establishing another convention like:

SEC("kprobe/tcp_sendmsg/arbitrary_sufix")

which would generate a handler that will get attached kprobe/tcp_sendmsg. We would simply dismiss everything after the second /.

[p-lambert]

Left a couple of nits, but I think this is fine to merge.

[leeavital]

we should make it explicit in the function name and documentation that handler is a section name.

[shang-wang]

UpdateKprobeNameForSecHandler? It cannot be any longer 🤣

[leeavital]

How about SetKprobeForSection ?

[shang-wang]

OK that could work. Although given that they mix all terms together, nothing seems to be able remove the confusion completely.