| Message ID | 20200108014006.938363-1-rdna@fb.com |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] bpf: Document BPF_F_QUERY_EFFECTIVE flag | expand |
> On Jan 7, 2020, at 5:40 PM, Andrey Ignatov <rdna@fb.com> wrote: > > Document BPF_F_QUERY_EFFECTIVE flag, mostly to clarify how it affects > attach_flags what may not be obvious and what may lead to confision. > > Specifically attach_flags is returned only for target_fd but if programs > are inherited from an ancestor cgroup then returned attach_flags for > current cgroup may be confusing. For example, two effective programs of > same attach_type can be returned but w/o BPF_F_ALLOW_MULTI in > attach_flags. > > Simple repro: > # bpftool c s /sys/fs/cgroup/path/to/task > ID AttachType AttachFlags Name > # bpftool c s /sys/fs/cgroup/path/to/task effective > ID AttachType AttachFlags Name > 95043 ingress tw_ipt_ingress > 95048 ingress tw_ingress > > Signed-off-by: Andrey Ignatov <rdna@fb.com> Acked-by: Song Liu <songliubraving@fb.com> With some nit below. > --- > include/uapi/linux/bpf.h | 7 ++++++- > tools/include/uapi/linux/bpf.h | 7 ++++++- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 7df436da542d..dc4b8a2d2a86 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -357,7 +357,12 @@ enum bpf_attach_type { > /* Enable memory-mapping BPF map */ > #define BPF_F_MMAPABLE (1U << 10) > > -/* flags for BPF_PROG_QUERY */ > +/* Flags for BPF_PROG_QUERY. */ > + > +/* Query effective (directly attached + inherited from ancestor cgroups) > + * programs that will be executed for events within a cgroup. > + * attach_flags with this flag are returned only for directly attached programs. This line is more than 75 byte long, I guess ./scripts/checkpatch.pl would complain about it? Thanks, Song
Song Liu <songliubraving@fb.com> [Wed, 2020-01-08 08:31 -0800]: [...] > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 7df436da542d..dc4b8a2d2a86 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -357,7 +357,12 @@ enum bpf_attach_type { > > /* Enable memory-mapping BPF map */ > > #define BPF_F_MMAPABLE (1U << 10) > > > > -/* flags for BPF_PROG_QUERY */ > > +/* Flags for BPF_PROG_QUERY. */ > > + > > +/* Query effective (directly attached + inherited from ancestor cgroups) > > + * programs that will be executed for events within a cgroup. > > + * attach_flags with this flag are returned only for directly attached programs. > > This line is more than 75 byte long, I guess ./scripts/checkpatch.pl would > complain about it? I run checkpatch.pl before sending it but it didn't complain: % scripts/checkpatch.pl p/0001-bpf-Document-BPF_F_QUERY_EFFECTIVE-flag.patch total: 0 errors, 0 warnings, 26 lines checked p/0001-bpf-Document-BPF_F_QUERY_EFFECTIVE-flag.patch has no obvious style problems and is ready for submission. I haven't debugged why, but this header has plenty of lines like this: % awk 'length($0) >= 75' include/uapi/linux/bpf.h | wc -l 74
On Wed, Jan 8, 2020 at 9:20 AM Andrey Ignatov <rdna@fb.com> wrote: > > Song Liu <songliubraving@fb.com> [Wed, 2020-01-08 08:31 -0800]: > [...] > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > > index 7df436da542d..dc4b8a2d2a86 100644 > > > --- a/include/uapi/linux/bpf.h > > > +++ b/include/uapi/linux/bpf.h > > > @@ -357,7 +357,12 @@ enum bpf_attach_type { > > > /* Enable memory-mapping BPF map */ > > > #define BPF_F_MMAPABLE (1U << 10) > > > > > > -/* flags for BPF_PROG_QUERY */ > > > +/* Flags for BPF_PROG_QUERY. */ > > > + > > > +/* Query effective (directly attached + inherited from ancestor cgroups) > > > + * programs that will be executed for events within a cgroup. > > > + * attach_flags with this flag are returned only for directly attached programs. > > > > This line is more than 75 byte long, I guess ./scripts/checkpatch.pl would > > complain about it? > > I run checkpatch.pl before sending it but it didn't complain: > > % scripts/checkpatch.pl p/0001-bpf-Document-BPF_F_QUERY_EFFECTIVE-flag.patch > total: 0 errors, 0 warnings, 26 lines checked > > p/0001-bpf-Document-BPF_F_QUERY_EFFECTIVE-flag.patch has no obvious style problems and is ready for submission. > > I haven't debugged why, but this header has plenty of lines like this: > > % awk 'length($0) >= 75' include/uapi/linux/bpf.h | wc -l > 74 Applied. Thanks
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 7df436da542d..dc4b8a2d2a86 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -357,7 +357,12 @@ enum bpf_attach_type { /* Enable memory-mapping BPF map */ #define BPF_F_MMAPABLE (1U << 10) -/* flags for BPF_PROG_QUERY */ +/* Flags for BPF_PROG_QUERY. */ + +/* Query effective (directly attached + inherited from ancestor cgroups) + * programs that will be executed for events within a cgroup. + * attach_flags with this flag are returned only for directly attached programs. + */ #define BPF_F_QUERY_EFFECTIVE (1U << 0) enum bpf_stack_build_id_status { diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 7df436da542d..dc4b8a2d2a86 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -357,7 +357,12 @@ enum bpf_attach_type { /* Enable memory-mapping BPF map */ #define BPF_F_MMAPABLE (1U << 10) -/* flags for BPF_PROG_QUERY */ +/* Flags for BPF_PROG_QUERY. */ + +/* Query effective (directly attached + inherited from ancestor cgroups) + * programs that will be executed for events within a cgroup. + * attach_flags with this flag are returned only for directly attached programs. + */ #define BPF_F_QUERY_EFFECTIVE (1U << 0) enum bpf_stack_build_id_status {
Document BPF_F_QUERY_EFFECTIVE flag, mostly to clarify how it affects attach_flags what may not be obvious and what may lead to confision. Specifically attach_flags is returned only for target_fd but if programs are inherited from an ancestor cgroup then returned attach_flags for current cgroup may be confusing. For example, two effective programs of same attach_type can be returned but w/o BPF_F_ALLOW_MULTI in attach_flags. Simple repro: # bpftool c s /sys/fs/cgroup/path/to/task ID AttachType AttachFlags Name # bpftool c s /sys/fs/cgroup/path/to/task effective ID AttachType AttachFlags Name 95043 ingress tw_ipt_ingress 95048 ingress tw_ingress Signed-off-by: Andrey Ignatov <rdna@fb.com> --- include/uapi/linux/bpf.h | 7 ++++++- tools/include/uapi/linux/bpf.h | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-)