| Message ID | a47ee7676254d3e94d3ff61afe20477eb8ace561.1576720240.git.rdna@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: Support replacing cgroup-bpf program in MULTI mode | expand |
On Wed, Dec 18, 2019 at 6:56 PM Andrey Ignatov <rdna@fb.com> wrote: > > Introduce a new bpf_prog_attach_xattr function that, in addition to > program fd, target fd and attach type, accepts an extendable struct > bpf_prog_attach_opts. > > bpf_prog_attach_opts relies on DECLARE_LIBBPF_OPTS macro to maintain > backward and forward compatibility and has the following "optional" > attach attributes: > > * existing attach_flags, since it's not required when attaching in NONE > mode. Even though it's quite often used in MULTI and OVERRIDE mode it > seems to be a good idea to reduce number of arguments to > bpf_prog_attach_xattr; > > * newly introduced attribute of BPF_PROG_ATTACH command: replace_prog_fd > that is fd of previously attached cgroup-bpf program to replace if > BPF_F_REPLACE flag is used. > > The new function is named to be consistent with other xattr-functions > (bpf_prog_test_run_xattr, bpf_create_map_xattr, bpf_load_program_xattr). > > The struct bpf_prog_attach_opts is supposed to be used with > DECLARE_LIBBPF_OPTS macro. > > Signed-off-by: Andrey Ignatov <rdna@fb.com> > --- > tools/lib/bpf/bpf.c | 16 +++++++++++++++- > tools/lib/bpf/bpf.h | 11 +++++++++++ > tools/lib/bpf/libbpf.map | 1 + > 3 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > index 98596e15390f..ebb4f8d71bdb 100644 > --- a/tools/lib/bpf/bpf.c > +++ b/tools/lib/bpf/bpf.c > @@ -466,6 +466,17 @@ int bpf_obj_get(const char *pathname) > > int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type, > unsigned int flags) > +{ > + DECLARE_LIBBPF_OPTS(bpf_prog_attach_opts, opts, > + .flags = flags, > + ); > + > + return bpf_prog_attach_xattr(prog_fd, target_fd, type, &opts); > +} > + > +int bpf_prog_attach_xattr(int prog_fd, int target_fd, > + enum bpf_attach_type type, > + const struct bpf_prog_attach_opts *opts) > { > union bpf_attr attr; > You need to validate opts with OPTS_VALID macro (see btf_dump__emit_type_decl() for simple example). > @@ -473,7 +484,10 @@ int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type, > attr.target_fd = target_fd; > attr.attach_bpf_fd = prog_fd; > attr.attach_type = type; > - attr.attach_flags = flags; > + if (opts) { > + attr.attach_flags = opts->flags; > + attr.replace_bpf_fd = opts->replace_prog_fd; > + } Please use OPTS_GET() macro to fetch values from opts struct and provide default value if they are not specified. > > return sys_bpf(BPF_PROG_ATTACH, &attr, sizeof(attr)); > } [...]
Andrii Nakryiko <andrii.nakryiko@gmail.com> [Wed, 2019-12-18 21:50 -0800]: > On Wed, Dec 18, 2019 at 6:56 PM Andrey Ignatov <rdna@fb.com> wrote: > > > > Introduce a new bpf_prog_attach_xattr function that, in addition to > > program fd, target fd and attach type, accepts an extendable struct > > bpf_prog_attach_opts. > > > > bpf_prog_attach_opts relies on DECLARE_LIBBPF_OPTS macro to maintain > > backward and forward compatibility and has the following "optional" > > attach attributes: > > > > * existing attach_flags, since it's not required when attaching in NONE > > mode. Even though it's quite often used in MULTI and OVERRIDE mode it > > seems to be a good idea to reduce number of arguments to > > bpf_prog_attach_xattr; > > > > * newly introduced attribute of BPF_PROG_ATTACH command: replace_prog_fd > > that is fd of previously attached cgroup-bpf program to replace if > > BPF_F_REPLACE flag is used. > > > > The new function is named to be consistent with other xattr-functions > > (bpf_prog_test_run_xattr, bpf_create_map_xattr, bpf_load_program_xattr). > > > > The struct bpf_prog_attach_opts is supposed to be used with > > DECLARE_LIBBPF_OPTS macro. > > > > Signed-off-by: Andrey Ignatov <rdna@fb.com> > > --- > > tools/lib/bpf/bpf.c | 16 +++++++++++++++- > > tools/lib/bpf/bpf.h | 11 +++++++++++ > > tools/lib/bpf/libbpf.map | 1 + > > 3 files changed, 27 insertions(+), 1 deletion(-) > > > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > > index 98596e15390f..ebb4f8d71bdb 100644 > > --- a/tools/lib/bpf/bpf.c > > +++ b/tools/lib/bpf/bpf.c > > @@ -466,6 +466,17 @@ int bpf_obj_get(const char *pathname) > > > > int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type, > > unsigned int flags) > > +{ > > + DECLARE_LIBBPF_OPTS(bpf_prog_attach_opts, opts, > > + .flags = flags, > > + ); > > + > > + return bpf_prog_attach_xattr(prog_fd, target_fd, type, &opts); > > +} > > + > > +int bpf_prog_attach_xattr(int prog_fd, int target_fd, > > + enum bpf_attach_type type, > > + const struct bpf_prog_attach_opts *opts) > > { > > union bpf_attr attr; > > > > You need to validate opts with OPTS_VALID macro (see > btf_dump__emit_type_decl() for simple example). > > > @@ -473,7 +484,10 @@ int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type, > > attr.target_fd = target_fd; > > attr.attach_bpf_fd = prog_fd; > > attr.attach_type = type; > > - attr.attach_flags = flags; > > + if (opts) { > > + attr.attach_flags = opts->flags; > > + attr.replace_bpf_fd = opts->replace_prog_fd; > > + } > > Please use OPTS_GET() macro to fetch values from opts struct and > provide default value if they are not specified. OK, will do both OPTS_VALID and OPTS_GET in v4. > > > > > > return sys_bpf(BPF_PROG_ATTACH, &attr, sizeof(attr)); > > } > > [...]
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c index 98596e15390f..ebb4f8d71bdb 100644 --- a/tools/lib/bpf/bpf.c +++ b/tools/lib/bpf/bpf.c @@ -466,6 +466,17 @@ int bpf_obj_get(const char *pathname) int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type, unsigned int flags) +{ + DECLARE_LIBBPF_OPTS(bpf_prog_attach_opts, opts, + .flags = flags, + ); + + return bpf_prog_attach_xattr(prog_fd, target_fd, type, &opts); +} + +int bpf_prog_attach_xattr(int prog_fd, int target_fd, + enum bpf_attach_type type, + const struct bpf_prog_attach_opts *opts) { union bpf_attr attr; @@ -473,7 +484,10 @@ int bpf_prog_attach(int prog_fd, int target_fd, enum bpf_attach_type type, attr.target_fd = target_fd; attr.attach_bpf_fd = prog_fd; attr.attach_type = type; - attr.attach_flags = flags; + if (opts) { + attr.attach_flags = opts->flags; + attr.replace_bpf_fd = opts->replace_prog_fd; + } return sys_bpf(BPF_PROG_ATTACH, &attr, sizeof(attr)); } diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h index 269807ce9ef5..f0ab8519986e 100644 --- a/tools/lib/bpf/bpf.h +++ b/tools/lib/bpf/bpf.h @@ -126,8 +126,19 @@ LIBBPF_API int bpf_map_get_next_key(int fd, const void *key, void *next_key); LIBBPF_API int bpf_map_freeze(int fd); LIBBPF_API int bpf_obj_pin(int fd, const char *pathname); LIBBPF_API int bpf_obj_get(const char *pathname); + +struct bpf_prog_attach_opts { + size_t sz; /* size of this struct for forward/backward compatibility */ + unsigned int flags; + int replace_prog_fd; +}; +#define bpf_prog_attach_opts__last_field replace_prog_fd + LIBBPF_API int bpf_prog_attach(int prog_fd, int attachable_fd, enum bpf_attach_type type, unsigned int flags); +LIBBPF_API int bpf_prog_attach_xattr(int prog_fd, int attachable_fd, + enum bpf_attach_type type, + const struct bpf_prog_attach_opts *opts); LIBBPF_API int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type); LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd, enum bpf_attach_type type); diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index e3a471f38a71..e9713a574243 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -219,6 +219,7 @@ LIBBPF_0.0.7 { bpf_object__detach_skeleton; bpf_object__load_skeleton; bpf_object__open_skeleton; + bpf_prog_attach_xattr; bpf_program__attach; bpf_program__name; btf__align_of;
Introduce a new bpf_prog_attach_xattr function that, in addition to program fd, target fd and attach type, accepts an extendable struct bpf_prog_attach_opts. bpf_prog_attach_opts relies on DECLARE_LIBBPF_OPTS macro to maintain backward and forward compatibility and has the following "optional" attach attributes: * existing attach_flags, since it's not required when attaching in NONE mode. Even though it's quite often used in MULTI and OVERRIDE mode it seems to be a good idea to reduce number of arguments to bpf_prog_attach_xattr; * newly introduced attribute of BPF_PROG_ATTACH command: replace_prog_fd that is fd of previously attached cgroup-bpf program to replace if BPF_F_REPLACE flag is used. The new function is named to be consistent with other xattr-functions (bpf_prog_test_run_xattr, bpf_create_map_xattr, bpf_load_program_xattr). The struct bpf_prog_attach_opts is supposed to be used with DECLARE_LIBBPF_OPTS macro. Signed-off-by: Andrey Ignatov <rdna@fb.com> --- tools/lib/bpf/bpf.c | 16 +++++++++++++++- tools/lib/bpf/bpf.h | 11 +++++++++++ tools/lib/bpf/libbpf.map | 1 + 3 files changed, 27 insertions(+), 1 deletion(-)