On Sat, Nov 27, 2021 at 1:28 PM Song Liu <song@xxxxxxxxxx> wrote:
>
> On Sat, Nov 27, 2021 at 1:04 PM grantseltzer <grantseltzer@xxxxxxxxx> wrote:
> >
> > From: Grant Seltzer <grantseltzer@xxxxxxxxx>
> >
> > This adds comments above functions in libbpf.h which document
> > their uses. These comments are of a format that doxygen and sphinx
> > can pick up and render. These are rendered by libbpf.readthedocs.org
> >
> > These doc comments are for:
> >
> > - bpf_object__open_file()
> > - bpf_object__open_mem()
> > - bpf_program__attach_uprobe()
> > - bpf_program__attach_uprobe_opts()
> >
> > Signed-off-by: Grant Seltzer <grantseltzer@xxxxxxxxx>
>
> s/libb.h/libbpf.h/ in subject
>
> > ---
> > tools/lib/bpf/libbpf.h | 45 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 45 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 4ec69f224342..acfb207e71d1 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -108,8 +108,26 @@ struct bpf_object_open_opts {
> > #define bpf_object_open_opts__last_field btf_custom_path
> >
> > LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
> > +
> > +/**
> > + * @brief **bpf_object__open_file()** creates a bpf_object by opening
> > + * the BPF object file pointed to by the passed path and loading it
Would be nice to mention that it's ELF, no? "BPF ELF object file", maybe?
> > + * into memory.
> > + * @param path BPF object file relative or absolute path
I started worrying about relative vs absolute paths after reading this
:) I think just stating "BPF object file path" should be totally fine.
Relative vs absolute works totally fine as expected with any API that
accepts file paths.
> > + * @param opts options for how to load the bpf object
let's mention that opts are optional (i.e., you can pass NULL)
> > + * @return pointer to the new bpf_object
>
> Please document return value on errors, i.e. libbpf_err_ptr(err)
> instead of NULL. Same for all functions here.
With libbpf 1.0 and forward APIs like this will return NULL on error,
so let's use that as the convention in documentation. So something
like "NULL is returned on error, error code is stored in errno"?
>
> > + */
> > LIBBPF_API struct bpf_object *
> > bpf_object__open_file(const char *path, const struct bpf_object_open_opts *opts);
> > +
> > +/**
> > + * @brief **bpf_object__open_mem()** creates a bpf_object by reading
> > + * the BPF objects raw bytes from an in memory buffer.
typo: "an in"? Also it's even more important to mention that those
bytes should be a valid BPF ELF object file?
> > + * @param obj_buf pointer to the buffer containing bpf object bytes
s/bpf object bytes/ELF file bytes/ ?
> > + * @param obj_buf_sz number of bytes in the buffer
> > + * @param opts options for how to load the bpf object
> > + * @return pointer to the new bpf_object
> > + */
> > LIBBPF_API struct bpf_object *
> > bpf_object__open_mem(const void *obj_buf, size_t obj_buf_sz,
> > const struct bpf_object_open_opts *opts);
> > @@ -344,10 +362,37 @@ struct bpf_uprobe_opts {
> > };
> > #define bpf_uprobe_opts__last_field retprobe
> >
> > +/**
> > + * @brief **bpf_program__attach_uprobe** attaches a BPF program
missing () after attach_uprobe
> > + * to the userspace function which is found by binary path and
> > + * offset. You can optionally specify a particular proccess to attach
> s/proccess/process/
>
> > + * to. You can also optionally attach the program to the function
> > + * exit instead of entry.
> > + *
> > + * @param prog BPF program to attach
> > + * @param retprobe Attach to function exit
> > + * @param pid Process ID to attach the uprobe to, -1 for all processes
There is also 0 for self (own process).
> > + * @param binary_path Path to binary that contains the function symbol
> > + * @param func_offset Offset within the binary of the function symbol
> > + * @return Reference to the newly created BPF link
or NULL on error (errno is set to error code).
> > + */
> > LIBBPF_API struct bpf_link *
> > bpf_program__attach_uprobe(const struct bpf_program *prog, bool retprobe,
> > pid_t pid, const char *binary_path,
> > size_t func_offset);
> > +
> > +/**
> > + * @brief **bpf_program__attach_uprobe_opts** is just like
() missing
> > + * bpf_program__attach_uprobe except with a options struct
(), let's use that around referenced to functions to make it clear
> > + * for various configurations.
> > + *
> > + * @param prog BPF program to attach
> > + * @param pid Process ID to attach the uprobe to, -1 for all processes
> > + * @param binary_path Path to binary that contains the function symbol
> > + * @param func_offset Offset within the binary of the function symbol
> > + * @param opts Options for altering program attachment
>
> Let's also document details about these options.
yep, but on uprobe_opts struct itself
>
> > + * @return Reference to the newly created BPF link
> > + */
> > LIBBPF_API struct bpf_link *
> > bpf_program__attach_uprobe_opts(const struct bpf_program *prog, pid_t pid,
> > const char *binary_path, size_t func_offset,
> > --
> > 2.31.1
> >
