bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers
Subsystems (7)
cover bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers
Standardize commonly used bpf enum names by introducing helper methods to libbpf.
When applications require enum to string mapping the related code is copy-pasted from bpftool. It hardens maintenance, e.g. when new enum values are added.
Patches 0001-0003 introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum type.
Patches 0004-0006 introduce helpers methods libbpf_str_from_(prog|map|attach)_type and libbpf_(prog|map|attach)_type_from_str.
Patches 0007-0008 extend and rename test_section_names test. Patches 0009-0010 introduce the helpers to bpftool.
An alternative for adding __MAX_BPF_(PROG|MAP)_TYPE is using an erroneous result of bpf_(prog|map|attach)_type_(from|to)_str as an indicator of loop bound. The disadvantages are: tests won't fail when a string name is not provided for a new enum value; whoever wants to loop over enum values should be aware about this side feature of newly introduced helpers.
Julia Kartseva (10):
bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values tools/bpf: sync bpf.h to tools/
tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements tools/bpf: add libbpf_prog_type_(from|to)_str helpers tools/bpf: add libbpf_map_type_(from|to)_str helpers tools/bpf: add libbpf_attach_type_(from|to)_str selftests/bpf: extend test_section_names with type_(from|to)_str selftests/bpf: rename test_section_names to
test_section_and_type_names
tools/bpftool: use libbpf_(prog|map)_type_to_str helpers tools/bpftool: use libbpf_attach_type_to_str helper
include/uapi/linux/bpf.h | 6 + tools/bpf/bpftool/cgroup.c | 60 +-- tools/bpf/bpftool/feature.c | 47 ++- tools/bpf/bpftool/main.h | 33 -- tools/bpf/bpftool/map.c | 80 +--- tools/bpf/bpftool/prog.c | 31 +- tools/include/uapi/linux/bpf.h | 6 + tools/lib/bpf/libbpf.c | 153 +++++++ tools/lib/bpf/libbpf.h | 17 + tools/lib/bpf/libbpf.map | 6 + tools/lib/bpf/libbpf_probes.c | 2 + tools/testing/selftests/bpf/Makefile | 2 +- .../bpf/test_section_and_type_names.c | 378 ++++++++++++++++++ .../selftests/bpf/test_section_names.c | 233 ----------- 14 files changed, 673 insertions(+), 381 deletions(-) create mode 100644 tools/testing/selftests/bpf/test_section_and_type_names.c delete mode 100644 tools/testing/selftests/bpf/test_section_names.c
Signature · 1 line
2.17.1
PATCH 1/10 bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values
Similar to __MAX_BPF_ATTACH_TYPE identifying the number of elements in bpf_attach_type enum, add tailing enum values __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE to simplify e.g. iteration over enums values in the case when new values are added.
--- include/uapi/linux/bpf.h | 6 ++++++ 1 file changed, 6 insertions(+)
include/uapi/linux/bpf.h +6 -0
@@ -136,8 +136,11 @@ enum bpf_map_type {BPF_MAP_TYPE_STACK ,BPF_MAP_TYPE_SK_STORAGE ,BPF_MAP_TYPE_DEVMAP_HASH , + __MAX_BPF_MAP_TYPE }; +#define MAX_BPF_MAP_TYPE __MAX_BPF_MAP_TYPE +/* Note that tracing related programs such as * BPF_PROG_TYPE_{KPROBE,TRACEPOINT,PERF_EVENT,RAW_TRACEPOINT} * are not subject to a stable API since kernel internal data
@@ -173,8 +176,11 @@ enum bpf_prog_type {BPF_PROG_TYPE_CGROUP_SYSCTL ,BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE ,BPF_PROG_TYPE_CGROUP_SOCKOPT , + __MAX_BPF_PROG_TYPE }; +#define MAX_BPF_PROG_TYPE __MAX_BPF_PROG_TYPE +enum bpf_attach_type {BPF_CGROUP_INET_INGRESS , BPF_CGROUP_INET_EGRESS,
Signature · 1 line
2.17.1
Re: [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values
Quoted 27 lines · Julia Kartseva
Similar to
__MAX_BPF_ATTACH_TYPEidentifying the number of elements inbpf_attach_typeenum, add tailing enum values__MAX_BPF_PROG_TYPEand__MAX_BPF_MAP_TYPEto simplify e.g. iteration over enums values in the case when new values are added.Signed-off-by Julia Kartseva <[email protected]>--- include/uapi/linux/bpf.h | 6 ++++++ 1 file changed, 6 insertions(+)include/uapi/linux/bpf.h +3
-0@@ -136,8 +136,11 @@ enum bpf_map_type {BPF_MAP_TYPE_STACK ,BPF_MAP_TYPE_SK_STORAGE ,BPF_MAP_TYPE_DEVMAP_HASH , + __MAX_BPF_MAP_TYPE }; +#define MAX_BPF_MAP_TYPE __MAX_BPF_MAP_TYPE +/* Note that tracing related programs such as * BPF_PROG_TYPE_{KPROBE,TRACEPOINT,PERF_EVENT,RAW_TRACEPOINT} * are not subject to a stable API since kernel internal data
include/uapi/linux/bpf.h enum bpf_prog_type { · line 183
Quoted 8 lines
include/uapi/linux/bpf.h +3
-0@@ -173,8 +176,11 @@ enum bpf_prog_type {BPF_PROG_TYPE_CGROUP_SYSCTL ,BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE ,BPF_PROG_TYPE_CGROUP_SOCKOPT , + __MAX_BPF_PROG_TYPE }; +#define MAX_BPF_PROG_TYPE __MAX_BPF_PROG_TYPE +
This came up before and my position is still the same. I'm against this type of band-aid in uapi. 'bpftool feature probe' can easily discover all supported prog and map types already.
PATCH 2/10 tools/bpf: sync bpf.h to tools/
In reply to Julia KartsevaIntroduce __MAX_BPF_MAP_TYPE and __MAX_BPF_MAP_TYPE enum values.
--- tools/include/uapi/linux/bpf.h | 6 ++++++ 1 file changed, 6 insertions(+)
tools/include/uapi/linux/bpf.h +6 -0
@@ -136,8 +136,11 @@ enum bpf_map_type {BPF_MAP_TYPE_STACK ,BPF_MAP_TYPE_SK_STORAGE ,BPF_MAP_TYPE_DEVMAP_HASH , + __MAX_BPF_MAP_TYPE }; +#define MAX_BPF_MAP_TYPE __MAX_BPF_MAP_TYPE +/* Note that tracing related programs such as * BPF_PROG_TYPE_{KPROBE,TRACEPOINT,PERF_EVENT,RAW_TRACEPOINT} * are not subject to a stable API since kernel internal data
@@ -173,8 +176,11 @@ enum bpf_prog_type {BPF_PROG_TYPE_CGROUP_SYSCTL ,BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE ,BPF_PROG_TYPE_CGROUP_SOCKOPT , + __MAX_BPF_PROG_TYPE }; +#define MAX_BPF_PROG_TYPE __MAX_BPF_PROG_TYPE +enum bpf_attach_type {BPF_CGROUP_INET_INGRESS , BPF_CGROUP_INET_EGRESS,
Signature · 1 line
2.17.1
PATCH 3/10 tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements
In reply to Julia KartsevaAdd cases to switch statements in probe_load, bpf_prog_type__needs_kver bpf_probe_map_type to fix enumeration value not handled in switch compilation error.prog_type_name array in bpftool/main.h doesn't have __MAX_BPF_PROG_TYPE entity, same for map, so probe won't be called.
--- tools/lib/bpf/libbpf.c | 1 + tools/lib/bpf/libbpf_probes.c | 2 ++ 2 files changed, 3 insertions(+)
tools/lib/bpf/libbpf.c +1 -0
@@ -3580,6 +3580,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)case BPF_PROG_TYPE_PERF_EVENT :case BPF_PROG_TYPE_CGROUP_SYSCTL :case BPF_PROG_TYPE_CGROUP_SOCKOPT : +case __MAX_BPF_PROG_TYPE:return false;case BPF_PROG_TYPE_KPROBE :default :
tools/lib/bpf/libbpf_probes.c +2 -0
@@ -102,6 +102,7 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,case BPF_PROG_TYPE_FLOW_DISSECTOR :case BPF_PROG_TYPE_CGROUP_SYSCTL :case BPF_PROG_TYPE_CGROUP_SOCKOPT : +case __MAX_BPF_PROG_TYPE:default :break ; }
@@ -250,6 +251,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)case BPF_MAP_TYPE_XSKMAP :case BPF_MAP_TYPE_SOCKHASH :case BPF_MAP_TYPE_REUSEPORT_SOCKARRAY : +case __MAX_BPF_MAP_TYPE:default :break ; }
Signature · 1 line
2.17.1
Re: [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements
Em Wed, Aug 28, 2019 at 02:03:06PM -0700, Julia Kartseva escreveu:
Quoted 5 lines
Add cases to switch statements in
probe_load,bpf_prog_type__needs_kverbpf_probe_map_typeto fix enumeration value not handled in switch compilation error.prog_type_namearray inbpftool/main.hdoesn't have__MAX_BPF_PROG_TYPEentity, same for map, so probe won't be called.
Shouldn't this be added when adding that __MAX_BPF_PROG_TYPE value to the enum? Otherwise the build will fail when __MAX_BPF_PROG_TYPE is added but not handled in the switches.
I.e. the tree should build patch by patch, not just at the end of patch series.
- Arnaldo
Quoted 40 lines
Signed-off-by Julia Kartseva <[email protected]>--- tools/lib/bpf/libbpf.c | 1 + tools/lib/bpf/libbpf_probes.c | 2 ++ 2 files changed, 3 insertions(+)tools/lib/bpf/libbpf.c +1
-0@@ -3580,6 +3580,7 @@ static bool bpf_prog_type__needs_kver(enum bpf_prog_type type)case BPF_PROG_TYPE_PERF_EVENT :case BPF_PROG_TYPE_CGROUP_SYSCTL :case BPF_PROG_TYPE_CGROUP_SOCKOPT : +case __MAX_BPF_PROG_TYPE:return false;case BPF_PROG_TYPE_KPROBE :default :tools/lib/bpf/libbpf_probes.c +2
-0@@ -102,6 +102,7 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns,case BPF_PROG_TYPE_FLOW_DISSECTOR :case BPF_PROG_TYPE_CGROUP_SYSCTL :case BPF_PROG_TYPE_CGROUP_SOCKOPT : +case __MAX_BPF_PROG_TYPE:default :break ; }@@ -250,6 +251,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)case BPF_MAP_TYPE_XSKMAP :case BPF_MAP_TYPE_SOCKHASH :case BPF_MAP_TYPE_REUSEPORT_SOCKARRAY : +case __MAX_BPF_MAP_TYPE:default :break ; }Signature · 1 line
2.17.1
Signature · 1 line
- Arnaldo
PATCH 4/10 tools/bpf: add libbpf_prog_type_(from|to)_str helpers
In reply to Julia KartsevaStandardize string representation of prog types by putting commonly used names to libbpf.
The prog_type to string mapping is taken from bpftool:tools/bpf/bpftool/main.h
--- tools/lib/bpf/libbpf.c | 51 ++++++++++++++++++++++++++++++++++++++++ tools/lib/bpf/libbpf.h | 8 +++++++ tools/lib/bpf/libbpf.map | 2 ++ 3 files changed, 61 insertions(+)
tools/lib/bpf/libbpf.c +51 -0
@@ -296,6 +296,35 @@ struct bpf_object { };#define obj_elf_valid (o) ((o)->efile.elf) +static const char *const prog_type_strs[] = { + [BPF_PROG_TYPE_UNSPEC ] ="unspec" , + [BPF_PROG_TYPE_SOCKET_FILTER ] ="socket_filter" , + [BPF_PROG_TYPE_KPROBE ] ="kprobe" , + [BPF_PROG_TYPE_SCHED_CLS ] ="sched_cls" , + [BPF_PROG_TYPE_SCHED_ACT ] ="sched_act" , + [BPF_PROG_TYPE_TRACEPOINT ] ="tracepoint" , + [BPF_PROG_TYPE_XDP ] ="xdp" , + [BPF_PROG_TYPE_PERF_EVENT ] ="perf_event" , + [BPF_PROG_TYPE_CGROUP_SKB ] ="cgroup_skb" , + [BPF_PROG_TYPE_CGROUP_SOCK ] ="cgroup_sock" , + [BPF_PROG_TYPE_LWT_IN ] ="lwt_in" , + [BPF_PROG_TYPE_LWT_OUT ] ="lwt_out" , + [BPF_PROG_TYPE_LWT_XMIT ] ="lwt_xmit" , + [BPF_PROG_TYPE_SOCK_OPS ] ="sock_ops" , + [BPF_PROG_TYPE_SK_SKB ] ="sk_skb" , + [BPF_PROG_TYPE_CGROUP_DEVICE ] ="cgroup_device" , + [BPF_PROG_TYPE_SK_MSG ] ="sk_msg" , + [BPF_PROG_TYPE_RAW_TRACEPOINT ] ="raw_tracepoint" , + [BPF_PROG_TYPE_CGROUP_SOCK_ADDR ] ="cgroup_sock_addr" , + [BPF_PROG_TYPE_LWT_SEG6LOCAL ] ="lwt_seg6local" , + [BPF_PROG_TYPE_LIRC_MODE2 ] ="lirc_mode2" , + [BPF_PROG_TYPE_SK_REUSEPORT ] ="sk_reuseport" , + [BPF_PROG_TYPE_FLOW_DISSECTOR ] ="flow_dissector" , + [BPF_PROG_TYPE_CGROUP_SYSCTL ] ="cgroup_sysctl" , + [BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE ] ="raw_tracepoint_writable" , + [BPF_PROG_TYPE_CGROUP_SOCKOPT ] ="cgroup_sockopt" , +}; +void bpf_program__unload (struct bpf_program *prog) {int i;
@@ -4632,6 +4661,28 @@ int libbpf_attach_type_by_name(const char *name,return -EINVAL ; } +int libbpf_prog_type_from_str (const char *str,enum bpf_prog_type *type) +{ +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (prog_type_strs); i++) +if (prog_type_strs[i] &&strcmp (prog_type_strs[i], str) ==0 ) { + *type = i; +return 0 ; + } + +return -EINVAL ; +} + +int libbpf_prog_type_to_str (enum bpf_prog_type type,const char **str) +{ +if (type <BPF_PROG_TYPE_UNSPEC || type >=ARRAY_SIZE (prog_type_strs)) +return -EINVAL ; + + *str = prog_type_strs[type]; +return 0 ; +} + static int bpf_program__identify_section(struct bpf_program *prog, enum bpf_prog_type *prog_type,
tools/lib/bpf/libbpf.h +8 -0
@@ -122,12 +122,20 @@ LIBBPF_API int bpf_object__set_priv(struct bpf_object *obj, void *priv, bpf_object_clear_priv_t clear_priv); LIBBPF_API void *bpf_object__priv(const struct bpf_object *prog); +/* Program and expected attach types by section name */ LIBBPF_API intlibbpf_prog_type_by_name (const char *name,enum bpf_prog_type *prog_type,enum bpf_attach_type *expected_attach_type); +/* Attach type by section name */ LIBBPF_API int libbpf_attach_type_by_name(const char *name,enum bpf_attach_type *attach_type); +/* String representation of program type */ +LIBBPF_API int libbpf_prog_type_from_str(const char *str, +enum bpf_prog_type *type); +LIBBPF_API int libbpf_prog_type_to_str(enum bpf_prog_type type, +const char **str); +/* Accessors of bpf_program */ struct bpf_program ; LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,
tools/lib/bpf/libbpf.map +2 -0
@@ -188,4 +188,6 @@ LIBBPF_0.0.4 { LIBBPF_0.0.5 { global: bpf_btf_get_next_id; + libbpf_prog_type_from_str; + libbpf_prog_type_to_str; } LIBBPF_0.0.4;
Signature · 1 line
2.17.1
Re: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Quoted 6 lines · Julia Kartseva
Standardize string representation of prog types by putting commonly used names to libbpf.
Theprog_typeto string mapping is taken from bpftool:tools/bpf/bpftool/main.hSigned-off-by Julia Kartseva <[email protected]>
This "libbpf patches have to be completely separate" just went to another level :/ Now we are splitting code moves into add and remove parts which are 5 patches apart? How are we supposed to review this?
Greg, Thomas, libbpf is extracted from the kernel sources and maintained in a clone repo on GitHub for ease of packaging.
IIUC Alexei's concern is that since we are moving the commits from the kernel repo to the GitHub one we have to preserve the commits exactly as they are, otherwise SOB lines lose their power.
Can you provide some guidance on whether that's a valid concern, or whether it's perfectly fine to apply a partial patch?
(HW vendors also back port tree-wide cleanups into their drivers,
so if SOB lines are voided by git format-patch -- driver/path/ that'd be quite an issue..)
Quoted 105 lines
tools/lib/bpf/libbpf.c +51
-0@@ -296,6 +296,35 @@ struct bpf_object { };#define obj_elf_valid (o) ((o)->efile.elf) +static const char *const prog_type_strs[] = { + [BPF_PROG_TYPE_UNSPEC ] ="unspec" , + [BPF_PROG_TYPE_SOCKET_FILTER ] ="socket_filter" , + [BPF_PROG_TYPE_KPROBE ] ="kprobe" , + [BPF_PROG_TYPE_SCHED_CLS ] ="sched_cls" , + [BPF_PROG_TYPE_SCHED_ACT ] ="sched_act" , + [BPF_PROG_TYPE_TRACEPOINT ] ="tracepoint" , + [BPF_PROG_TYPE_XDP ] ="xdp" , + [BPF_PROG_TYPE_PERF_EVENT ] ="perf_event" , + [BPF_PROG_TYPE_CGROUP_SKB ] ="cgroup_skb" , + [BPF_PROG_TYPE_CGROUP_SOCK ] ="cgroup_sock" , + [BPF_PROG_TYPE_LWT_IN ] ="lwt_in" , + [BPF_PROG_TYPE_LWT_OUT ] ="lwt_out" , + [BPF_PROG_TYPE_LWT_XMIT ] ="lwt_xmit" , + [BPF_PROG_TYPE_SOCK_OPS ] ="sock_ops" , + [BPF_PROG_TYPE_SK_SKB ] ="sk_skb" , + [BPF_PROG_TYPE_CGROUP_DEVICE ] ="cgroup_device" , + [BPF_PROG_TYPE_SK_MSG ] ="sk_msg" , + [BPF_PROG_TYPE_RAW_TRACEPOINT ] ="raw_tracepoint" , + [BPF_PROG_TYPE_CGROUP_SOCK_ADDR ] ="cgroup_sock_addr" , + [BPF_PROG_TYPE_LWT_SEG6LOCAL ] ="lwt_seg6local" , + [BPF_PROG_TYPE_LIRC_MODE2 ] ="lirc_mode2" , + [BPF_PROG_TYPE_SK_REUSEPORT ] ="sk_reuseport" , + [BPF_PROG_TYPE_FLOW_DISSECTOR ] ="flow_dissector" , + [BPF_PROG_TYPE_CGROUP_SYSCTL ] ="cgroup_sysctl" , + [BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE ] ="raw_tracepoint_writable" , + [BPF_PROG_TYPE_CGROUP_SOCKOPT ] ="cgroup_sockopt" , +}; +void bpf_program__unload (struct bpf_program *prog) {int i;@@ -4632,6 +4661,28 @@ int libbpf_attach_type_by_name(const char *name,return -EINVAL ; } +int libbpf_prog_type_from_str (const char *str,enum bpf_prog_type *type) +{ +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (prog_type_strs); i++) +if (prog_type_strs[i] &&strcmp (prog_type_strs[i], str) ==0 ) { + *type = i; +return 0 ; + } + +return -EINVAL ; +} + +int libbpf_prog_type_to_str (enum bpf_prog_type type,const char **str) +{ +if (type <BPF_PROG_TYPE_UNSPEC || type >=ARRAY_SIZE (prog_type_strs)) +return -EINVAL ; + + *str = prog_type_strs[type]; +return 0 ; +} + static int bpf_program__identify_section(struct bpf_program *prog, enum bpf_prog_type *prog_type,tools/lib/bpf/libbpf.h +8
-0@@ -122,12 +122,20 @@ LIBBPF_API int bpf_object__set_priv(struct bpf_object *obj, void *priv, bpf_object_clear_priv_t clear_priv); LIBBPF_API void *bpf_object__priv(const struct bpf_object *prog); +/* Program and expected attach types by section name */ LIBBPF_API intlibbpf_prog_type_by_name (const char *name,enum bpf_prog_type *prog_type,enum bpf_attach_type *expected_attach_type); +/* Attach type by section name */ LIBBPF_API int libbpf_attach_type_by_name(const char *name,enum bpf_attach_type *attach_type); +/* String representation of program type */ +LIBBPF_API int libbpf_prog_type_from_str(const char *str, +enum bpf_prog_type *type); +LIBBPF_API int libbpf_prog_type_to_str(enum bpf_prog_type type, +const char **str); +/* Accessors of bpf_program */ struct bpf_program ; LIBBPF_API struct bpf_program *bpf_program__next(struct bpf_program *prog,tools/lib/bpf/libbpf.map +2
-0@@ -188,4 +188,6 @@ LIBBPF_0.0.4 { LIBBPF_0.0.5 { global: bpf_btf_get_next_id; + libbpf_prog_type_from_str; + libbpf_prog_type_to_str; } LIBBPF_0.0.4;
auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Quoted 10 lines · Jakub Kicinski
Greg, Thomas, libbpf is extracted from the kernel sources and maintained in a clone repo on GitHub for ease of packaging.
IIUC Alexei's concern is that since we are moving the commits from the kernel repo to the GitHub one we have to preserve the commits exactly as they are, otherwise SOB lines lose their power.
Can you provide some guidance on whether that's a valid concern, or whether it's perfectly fine to apply a partial patch?
Right. That's exactly the concern.
Greg, Thomas,
could you please put your legal hat on and clarify the following. Say some developer does a patch that modifies include/uapi/linux/bpf.h
..some other kernel code...and tools/include/uapi/linux/bpf.h
That tools/include/uapi/linux/bpf.h is used by perf and by libbpf. We have automatic mirror of tools/libbpf into github/libbpf/ so that external projects and can do git submodule of it, can build packages out of it, etc.
The question is whether it's ok to split tools/* part out of original commit, keep Author and SOB, create new commit out of it, and automatically push that auto-generated commit into github mirror.
So far we've requested all developers to split their patches manually. So that tools/* update is an individual commit that mirror can simply git cherry-pick.
Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Quoted 21 lines · Alexei Starovoitov
Quoted 10 lines · Jakub Kicinski
Greg, Thomas, libbpf is extracted from the kernel sources and maintained in a clone repo on GitHub for ease of packaging.
IIUC Alexei's concern is that since we are moving the commits from the kernel repo to the GitHub one we have to preserve the commits exactly as they are, otherwise SOB lines lose their power.
Can you provide some guidance on whether that's a valid concern, or whether it's perfectly fine to apply a partial patch?
Right. That's exactly the concern.
Greg, Thomas,
could you please put your legal hat on and clarify the following. Say some developer does a patch that modifiesinclude/uapi/linux/bpf.h
..some other kernel code...andtools/include/uapi/linux/bpf.h
Quoted 8 lines
That
tools/include/uapi/linux/bpf.his used by perf and by libbpf. We have automatic mirror oftools/libbpfinto github/libbpf/ so that external projects and can do git submodule of it, can build packages out of it, etc.The question is whether it's ok to split
tools/*part out of original commit, keep Author and SOB, create new commit out of it, and automatically push that auto-generated commit into github mirror.
Note, I am not a laywer, and am not _your_ lawyer either, only _your_ lawyer can answer questions as to what is best for you.
That being said, from a "are you keeping the correct authorship info", yes, it sounds like you are doing the correct thing here.
Look at what I do for stable kernels, I take the original commit and add it to "another tree" keeping the original author and s-o-b chain intact, and adding a "this is the original git commit id" type message to the changelog text so that people can link it back to the original.
If you are doing that here as well, I don't see how anyone can object.
thanks,
greg k-h
Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Quoted 34 lines · Greg Kroah-Hartman
Quoted 21 lines · Alexei Starovoitov
Quoted 10 lines · Jakub Kicinski
Greg, Thomas, libbpf is extracted from the kernel sources and maintained in a clone repo on GitHub for ease of packaging.
IIUC Alexei's concern is that since we are moving the commits from the kernel repo to the GitHub one we have to preserve the commits exactly as they are, otherwise SOB lines lose their power.
Can you provide some guidance on whether that's a valid concern, or whether it's perfectly fine to apply a partial patch?
Right. That's exactly the concern.
Greg, Thomas,
could you please put your legal hat on and clarify the following. Say some developer does a patch that modifiesinclude/uapi/linux/bpf.h
..some other kernel code...andtools/include/uapi/linux/bpf.hQuoted 8 lines
That
tools/include/uapi/linux/bpf.his used by perf and by libbpf. We have automatic mirror oftools/libbpfinto github/libbpf/ so that external projects and can do git submodule of it, can build packages out of it, etc.The question is whether it's ok to split
tools/*part out of original commit, keep Author and SOB, create new commit out of it, and automatically push that auto-generated commit into github mirror.Note, I am not a laywer, and am not _your_ lawyer either, only _your_ lawyer can answer questions as to what is best for you.
Quoted 7 lines
That being said, from a "are you keeping the correct authorship info", yes, it sounds like you are doing the correct thing here.
Look at what I do for stable kernels, I take the original commit and add it to "another tree" keeping the original author and s-o-b chain intact, and adding a "this is the original git commit id" type message to the changelog text so that people can link it back to the original.
I think you're describing 'git cherry-pick -x'.
The question was about taking pieces of the original commit. Not the whole commit. Author field obviously stays, but SOB is questionable.
If author meant to change X and Y and Z. Silently taking only Z chunk of the diff doesn't quite seem right.
If we document that such commit split happens in Documentation/bpf/bpf_devel_QA.rst do you think it will be enough to properly inform developers?
The main concern is the surprise factor when people start seeing their commits in the mirror, but not their full commits.
Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Em Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov escreveu:
Quoted 9 lines · Greg Kroah-Hartman
Quoted 7 lines
That being said, from a "are you keeping the correct authorship info", yes, it sounds like you are doing the correct thing here.
Look at what I do for stable kernels, I take the original commit and add it to "another tree" keeping the original author and s-o-b chain intact, and adding a "this is the original git commit id" type message to the changelog text so that people can link it back to the original.
Quoted 7 lines
I think you're describing 'git cherry-pick -x'.
The question was about taking pieces of the original commit. Not the whole commit. Author field obviously stays, but SOB is questionable.
If author meant to change X and Y and Z. Silently taking only Z chunk of the diff doesn't quite seem right.
If we document that such commit split happens inDocumentation/bpf/bpf_devel_QA.rstdo you think it will be enough to properly inform developers?
Can't we instead establish the rule that for something to be added to tools/include/ it should first land in a separate commit in include/, ditto for the other things tools/ copies from the kernel sources.
That was the initial intention of tools/include/ and also that is how tools/perf/check-headers.h works, warning when something ot out of sync, etc.
I.e. the tools/ maintainers should refuse patches that touch both tools/include and tools/.
wdyt?
- Arnaldo
Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Quoted 18 lines · Arnaldo Carvalho de Melo
Em Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov escreveu:
Quoted 9 lines · Greg Kroah-Hartman
Quoted 7 lines
That being said, from a "are you keeping the correct authorship info", yes, it sounds like you are doing the correct thing here.
Look at what I do for stable kernels, I take the original commit and add it to "another tree" keeping the original author and s-o-b chain intact, and adding a "this is the original git commit id" type message to the changelog text so that people can link it back to the original.
Quoted 7 lines
I think you're describing 'git cherry-pick -x'.
The question was about taking pieces of the original commit. Not the whole commit. Author field obviously stays, but SOB is questionable.
If author meant to change X and Y and Z. Silently taking only Z chunk of the diff doesn't quite seem right.
If we document that such commit split happens inDocumentation/bpf/bpf_devel_QA.rstdo you think it will be enough to properly inform developers?
Quoted 3 lines
Can't we instead establish the rule that for something to be added to
tools/include/it should first land in a separate commit ininclude/, ditto for the other thingstools/copies from the kernel sources.
In practice in for BPF work the tools/include/ patch is always part of the same patch set, since the patch sets usually include libbpf support, tests that need libbpf etc.
Quoted 8 lines
That was the initial intention of
tools/include/and also that is howtools/perf/check-headers.hworks, warning when something ot out of sync, etc.I.e. the
tools/maintainers should refuse patches that touch bothtools/includeandtools/.wdyt?
It's not only about include/. The series that sparked this query is moving code from tools/bpf/ to tools/lib/bpf/. And each move is split into two commits add and delete. That's utterly pointless and a waste of reviewers' time.
But the question is larger still. As I said vendors maintain out-of-tree version of their drivers, by necessity, e.g.:
https://github.com/Netronome/nfp-drv-kmods is a #ifdef'd version of driver/net/ethernet/netronome/nfp.
If there is a problem of loosing SOB when we only apply a part of a commit, e.g.
https://github.com/Netronome/nfp-drv-kmods/commit/79941cccea4a7720539e35a72c3ba789e4d4bf8c
which is part of:
ef01adae0e43 ("net: sched: use major priority number as hardware priority")
upstream - then we really need a clear ruling here.
Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
In reply to Alexei StarovoitovQuoted 38 lines · Alexei Starovoitov
Quoted 37 lines · Greg Kroah-Hartman
Quoted 21 lines · Alexei Starovoitov
Quoted 10 lines · Jakub Kicinski
Greg, Thomas, libbpf is extracted from the kernel sources and maintained in a clone repo on GitHub for ease of packaging.
IIUC Alexei's concern is that since we are moving the commits from the kernel repo to the GitHub one we have to preserve the commits exactly as they are, otherwise SOB lines lose their power.
Can you provide some guidance on whether that's a valid concern, or whether it's perfectly fine to apply a partial patch?
Right. That's exactly the concern.
Greg, Thomas,
could you please put your legal hat on and clarify the following. Say some developer does a patch that modifiesinclude/uapi/linux/bpf.h
..some other kernel code...andtools/include/uapi/linux/bpf.hQuoted 8 lines
That
tools/include/uapi/linux/bpf.his used by perf and by libbpf. We have automatic mirror oftools/libbpfinto github/libbpf/ so that external projects and can do git submodule of it, can build packages out of it, etc.The question is whether it's ok to split
tools/*part out of original commit, keep Author and SOB, create new commit out of it, and automatically push that auto-generated commit into github mirror.Note, I am not a laywer, and am not _your_ lawyer either, only _your_ lawyer can answer questions as to what is best for you.
That being said, from a "are you keeping the correct authorship info", yes, it sounds like you are doing the correct thing here.
Quoted 6 lines
Quoted 4 lines
Look at what I do for stable kernels, I take the original commit and add it to "another tree" keeping the original author and s-o-b chain intact, and adding a "this is the original git commit id" type message to the changelog text so that people can link it back to the original.
I think you're describing 'git cherry-pick -x'.
Well, my scripts don't use git, but yes, it's much the same thing :)
Quoted 2 lines
The question was about taking pieces of the original commit. Not the whole commit. Author field obviously stays, but SOB is questionable.
sob matters to the file the commit is touching, and if it is identical to the original file (including same license), then it should be fine.
Quoted 2 lines
If author meant to change X and Y and Z. Silently taking only Z chunk of the diff doesn't quite seem right.
It can be confusing, I agree.
Quoted 4 lines
If we document that such commit split happens in
Documentation/bpf/bpf_devel_QA.rstdo you think it will be enough to properly inform developers?
The main concern is the surprise factor when people start seeing their commits in the mirror, but not their full commits.
Personally, I wouldn't care, but maybe you should just enforce the fact that the original patch should ONLY touch Z, and not X and Y in the same patch, to make this a lot more obvious.
Patches should only be doing "one logical thing" in the first place, but maybe you also need to touch other things when doing a change that you can't do this, I really do not know, sorry.
greg k-h
Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
Quoted 64 lines · Greg Kroah-Hartman
Quoted 38 lines · Alexei Starovoitov
Quoted 37 lines · Greg Kroah-Hartman
Quoted 21 lines · Alexei Starovoitov
Quoted 10 lines · Jakub Kicinski
Greg, Thomas, libbpf is extracted from the kernel sources and maintained in a clone repo on GitHub for ease of packaging.
IIUC Alexei's concern is that since we are moving the commits from the kernel repo to the GitHub one we have to preserve the commits exactly as they are, otherwise SOB lines lose their power.
Can you provide some guidance on whether that's a valid concern, or whether it's perfectly fine to apply a partial patch?
Right. That's exactly the concern.
Greg, Thomas,
could you please put your legal hat on and clarify the following. Say some developer does a patch that modifiesinclude/uapi/linux/bpf.h
..some other kernel code...andtools/include/uapi/linux/bpf.hQuoted 8 lines
That
tools/include/uapi/linux/bpf.his used by perf and by libbpf. We have automatic mirror oftools/libbpfinto github/libbpf/ so that external projects and can do git submodule of it, can build packages out of it, etc.The question is whether it's ok to split
tools/*part out of original commit, keep Author and SOB, create new commit out of it, and automatically push that auto-generated commit into github mirror.Note, I am not a laywer, and am not _your_ lawyer either, only _your_ lawyer can answer questions as to what is best for you.
That being said, from a "are you keeping the correct authorship info", yes, it sounds like you are doing the correct thing here.
Quoted 6 lines
Quoted 4 lines
Look at what I do for stable kernels, I take the original commit and add it to "another tree" keeping the original author and s-o-b chain intact, and adding a "this is the original git commit id" type message to the changelog text so that people can link it back to the original.
I think you're describing 'git cherry-pick -x'.
Well, my scripts don't use git, but yes, it's much the same thing :)
Quoted 2 lines
The question was about taking pieces of the original commit. Not the whole commit. Author field obviously stays, but SOB is questionable.
sob matters to the file the commit is touching, and if it is identical to the original file (including same license), then it should be fine.
Quoted 2 lines
If author meant to change X and Y and Z. Silently taking only Z chunk of the diff doesn't quite seem right.
It can be confusing, I agree.
Quoted 4 lines
If we document that such commit split happens in
Documentation/bpf/bpf_devel_QA.rstdo you think it will be enough to properly inform developers?
The main concern is the surprise factor when people start seeing their commits in the mirror, but not their full commits.
Quoted 7 lines
Personally, I wouldn't care, but maybe you should just enforce the fact that the original patch should ONLY touch Z, and not X and Y in the same patch, to make this a lot more obvious.
Patches should only be doing "one logical thing" in the first place, but maybe you also need to touch other things when doing a change that you can't do this, I really do not know, sorry.
As someone who has been asked to split otherwise logically consistent patches to accommodate the sync, I would very much appreciate it if the process could be set up so this was not necessary :)
-Toke
PATCH 5/10 tools/bpf: add libbpf_map_type_(from|to)_str helpers
In reply to Julia KartsevaSimilar to prog_type to string mapping, standardize string representation of map types by putting commonly used names to libbpf.
The map_type to string mapping is taken from bpftool:tools/bpf/bpftool/map.c
--- tools/lib/bpf/libbpf.c | 51 ++++++++++++++++++++++++++++++++++++++++ tools/lib/bpf/libbpf.h | 4 ++++ tools/lib/bpf/libbpf.map | 2 ++ 3 files changed, 57 insertions(+)
tools/lib/bpf/libbpf.c +51 -0
@@ -325,6 +325,35 @@ static const char *const prog_type_strs[] = { [BPF_PROG_TYPE_CGROUP_SOCKOPT] = "cgroup_sockopt", }; +static const char *const map_type_strs[] = { + [BPF_MAP_TYPE_UNSPEC ] ="unspec" , + [BPF_MAP_TYPE_HASH ] ="hash" , + [BPF_MAP_TYPE_ARRAY ] ="array" , + [BPF_MAP_TYPE_PROG_ARRAY ] ="prog_array" , + [BPF_MAP_TYPE_PERF_EVENT_ARRAY ] ="perf_event_array" , + [BPF_MAP_TYPE_PERCPU_HASH ] ="percpu_hash" , + [BPF_MAP_TYPE_PERCPU_ARRAY ] ="percpu_array" , + [BPF_MAP_TYPE_STACK_TRACE ] ="stack_trace" , + [BPF_MAP_TYPE_CGROUP_ARRAY ] ="cgroup_array" , + [BPF_MAP_TYPE_LRU_HASH ] ="lru_hash" , + [BPF_MAP_TYPE_LRU_PERCPU_HASH ] ="lru_percpu_hash" , + [BPF_MAP_TYPE_LPM_TRIE ] ="lpm_trie" , + [BPF_MAP_TYPE_ARRAY_OF_MAPS ] ="array_of_maps" , + [BPF_MAP_TYPE_HASH_OF_MAPS ] ="hash_of_maps" , + [BPF_MAP_TYPE_DEVMAP ] ="devmap" , + [BPF_MAP_TYPE_SOCKMAP ] ="sockmap" , + [BPF_MAP_TYPE_CPUMAP ] ="cpumap" , + [BPF_MAP_TYPE_XSKMAP ] ="xskmap" , + [BPF_MAP_TYPE_SOCKHASH ] ="sockhash" , + [BPF_MAP_TYPE_CGROUP_STORAGE ] ="cgroup_storage" , + [BPF_MAP_TYPE_REUSEPORT_SOCKARRAY ] ="reuseport_sockarray" , + [BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE ] ="percpu_cgroup_storage" , + [BPF_MAP_TYPE_QUEUE ] ="queue" , + [BPF_MAP_TYPE_STACK ] ="stack" , + [BPF_MAP_TYPE_SK_STORAGE ] ="sk_storage" , + [BPF_MAP_TYPE_DEVMAP_HASH ] ="devmap_hash" +}; +void bpf_program__unload (struct bpf_program *prog) {int i;
@@ -4683,6 +4712,28 @@ int libbpf_prog_type_to_str(enum bpf_prog_type type, const char **str)return 0 ; } +int libbpf_map_type_from_str (const char *str,enum bpf_map_type *type) +{ +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (map_type_strs); i++) +if (map_type_strs[i] &&strcmp (map_type_strs[i], str) ==0 ) { + *type = i; +return 0 ; + } + +return -EINVAL ; +} + +int libbpf_map_type_to_str (enum bpf_map_type type,const char **str) +{ +if (type <BPF_MAP_TYPE_UNSPEC || type >=ARRAY_SIZE (map_type_strs)) +return -EINVAL ; + + *str = map_type_strs[type]; +return 0 ; +} + static int bpf_program__identify_section(struct bpf_program *prog, enum bpf_prog_type *prog_type,
tools/lib/bpf/libbpf.h +4 -0
@@ -135,6 +135,10 @@ LIBBPF_API int libbpf_prog_type_from_str(const char *str, enum bpf_prog_type *type); LIBBPF_API int libbpf_prog_type_to_str(enum bpf_prog_type type,const char **str); +/* String representation of map type */ +LIBBPF_API int libbpf_map_type_from_str(const char *str, +enum bpf_map_type *type); +LIBBPF_API int libbpf_map_type_to_str(enum bpf_map_type type, const char **str);/* Accessors of bpf_program */ struct bpf_program ;
tools/lib/bpf/libbpf.map +2 -0
@@ -190,4 +190,6 @@ LIBBPF_0.0.5 { bpf_btf_get_next_id; libbpf_prog_type_from_str; libbpf_prog_type_to_str; + libbpf_map_type_from_str; + libbpf_map_type_to_str; } LIBBPF_0.0.4;
Signature · 1 line
2.17.1
PATCH 6/10 tools/bpf: add libbpf_attach_type_(from|to)_str
In reply to Julia KartsevaStandardize string representation of attach types by putting commonly used names to libbpf.
The attach_type to string mapping is taken from bpftool:tools/bpf/bpftool/cgroup.ctools/bpf/bpftool/prog.c
--- tools/lib/bpf/libbpf.c | 50 ++++++++++++++++++++++++++++++++++++++++ tools/lib/bpf/libbpf.h | 5 ++++ tools/lib/bpf/libbpf.map | 2 ++ 3 files changed, 57 insertions(+)
tools/lib/bpf/libbpf.c +50 -0
@@ -354,6 +354,32 @@ static const char *const map_type_strs[] = { [BPF_MAP_TYPE_DEVMAP_HASH ] ="devmap_hash" }; +static const char *const attach_type_strs[] = { + [BPF_CGROUP_INET_INGRESS ] ="ingress" , + [BPF_CGROUP_INET_EGRESS ] ="egress" , + [BPF_CGROUP_INET_SOCK_CREATE ] ="sock_create" , + [BPF_CGROUP_SOCK_OPS ] ="sock_ops" , + [BPF_SK_SKB_STREAM_PARSER ] ="stream_parser" , + [BPF_SK_SKB_STREAM_VERDICT ] ="stream_verdict" , + [BPF_CGROUP_DEVICE ] ="device" , + [BPF_SK_MSG_VERDICT ] ="msg_verdict" , + [BPF_CGROUP_INET4_BIND ] ="bind4" , + [BPF_CGROUP_INET6_BIND ] ="bind6" , + [BPF_CGROUP_INET4_CONNECT ] ="connect4" , + [BPF_CGROUP_INET6_CONNECT ] ="connect6" , + [BPF_CGROUP_INET4_POST_BIND ] ="post_bind4" , + [BPF_CGROUP_INET6_POST_BIND ] ="post_bind6" , + [BPF_CGROUP_UDP4_SENDMSG ] ="sendmsg4" , + [BPF_CGROUP_UDP6_SENDMSG ] ="sendmsg6" , + [BPF_LIRC_MODE2 ] ="lirc_mode2" , + [BPF_FLOW_DISSECTOR ] ="flow_dissector" , + [BPF_CGROUP_SYSCTL ] ="sysctl" , + [BPF_CGROUP_UDP4_RECVMSG ] ="recvmsg4" , + [BPF_CGROUP_UDP6_RECVMSG ] ="recvmsg6" , + [BPF_CGROUP_GETSOCKOPT ] ="getsockopt" , + [BPF_CGROUP_SETSOCKOPT ] ="setsockopt" +}; +void bpf_program__unload (struct bpf_program *prog) {int i;
@@ -4734,6 +4760,30 @@ int libbpf_map_type_to_str(enum bpf_map_type type, const char **str)return 0 ; } +int libbpf_attach_type_from_str (const char *str,enum bpf_attach_type *type) +{ +unsigned int i; + +for (i =0 ; i <ARRAY_SIZE (attach_type_strs); i++) +if (attach_type_strs[i] && +strcmp (attach_type_strs[i], str) ==0 ) { + *type = i; +return 0 ; + } + +return -EINVAL ; +} + +int libbpf_attach_type_to_str (enum bpf_attach_type type,const char **str) +{ +if (type <BPF_CGROUP_INET_INGRESS || + type >=ARRAY_SIZE (attach_type_strs)) +return -EINVAL ; + + *str = attach_type_strs[type]; +return 0 ; +} + static int bpf_program__identify_section(struct bpf_program *prog, enum bpf_prog_type *prog_type,
tools/lib/bpf/libbpf.h +5 -0
@@ -139,6 +139,11 @@ LIBBPF_API int libbpf_prog_type_to_str(enum bpf_prog_type type, LIBBPF_API int libbpf_map_type_from_str(const char *str,enum bpf_map_type *type); LIBBPF_API int libbpf_map_type_to_str(enum bpf_map_type type, const char **str); +/* String representation of attach type */ +LIBBPF_API int libbpf_attach_type_from_str(const char *str, +enum bpf_attach_type *type); +LIBBPF_API int libbpf_attach_type_to_str(enum bpf_attach_type type, +const char **str);/* Accessors of bpf_program */ struct bpf_program ;
tools/lib/bpf/libbpf.map +2 -0
@@ -192,4 +192,6 @@ LIBBPF_0.0.5 { libbpf_prog_type_to_str; libbpf_map_type_from_str; libbpf_map_type_to_str; + libbpf_attach_type_from_str; + libbpf_attach_type_to_str; } LIBBPF_0.0.4;
Signature · 1 line
2.17.1
PATCH 7/10 selftests/bpf: extend test_section_names with type_(from|to)_str
In reply to Julia KartsevaTest bpf enum stringification helpers:
libbpf_(prog|map|attach)_type_(from|to)_str
--- .../selftests/bpf/test_section_names.c | 149 +++++++++++++++++- 1 file changed, 147 insertions(+), 2 deletions(-)
tools/testing/selftests/bpf/test_section_names.c +147 -2
@@ -202,7 +202,137 @@ static int test_attach_type_by_name(const struct sec_name_test *test)return 0 ; } -static int run_test_case (const struct sec_name_test *test) +static int test_prog_type_from_to_str (void ) +{ +enum bpf_prog_type type, actual_type; +const char *str; +int rc; + +for (type =BPF_PROG_TYPE_UNSPEC ; type < __MAX_BPF_PROG_TYPE; type++) { + rc =libbpf_prog_type_to_str (type, &str); +if (rc) { +warnx ("prog_type_to_str: unexpected rc=%d for type %d" , + rc, type); +return rc; + } + + rc =libbpf_prog_type_from_str (str, &actual_type); +if (rc) { +warnx ("prog_type_from_str: unexpected rc=%d for str %s" , + rc, str); +return rc; + } + +if (actual_type != type) { +warnx ("prog: unexpected prog_type for str %s, %d != %d" , + str, actual_type, type); +return -EINVAL ; + } + } + + rc =libbpf_prog_type_to_str (__MAX_BPF_PROG_TYPE, &str); +if (!rc) { +warnx ("prog: unexpected result for __MAX_BPF_PROG_TYPE" ); +return -EINVAL ; + } + + rc =libbpf_prog_type_from_str ("NonExistent" , &type); +if (!rc) { +warnx ("prog: unexpected result for non existent key" ); +return -EINVAL ; + } + +return 0 ; +} + +static int test_map_type_from_to_str (void ) +{ +enum bpf_map_type type, actual_type; +const char *str; +int rc; + +for (type =BPF_MAP_TYPE_UNSPEC ; type < __MAX_BPF_MAP_TYPE; type++) { + rc =libbpf_map_type_to_str (type, &str); +if (rc) { +warnx ("map_type_to_str: unexpected rc=%d for type %d" , + rc, type); +return rc; + } + + rc =libbpf_map_type_from_str (str, &actual_type); +if (rc) { +warnx ("map_type_from_str: unexpected rc=%d for str %s" , + rc, str); +return rc; + } + +if (actual_type != type) { +warnx ("map: unexpected map_type for str %s, %d != %d" , + str, actual_type, type); +return -EINVAL ; + } + } + + rc =libbpf_map_type_to_str (__MAX_BPF_MAP_TYPE, &str); +if (!rc) { +warnx ("map: unexpected result for __MAX_BPF_MAP_TYPE" ); +return -EINVAL ; + } + + rc =libbpf_map_type_from_str ("NonExistent" , &type); +if (!rc) { +warnx ("map: unexpected result for non existent key" ); +return -EINVAL ; + } + +return 0 ; +} + +static int test_attach_type_from_to_str (void ) +{ +enum bpf_attach_type type, actual_type; +const char *str; +int rc; + +for (type =BPF_CGROUP_INET_INGRESS ; type < __MAX_BPF_ATTACH_TYPE; + type++) { + rc =libbpf_attach_type_to_str (type, &str); +if (rc) { +warnx ("attach: unexpected rc=%d for type %d" , + rc, type); +return rc; + } + + rc =libbpf_attach_type_from_str (str, &actual_type); +if (rc) { +warnx ("attach: unexpected rc=%d for str %s" , + rc, str); +return rc; + } + +if (actual_type != type) { +warnx ("attach: unexpected type for str %s, %d != %d" , + str, actual_type, type); +return -EINVAL ; + } + } + + rc =libbpf_attach_type_to_str (__MAX_BPF_ATTACH_TYPE, &str); +if (!rc) { +warnx ("attach: unexpected result for __MAX_BPF_ATTACH_TYPE" ); +return -EINVAL ; + } + + rc =libbpf_attach_type_from_str ("NonExistent" , &type); +if (!rc) { +warnx ("attach: unexpected result for non existent key" ); +return -EINVAL ; + } + +return 0 ; +} + +static int run_sec_name_test_case (const struct sec_name_test *test) {if (test_prog_type_by_name (test))return -1 ;
@@ -218,11 +348,26 @@ static int run_tests(void)int i;for (i =0 ; i <ARRAY_SIZE (tests); ++i) { -if (run_test_case (&tests[i])) +if (run_sec_name_test_case (&tests[i])) ++fails;else ++passes; } + +if (test_prog_type_from_to_str ()) + ++fails; +else + ++passes; + +if (test_map_type_from_to_str ()) + ++fails; +else + ++passes; + +if (test_attach_type_from_to_str ()) + ++fails; +else + ++passes;printf ("Summary: %d PASSED, %d FAILED\n" , passes, fails);return fails ?-1 :0 ; }
Signature · 1 line
2.17.1
PATCH 8/10 selftests/bpf: rename test_section_names to test_section_and_type_names
In reply to Julia KartsevaChange the test name after extending it with enum stringification helpers.
--- tools/testing/selftests/bpf/Makefile | 2 +- .../bpf/test_section_and_type_names.c | 378 ++++++++++++++++++ .../selftests/bpf/test_section_names.c | 378 ------------------ 3 files changed, 379 insertions(+), 379 deletions(-) create mode 100644 tools/testing/selftests/bpf/test_section_and_type_names.c delete mode 100644 tools/testing/selftests/bpf/test_section_names.c
tools/testing/selftests/bpf/Makefile +1 -1
@@ -26,7 +26,7 @@ LDLIBS += -lcap -lelf -lrt -lpthread TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ test_sock test_btf test_sockmap get_cgroup_id_user test_socket_cookie \ - test_cgroup_storage test_select_reuseport test_section_names \ + test_cgroup_storage test_select_reuseport test_section_and_type_names \ test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \ test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \ test_sockopt_multi test_sockopt_inherit test_tcp_rtt
tools/testing/selftests/bpf/test_section_and_type_names.c new file +378 -0
378 lines not on this page · raw message
tools/testing/selftests/bpf/test_section_names.c deleted +0 -378
378 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH 9/10 tools/bpftool: use libbpf_(prog|map)_type_to_str helpers
In reply to Julia KartsevaReplace lookup in (prog|map)_type_name arrays with libbpf_(prog|map)_type_to_str helpers.
Use __MAX_BPF_(PROG|MAP)_TYPE enum values as loop bounds.
--- tools/bpf/bpftool/feature.c | 47 ++++++++++++++-------- tools/bpf/bpftool/main.h | 33 --------------- tools/bpf/bpftool/map.c | 80 ++++++++++--------------------------- tools/bpf/bpftool/prog.c | 11 ++--- 4 files changed, 59 insertions(+), 112 deletions(-)
tools/bpf/bpftool/feature.c +31 -16
@@ -462,7 +462,7 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types, const char *define_prefix, __u32 ifindex) { char feat_name[128], plain_desc[128], define_name[128]; -const char *plain_comment ="eBPF program_type " ; +const char *ptype_name, *plain_comment ="eBPF program_type " ;size_t maxlen;bool res;
@@ -480,16 +480,21 @@ probe_prog_type(enum bpf_prog_type prog_type, bool *supported_types, supported_types[prog_type] |= res; +if (libbpf_prog_type_to_str (prog_type, &ptype_name)) { +p_info ("program type name does not exist" ); +return ; + } + maxlen =sizeof (plain_desc) -strlen (plain_comment) -1 ; -if (strlen (prog_type_name[prog_type]) > maxlen) { +if (strlen (ptype_name) > maxlen) {p_info ("program type name too long" );return ; } -sprintf (feat_name,"have_%s_prog_type" , prog_type_name[prog_type]); -sprintf (define_name,"%s_prog_type" , prog_type_name[prog_type]); +sprintf (feat_name,"have_%s_prog_type" , ptype_name); +sprintf (define_name,"%s_prog_type" , ptype_name);uppercase (define_name,sizeof (define_name)); -sprintf (plain_desc,"%s%s" , plain_comment, prog_type_name[prog_type]); +sprintf (plain_desc,"%s%s" , plain_comment, ptype_name);print_bool_feature (feat_name, plain_desc, define_name, res, define_prefix); }
@@ -499,22 +504,27 @@ probe_map_type(enum bpf_map_type map_type, const char *define_prefix, __u32 ifindex) {char feat_name[128 ], plain_desc[128 ], define_name[128 ]; -const char *plain_comment ="eBPF map_type " ; +const char *mtype_name, *plain_comment ="eBPF map_type " ;size_t maxlen;bool res; res =bpf_probe_map_type (map_type, ifindex); +if (libbpf_map_type_to_str (map_type, &mtype_name)) { +p_info ("map type name does not exist" ); +return ; + } + maxlen =sizeof (plain_desc) -strlen (plain_comment) -1 ; -if (strlen (map_type_name[map_type]) > maxlen) { +if (strlen (mtype_name) > maxlen) {p_info ("map type name too long" );return ; } -sprintf (feat_name,"have_%s_map_type" , map_type_name[map_type]); -sprintf (define_name,"%s_map_type" , map_type_name[map_type]); +sprintf (feat_name,"have_%s_map_type" , mtype_name); +sprintf (define_name,"%s_map_type" , mtype_name);uppercase (define_name,sizeof (define_name)); -sprintf (plain_desc,"%s%s" , plain_comment, map_type_name[map_type]); +sprintf (plain_desc,"%s%s" , plain_comment, mtype_name);print_bool_feature (feat_name, plain_desc, define_name, res, define_prefix); }
@@ -523,11 +533,16 @@ static void probe_helpers_for_progtype(enum bpf_prog_type prog_type, bool supported_type, const char *define_prefix, __u32 ifindex) { - const char *ptype_name = prog_type_name[prog_type]; + const char *ptype_name;char feat_name[128 ];unsigned int id;bool res; +if (libbpf_prog_type_to_str (prog_type, &ptype_name)) { +p_info ("map type name does not exist" ); +return ; + } +if (ifindex)/* Only test helpers for offload-able program types */ switch (prog_type) {
@@ -689,7 +704,7 @@ static int do_probe(int argc, char **argv)"/*** eBPF program types ***/" , define_prefix); -for (i =BPF_PROG_TYPE_UNSPEC +1 ; i <ARRAY_SIZE (prog_type_name); i++) +for (i =BPF_PROG_TYPE_UNSPEC +1 ; i < __MAX_BPF_PROG_TYPE; i++)probe_prog_type (i, supported_types, define_prefix, ifindex); print_end_then_start_section("map_types",
@@ -697,7 +712,7 @@ static int do_probe(int argc, char **argv)"/*** eBPF map types ***/" , define_prefix); -for (i =BPF_MAP_TYPE_UNSPEC +1 ; i < map_type_name_size; i++) +for (i =BPF_MAP_TYPE_UNSPEC +1 ; i < __MAX_BPF_MAP_TYPE; i++)probe_map_type (i, define_prefix, ifindex); print_end_then_start_section("helpers",
@@ -720,9 +735,9 @@ static int do_probe(int argc, char **argv)" %sBPF__PROG_TYPE_ ## prog_type ## __HELPER_ ## helper\n" , define_prefix, define_prefix, define_prefix, define_prefix); -for (i =BPF_PROG_TYPE_UNSPEC +1 ; i <ARRAY_SIZE (prog_type_name); i++) -probe_helpers_for_progtype (i, supported_types[i], - define_prefix, ifindex); +for (i =BPF_PROG_TYPE_UNSPEC +1 ; i <= __MAX_BPF_PROG_TYPE; i++) +probe_helpers_for_progtype (i, supported_types[i], define_prefix, + ifindex); exit_close_json:if (json_output) {
tools/bpf/bpftool/main.h +0 -33
@@ -48,39 +48,6 @@ "\t {-m|--mapcompat} | {-n|--nomount} }"#define HELP_SPEC_MAP \ "MAP := { id MAP_ID | pinned FILE }" - -static const char *const prog_type_name[] = { - [BPF_PROG_TYPE_UNSPEC ] ="unspec" , - [BPF_PROG_TYPE_SOCKET_FILTER ] ="socket_filter" , - [BPF_PROG_TYPE_KPROBE ] ="kprobe" , - [BPF_PROG_TYPE_SCHED_CLS ] ="sched_cls" , - [BPF_PROG_TYPE_SCHED_ACT ] ="sched_act" , - [BPF_PROG_TYPE_TRACEPOINT ] ="tracepoint" , - [BPF_PROG_TYPE_XDP ] ="xdp" , - [BPF_PROG_TYPE_PERF_EVENT ] ="perf_event" , - [BPF_PROG_TYPE_CGROUP_SKB ] ="cgroup_skb" , - [BPF_PROG_TYPE_CGROUP_SOCK ] ="cgroup_sock" , - [BPF_PROG_TYPE_LWT_IN ] ="lwt_in" , - [BPF_PROG_TYPE_LWT_OUT ] ="lwt_out" , - [BPF_PROG_TYPE_LWT_XMIT ] ="lwt_xmit" , - [BPF_PROG_TYPE_SOCK_OPS ] ="sock_ops" , - [BPF_PROG_TYPE_SK_SKB ] ="sk_skb" , - [BPF_PROG_TYPE_CGROUP_DEVICE ] ="cgroup_device" , - [BPF_PROG_TYPE_SK_MSG ] ="sk_msg" , - [BPF_PROG_TYPE_RAW_TRACEPOINT ] ="raw_tracepoint" , - [BPF_PROG_TYPE_CGROUP_SOCK_ADDR ] ="cgroup_sock_addr" , - [BPF_PROG_TYPE_LWT_SEG6LOCAL ] ="lwt_seg6local" , - [BPF_PROG_TYPE_LIRC_MODE2 ] ="lirc_mode2" , - [BPF_PROG_TYPE_SK_REUSEPORT ] ="sk_reuseport" , - [BPF_PROG_TYPE_FLOW_DISSECTOR ] ="flow_dissector" , - [BPF_PROG_TYPE_CGROUP_SYSCTL ] ="cgroup_sysctl" , - [BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE ] ="raw_tracepoint_writable" , - [BPF_PROG_TYPE_CGROUP_SOCKOPT ] ="cgroup_sockopt" , -}; - -extern const char *const map_type_name[]; -extern const size_t map_type_name_size; -enum bpf_obj_type {BPF_OBJ_UNKNOWN ,BPF_OBJ_PROG ,
tools/bpf/bpftool/map.c +22 -58
157 lines not on this page · raw message
tools/bpf/bpftool/prog.c +6 -5
@@ -196,13 +196,13 @@ static void show_prog_maps(int fd, u32 num_maps)static void print_prog_json (struct bpf_prog_info *info,int fd) { +const char *ptype_name;char *memlock;jsonw_start_object (json_wtr);jsonw_uint_field (json_wtr,"id" , info->id); -if (info->type <ARRAY_SIZE (prog_type_name)) -jsonw_string_field (json_wtr,"type" , - prog_type_name[info->type]); +if (!libbpf_prog_type_to_str (info->type, &ptype_name)) +jsonw_string_field (json_wtr,"type" , ptype_name);else jsonw_uint_field (json_wtr,"type" , info->type);
@@ -270,11 +270,12 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)static void print_prog_plain (struct bpf_prog_info *info,int fd) { +const char *ptype_name;char *memlock;printf ("%u: " , info->id); -if (info->type <ARRAY_SIZE (prog_type_name)) -printf ("%s " , prog_type_name[info->type]); +if (!libbpf_prog_type_to_str (info->type, &ptype_name)) +printf ("%s " , ptype_name);else printf ("type %u " , info->type);
Signature · 1 line
2.17.1
PATCH 10/10 tools/bpftool: use libbpf_attach_type_to_str helper
In reply to Julia KartsevaReplace lookup in attach_type_strings with libbpf_attach_type_to_str helper for cgroup (cgroup.c) and non-cgroup (prog.c) attach types.
--- tools/bpf/bpftool/cgroup.c | 60 +++++++++++++++++++++----------------- tools/bpf/bpftool/prog.c | 20 +++++++------ 2 files changed, 45 insertions(+), 35 deletions(-)
tools/bpf/bpftool/cgroup.c +34 -26
@@ -15,6 +15,7 @@#include <unistd.h> #include <bpf.h> +#include <libbpf.h> #include "main.h"
@@ -31,35 +32,37 @@static unsigned int query_flags; -static const char *const attach_type_strings[] = { - [BPF_CGROUP_INET_INGRESS ] ="ingress" , - [BPF_CGROUP_INET_EGRESS ] ="egress" , - [BPF_CGROUP_INET_SOCK_CREATE ] ="sock_create" , - [BPF_CGROUP_SOCK_OPS ] ="sock_ops" , - [BPF_CGROUP_DEVICE ] ="device" , - [BPF_CGROUP_INET4_BIND ] ="bind4" , - [BPF_CGROUP_INET6_BIND ] ="bind6" , - [BPF_CGROUP_INET4_CONNECT ] ="connect4" , - [BPF_CGROUP_INET6_CONNECT ] ="connect6" , - [BPF_CGROUP_INET4_POST_BIND ] ="post_bind4" , - [BPF_CGROUP_INET6_POST_BIND ] ="post_bind6" , - [BPF_CGROUP_UDP4_SENDMSG ] ="sendmsg4" , - [BPF_CGROUP_UDP6_SENDMSG ] ="sendmsg6" , - [BPF_CGROUP_SYSCTL ] ="sysctl" , - [BPF_CGROUP_UDP4_RECVMSG ] ="recvmsg4" , - [BPF_CGROUP_UDP6_RECVMSG ] ="recvmsg6" , - [BPF_CGROUP_GETSOCKOPT ] ="getsockopt" , - [BPF_CGROUP_SETSOCKOPT ] ="setsockopt" , - [__MAX_BPF_ATTACH_TYPE] =NULL , +static const enum bpf_attach_type cgroup_attach_types[] = { +BPF_CGROUP_INET_INGRESS , +BPF_CGROUP_INET_EGRESS , +BPF_CGROUP_INET_SOCK_CREATE , +BPF_CGROUP_SOCK_OPS , +BPF_CGROUP_DEVICE , +BPF_CGROUP_INET4_BIND , +BPF_CGROUP_INET6_BIND , +BPF_CGROUP_INET4_CONNECT , +BPF_CGROUP_INET6_CONNECT , +BPF_CGROUP_INET4_POST_BIND , +BPF_CGROUP_INET6_POST_BIND , +BPF_CGROUP_UDP4_SENDMSG , +BPF_CGROUP_UDP6_SENDMSG , +BPF_CGROUP_SYSCTL , +BPF_CGROUP_UDP4_RECVMSG , +BPF_CGROUP_UDP6_RECVMSG , +BPF_CGROUP_GETSOCKOPT , +BPF_CGROUP_SETSOCKOPT , };static enum bpf_attach_type parse_attach_type (const char *str) {enum bpf_attach_type type; +const char *atype_str; +unsigned int i; -for (type =0 ; type < __MAX_BPF_ATTACH_TYPE; type++) { -if (attach_type_strings[type] && -is_prefix (str, attach_type_strings[type])) +for (i =0 ; i <ARRAY_SIZE (cgroup_attach_types); i++) { + type = cgroup_attach_types[i]; +if (!libbpf_attach_type_to_str (type, &atype_str) && +is_prefix (str, atype_str))return type; }
@@ -120,7 +123,7 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)static int show_attached_bpf_progs (int cgroup_fd,enum bpf_attach_type type,int level) { -const char *attach_flags_str; +const char *attach_flags_str, *atype_str;__u32 prog_ids[1024 ] = {0 };__u32 prog_cnt, iter;__u32 attach_flags;
@@ -136,6 +139,11 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,if (prog_cnt ==0 )return 0 ; + ret =libbpf_attach_type_to_str (type, &atype_str); + +if (ret) +return 0 ; +switch (attach_flags) {case BPF_F_ALLOW_MULTI : attach_flags_str ="multi" ;
@@ -152,8 +160,8 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type, }for (iter =0 ; iter < prog_cnt; iter++) -show_bpf_prog (prog_ids[iter], attach_type_strings[type], - attach_flags_str, level); +show_bpf_prog (prog_ids[iter], atype_str, attach_flags_str, + level);return 0 ; }
tools/bpf/bpftool/prog.c +11 -9
@@ -25,21 +25,23 @@#include "main.h" #include "xlated_dumper.h" -static const char *const attach_type_strings[] = { - [BPF_SK_SKB_STREAM_PARSER ] ="stream_parser" , - [BPF_SK_SKB_STREAM_VERDICT ] ="stream_verdict" , - [BPF_SK_MSG_VERDICT ] ="msg_verdict" , - [BPF_FLOW_DISSECTOR ] ="flow_dissector" , - [__MAX_BPF_ATTACH_TYPE] =NULL , +static const enum bpf_attach_type attach_types[] = { +BPF_SK_SKB_STREAM_PARSER , +BPF_SK_SKB_STREAM_VERDICT , +BPF_SK_MSG_VERDICT , +BPF_FLOW_DISSECTOR , };static enum bpf_attach_type parse_attach_type (const char *str) {enum bpf_attach_type type; +const char *atype_str; +unsigned int i; -for (type =0 ; type < __MAX_BPF_ATTACH_TYPE; type++) { -if (attach_type_strings[type] && -is_prefix (str, attach_type_strings[type])) +for (i =0 ; type <ARRAY_SIZE (attach_types); i++) { + type = attach_types[i]; +if (!libbpf_attach_type_to_str (type, &atype_str) && +is_prefix (str, atype_str))return type; }
Signature · 1 line
2.17.1