netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers
@ 2019-08-28 21:03 Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values Julia Kartseva
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

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

-- 
2.17.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:33   ` Alexei Starovoitov
  2019-08-28 21:03 ` [PATCH bpf-next 02/10] tools/bpf: sync bpf.h to tools/ Julia Kartseva
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

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.

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 include/uapi/linux/bpf.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 5d2fb183ee2d..9b681bb82211 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -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,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 02/10] tools/bpf: sync bpf.h to tools/
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements Julia Kartseva
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Introduce __MAX_BPF_MAP_TYPE and __MAX_BPF_MAP_TYPE enum values.

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 tools/include/uapi/linux/bpf.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 5d2fb183ee2d..9b681bb82211 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -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,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 02/10] tools/bpf: sync bpf.h to tools/ Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:19   ` Arnaldo Carvalho de Melo
  2019-08-28 21:03 ` [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers Julia Kartseva
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Add 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.

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 tools/lib/bpf/libbpf.c        | 1 +
 tools/lib/bpf/libbpf_probes.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2233f919dd88..72e6e5eb397f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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:
diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
index 4b0b0364f5fc..8f2ba6a457ac 100644
--- a/tools/lib/bpf/libbpf_probes.c
+++ b/tools/lib/bpf/libbpf_probes.c
@@ -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;
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (2 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 23:34   ` Jakub Kicinski
  2019-08-28 21:03 ` [PATCH bpf-next 05/10] tools/bpf: add libbpf_map_type_(from|to)_str helpers Julia Kartseva
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Standardize 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

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 tools/lib/bpf/libbpf.c   | 51 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  8 +++++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 61 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 72e6e5eb397f..946a4d41f223 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e8f70977d137..6846c488d8a2 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -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 int
 libbpf_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,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 664ce8e7a60e..2ea7c99f1579 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 05/10] tools/bpf: add libbpf_map_type_(from|to)_str helpers
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (3 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 06/10] tools/bpf: add libbpf_attach_type_(from|to)_str Julia Kartseva
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Similar 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

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 tools/lib/bpf/libbpf.c   | 51 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  4 ++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 57 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 946a4d41f223..9c531256888b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 6846c488d8a2..90daeb2cdefb 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -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;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2ea7c99f1579..e4ecf5414bb7 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 06/10] tools/bpf: add libbpf_attach_type_(from|to)_str
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (4 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 05/10] tools/bpf: add libbpf_map_type_(from|to)_str helpers Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 07/10] selftests/bpf: extend test_section_names with type_(from|to)_str Julia Kartseva
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Standardize 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.c
tools/bpf/bpftool/prog.c

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 tools/lib/bpf/libbpf.c   | 50 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  5 ++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 57 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9c531256888b..b5b07493655f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 90daeb2cdefb..0ad941951b0d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -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;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index e4ecf5414bb7..d87a6dc8a71f 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 07/10] selftests/bpf: extend test_section_names with type_(from|to)_str
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (5 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 06/10] tools/bpf: add libbpf_attach_type_(from|to)_str Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 08/10] selftests/bpf: rename test_section_names to test_section_and_type_names Julia Kartseva
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Test bpf enum stringification helpers:
libbpf_(prog|map|attach)_type_(from|to)_str

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 .../selftests/bpf/test_section_names.c        | 149 +++++++++++++++++-
 1 file changed, 147 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_section_names.c b/tools/testing/selftests/bpf/test_section_names.c
index 29833aeaf0de..564585a07592 100644
--- a/tools/testing/selftests/bpf/test_section_names.c
+++ b/tools/testing/selftests/bpf/test_section_names.c
@@ -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;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 08/10] selftests/bpf: rename test_section_names to test_section_and_type_names
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (6 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 07/10] selftests/bpf: extend test_section_names with type_(from|to)_str Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 09/10] tools/bpftool: use libbpf_(prog|map)_type_to_str helpers Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 10/10] tools/bpftool: use libbpf_attach_type_to_str helper Julia Kartseva
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Change the test name after extending it with enum stringification
helpers.

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 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

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1faad0c3c3c9..8212a6240297 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -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
diff --git a/tools/testing/selftests/bpf/test_section_and_type_names.c b/tools/testing/selftests/bpf/test_section_and_type_names.c
new file mode 100644
index 000000000000..564585a07592
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_section_and_type_names.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <err.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_util.h"
+
+struct sec_name_test {
+	const char sec_name[32];
+	struct {
+		int rc;
+		enum bpf_prog_type prog_type;
+		enum bpf_attach_type expected_attach_type;
+	} expected_load;
+	struct {
+		int rc;
+		enum bpf_attach_type attach_type;
+	} expected_attach;
+};
+
+static struct sec_name_test tests[] = {
+	{"InvAliD", {-EINVAL, 0, 0}, {-EINVAL, 0} },
+	{"cgroup", {-EINVAL, 0, 0}, {-EINVAL, 0} },
+	{"socket", {0, BPF_PROG_TYPE_SOCKET_FILTER, 0}, {-EINVAL, 0} },
+	{"kprobe/", {0, BPF_PROG_TYPE_KPROBE, 0}, {-EINVAL, 0} },
+	{"kretprobe/", {0, BPF_PROG_TYPE_KPROBE, 0}, {-EINVAL, 0} },
+	{"classifier", {0, BPF_PROG_TYPE_SCHED_CLS, 0}, {-EINVAL, 0} },
+	{"action", {0, BPF_PROG_TYPE_SCHED_ACT, 0}, {-EINVAL, 0} },
+	{"tracepoint/", {0, BPF_PROG_TYPE_TRACEPOINT, 0}, {-EINVAL, 0} },
+	{
+		"raw_tracepoint/",
+		{0, BPF_PROG_TYPE_RAW_TRACEPOINT, 0},
+		{-EINVAL, 0},
+	},
+	{"xdp", {0, BPF_PROG_TYPE_XDP, 0}, {-EINVAL, 0} },
+	{"perf_event", {0, BPF_PROG_TYPE_PERF_EVENT, 0}, {-EINVAL, 0} },
+	{"lwt_in", {0, BPF_PROG_TYPE_LWT_IN, 0}, {-EINVAL, 0} },
+	{"lwt_out", {0, BPF_PROG_TYPE_LWT_OUT, 0}, {-EINVAL, 0} },
+	{"lwt_xmit", {0, BPF_PROG_TYPE_LWT_XMIT, 0}, {-EINVAL, 0} },
+	{"lwt_seg6local", {0, BPF_PROG_TYPE_LWT_SEG6LOCAL, 0}, {-EINVAL, 0} },
+	{
+		"cgroup_skb/ingress",
+		{0, BPF_PROG_TYPE_CGROUP_SKB, 0},
+		{0, BPF_CGROUP_INET_INGRESS},
+	},
+	{
+		"cgroup_skb/egress",
+		{0, BPF_PROG_TYPE_CGROUP_SKB, 0},
+		{0, BPF_CGROUP_INET_EGRESS},
+	},
+	{"cgroup/skb", {0, BPF_PROG_TYPE_CGROUP_SKB, 0}, {-EINVAL, 0} },
+	{
+		"cgroup/sock",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK, 0},
+		{0, BPF_CGROUP_INET_SOCK_CREATE},
+	},
+	{
+		"cgroup/post_bind4",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND},
+		{0, BPF_CGROUP_INET4_POST_BIND},
+	},
+	{
+		"cgroup/post_bind6",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND},
+		{0, BPF_CGROUP_INET6_POST_BIND},
+	},
+	{
+		"cgroup/dev",
+		{0, BPF_PROG_TYPE_CGROUP_DEVICE, 0},
+		{0, BPF_CGROUP_DEVICE},
+	},
+	{"sockops", {0, BPF_PROG_TYPE_SOCK_OPS, 0}, {0, BPF_CGROUP_SOCK_OPS} },
+	{
+		"sk_skb/stream_parser",
+		{0, BPF_PROG_TYPE_SK_SKB, 0},
+		{0, BPF_SK_SKB_STREAM_PARSER},
+	},
+	{
+		"sk_skb/stream_verdict",
+		{0, BPF_PROG_TYPE_SK_SKB, 0},
+		{0, BPF_SK_SKB_STREAM_VERDICT},
+	},
+	{"sk_skb", {0, BPF_PROG_TYPE_SK_SKB, 0}, {-EINVAL, 0} },
+	{"sk_msg", {0, BPF_PROG_TYPE_SK_MSG, 0}, {0, BPF_SK_MSG_VERDICT} },
+	{"lirc_mode2", {0, BPF_PROG_TYPE_LIRC_MODE2, 0}, {0, BPF_LIRC_MODE2} },
+	{
+		"flow_dissector",
+		{0, BPF_PROG_TYPE_FLOW_DISSECTOR, 0},
+		{0, BPF_FLOW_DISSECTOR},
+	},
+	{
+		"cgroup/bind4",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND},
+		{0, BPF_CGROUP_INET4_BIND},
+	},
+	{
+		"cgroup/bind6",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND},
+		{0, BPF_CGROUP_INET6_BIND},
+	},
+	{
+		"cgroup/connect4",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT},
+		{0, BPF_CGROUP_INET4_CONNECT},
+	},
+	{
+		"cgroup/connect6",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT},
+		{0, BPF_CGROUP_INET6_CONNECT},
+	},
+	{
+		"cgroup/sendmsg4",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG},
+		{0, BPF_CGROUP_UDP4_SENDMSG},
+	},
+	{
+		"cgroup/sendmsg6",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG},
+		{0, BPF_CGROUP_UDP6_SENDMSG},
+	},
+	{
+		"cgroup/recvmsg4",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG},
+		{0, BPF_CGROUP_UDP4_RECVMSG},
+	},
+	{
+		"cgroup/recvmsg6",
+		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG},
+		{0, BPF_CGROUP_UDP6_RECVMSG},
+	},
+	{
+		"cgroup/sysctl",
+		{0, BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL},
+		{0, BPF_CGROUP_SYSCTL},
+	},
+	{
+		"cgroup/getsockopt",
+		{0, BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT},
+		{0, BPF_CGROUP_GETSOCKOPT},
+	},
+	{
+		"cgroup/setsockopt",
+		{0, BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT},
+		{0, BPF_CGROUP_SETSOCKOPT},
+	},
+};
+
+static int test_prog_type_by_name(const struct sec_name_test *test)
+{
+	enum bpf_attach_type expected_attach_type;
+	enum bpf_prog_type prog_type;
+	int rc;
+
+	rc = libbpf_prog_type_by_name(test->sec_name, &prog_type,
+				      &expected_attach_type);
+
+	if (rc != test->expected_load.rc) {
+		warnx("prog: unexpected rc=%d for %s", rc, test->sec_name);
+		return -1;
+	}
+
+	if (rc)
+		return 0;
+
+	if (prog_type != test->expected_load.prog_type) {
+		warnx("prog: unexpected prog_type=%d for %s", prog_type,
+		      test->sec_name);
+		return -1;
+	}
+
+	if (expected_attach_type != test->expected_load.expected_attach_type) {
+		warnx("prog: unexpected expected_attach_type=%d for %s",
+		      expected_attach_type, test->sec_name);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int test_attach_type_by_name(const struct sec_name_test *test)
+{
+	enum bpf_attach_type attach_type;
+	int rc;
+
+	rc = libbpf_attach_type_by_name(test->sec_name, &attach_type);
+
+	if (rc != test->expected_attach.rc) {
+		warnx("attach: unexpected rc=%d for %s", rc, test->sec_name);
+		return -1;
+	}
+
+	if (rc)
+		return 0;
+
+	if (attach_type != test->expected_attach.attach_type) {
+		warnx("attach: unexpected attach_type=%d for %s", attach_type,
+		      test->sec_name);
+		return -1;
+	}
+
+	return 0;
+}
+
+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;
+	if (test_attach_type_by_name(test))
+		return -1;
+	return 0;
+}
+
+static int run_tests(void)
+{
+	int passes = 0;
+	int fails = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(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;
+}
+
+int main(int argc, char **argv)
+{
+	return run_tests();
+}
diff --git a/tools/testing/selftests/bpf/test_section_names.c b/tools/testing/selftests/bpf/test_section_names.c
deleted file mode 100644
index 564585a07592..000000000000
--- a/tools/testing/selftests/bpf/test_section_names.c
+++ /dev/null
@@ -1,378 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (c) 2018 Facebook
-
-#include <err.h>
-#include <bpf/libbpf.h>
-
-#include "bpf_util.h"
-
-struct sec_name_test {
-	const char sec_name[32];
-	struct {
-		int rc;
-		enum bpf_prog_type prog_type;
-		enum bpf_attach_type expected_attach_type;
-	} expected_load;
-	struct {
-		int rc;
-		enum bpf_attach_type attach_type;
-	} expected_attach;
-};
-
-static struct sec_name_test tests[] = {
-	{"InvAliD", {-EINVAL, 0, 0}, {-EINVAL, 0} },
-	{"cgroup", {-EINVAL, 0, 0}, {-EINVAL, 0} },
-	{"socket", {0, BPF_PROG_TYPE_SOCKET_FILTER, 0}, {-EINVAL, 0} },
-	{"kprobe/", {0, BPF_PROG_TYPE_KPROBE, 0}, {-EINVAL, 0} },
-	{"kretprobe/", {0, BPF_PROG_TYPE_KPROBE, 0}, {-EINVAL, 0} },
-	{"classifier", {0, BPF_PROG_TYPE_SCHED_CLS, 0}, {-EINVAL, 0} },
-	{"action", {0, BPF_PROG_TYPE_SCHED_ACT, 0}, {-EINVAL, 0} },
-	{"tracepoint/", {0, BPF_PROG_TYPE_TRACEPOINT, 0}, {-EINVAL, 0} },
-	{
-		"raw_tracepoint/",
-		{0, BPF_PROG_TYPE_RAW_TRACEPOINT, 0},
-		{-EINVAL, 0},
-	},
-	{"xdp", {0, BPF_PROG_TYPE_XDP, 0}, {-EINVAL, 0} },
-	{"perf_event", {0, BPF_PROG_TYPE_PERF_EVENT, 0}, {-EINVAL, 0} },
-	{"lwt_in", {0, BPF_PROG_TYPE_LWT_IN, 0}, {-EINVAL, 0} },
-	{"lwt_out", {0, BPF_PROG_TYPE_LWT_OUT, 0}, {-EINVAL, 0} },
-	{"lwt_xmit", {0, BPF_PROG_TYPE_LWT_XMIT, 0}, {-EINVAL, 0} },
-	{"lwt_seg6local", {0, BPF_PROG_TYPE_LWT_SEG6LOCAL, 0}, {-EINVAL, 0} },
-	{
-		"cgroup_skb/ingress",
-		{0, BPF_PROG_TYPE_CGROUP_SKB, 0},
-		{0, BPF_CGROUP_INET_INGRESS},
-	},
-	{
-		"cgroup_skb/egress",
-		{0, BPF_PROG_TYPE_CGROUP_SKB, 0},
-		{0, BPF_CGROUP_INET_EGRESS},
-	},
-	{"cgroup/skb", {0, BPF_PROG_TYPE_CGROUP_SKB, 0}, {-EINVAL, 0} },
-	{
-		"cgroup/sock",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK, 0},
-		{0, BPF_CGROUP_INET_SOCK_CREATE},
-	},
-	{
-		"cgroup/post_bind4",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND},
-		{0, BPF_CGROUP_INET4_POST_BIND},
-	},
-	{
-		"cgroup/post_bind6",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET6_POST_BIND},
-		{0, BPF_CGROUP_INET6_POST_BIND},
-	},
-	{
-		"cgroup/dev",
-		{0, BPF_PROG_TYPE_CGROUP_DEVICE, 0},
-		{0, BPF_CGROUP_DEVICE},
-	},
-	{"sockops", {0, BPF_PROG_TYPE_SOCK_OPS, 0}, {0, BPF_CGROUP_SOCK_OPS} },
-	{
-		"sk_skb/stream_parser",
-		{0, BPF_PROG_TYPE_SK_SKB, 0},
-		{0, BPF_SK_SKB_STREAM_PARSER},
-	},
-	{
-		"sk_skb/stream_verdict",
-		{0, BPF_PROG_TYPE_SK_SKB, 0},
-		{0, BPF_SK_SKB_STREAM_VERDICT},
-	},
-	{"sk_skb", {0, BPF_PROG_TYPE_SK_SKB, 0}, {-EINVAL, 0} },
-	{"sk_msg", {0, BPF_PROG_TYPE_SK_MSG, 0}, {0, BPF_SK_MSG_VERDICT} },
-	{"lirc_mode2", {0, BPF_PROG_TYPE_LIRC_MODE2, 0}, {0, BPF_LIRC_MODE2} },
-	{
-		"flow_dissector",
-		{0, BPF_PROG_TYPE_FLOW_DISSECTOR, 0},
-		{0, BPF_FLOW_DISSECTOR},
-	},
-	{
-		"cgroup/bind4",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND},
-		{0, BPF_CGROUP_INET4_BIND},
-	},
-	{
-		"cgroup/bind6",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_BIND},
-		{0, BPF_CGROUP_INET6_BIND},
-	},
-	{
-		"cgroup/connect4",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_CONNECT},
-		{0, BPF_CGROUP_INET4_CONNECT},
-	},
-	{
-		"cgroup/connect6",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET6_CONNECT},
-		{0, BPF_CGROUP_INET6_CONNECT},
-	},
-	{
-		"cgroup/sendmsg4",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_SENDMSG},
-		{0, BPF_CGROUP_UDP4_SENDMSG},
-	},
-	{
-		"cgroup/sendmsg6",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_SENDMSG},
-		{0, BPF_CGROUP_UDP6_SENDMSG},
-	},
-	{
-		"cgroup/recvmsg4",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP4_RECVMSG},
-		{0, BPF_CGROUP_UDP4_RECVMSG},
-	},
-	{
-		"cgroup/recvmsg6",
-		{0, BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_UDP6_RECVMSG},
-		{0, BPF_CGROUP_UDP6_RECVMSG},
-	},
-	{
-		"cgroup/sysctl",
-		{0, BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL},
-		{0, BPF_CGROUP_SYSCTL},
-	},
-	{
-		"cgroup/getsockopt",
-		{0, BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_GETSOCKOPT},
-		{0, BPF_CGROUP_GETSOCKOPT},
-	},
-	{
-		"cgroup/setsockopt",
-		{0, BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT},
-		{0, BPF_CGROUP_SETSOCKOPT},
-	},
-};
-
-static int test_prog_type_by_name(const struct sec_name_test *test)
-{
-	enum bpf_attach_type expected_attach_type;
-	enum bpf_prog_type prog_type;
-	int rc;
-
-	rc = libbpf_prog_type_by_name(test->sec_name, &prog_type,
-				      &expected_attach_type);
-
-	if (rc != test->expected_load.rc) {
-		warnx("prog: unexpected rc=%d for %s", rc, test->sec_name);
-		return -1;
-	}
-
-	if (rc)
-		return 0;
-
-	if (prog_type != test->expected_load.prog_type) {
-		warnx("prog: unexpected prog_type=%d for %s", prog_type,
-		      test->sec_name);
-		return -1;
-	}
-
-	if (expected_attach_type != test->expected_load.expected_attach_type) {
-		warnx("prog: unexpected expected_attach_type=%d for %s",
-		      expected_attach_type, test->sec_name);
-		return -1;
-	}
-
-	return 0;
-}
-
-static int test_attach_type_by_name(const struct sec_name_test *test)
-{
-	enum bpf_attach_type attach_type;
-	int rc;
-
-	rc = libbpf_attach_type_by_name(test->sec_name, &attach_type);
-
-	if (rc != test->expected_attach.rc) {
-		warnx("attach: unexpected rc=%d for %s", rc, test->sec_name);
-		return -1;
-	}
-
-	if (rc)
-		return 0;
-
-	if (attach_type != test->expected_attach.attach_type) {
-		warnx("attach: unexpected attach_type=%d for %s", attach_type,
-		      test->sec_name);
-		return -1;
-	}
-
-	return 0;
-}
-
-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;
-	if (test_attach_type_by_name(test))
-		return -1;
-	return 0;
-}
-
-static int run_tests(void)
-{
-	int passes = 0;
-	int fails = 0;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(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;
-}
-
-int main(int argc, char **argv)
-{
-	return run_tests();
-}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 09/10] tools/bpftool: use libbpf_(prog|map)_type_to_str helpers
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (7 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 08/10] selftests/bpf: rename test_section_names to test_section_and_type_names Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  2019-08-28 21:03 ` [PATCH bpf-next 10/10] tools/bpftool: use libbpf_attach_type_to_str helper Julia Kartseva
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Replace 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.

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 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(-)

diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index 03bdc5b3ac49..e9416005e721 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -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) {
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index af9ad56c303a..bb840d900fb4 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -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,
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index de61d73b9030..ca3760b5c33e 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -16,42 +16,12 @@
 #include <sys/stat.h>
 
 #include <bpf.h>
+#include <libbpf.h>
 
 #include "btf.h"
 #include "json_writer.h"
 #include "main.h"
 
-const char * const map_type_name[] = {
-	[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_DEVMAP_HASH]		= "devmap_hash",
-	[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",
-};
-
-const size_t map_type_name_size = ARRAY_SIZE(map_type_name);
-
 static bool map_is_per_cpu(__u32 type)
 {
 	return type == BPF_MAP_TYPE_PERCPU_HASH ||
@@ -71,17 +41,6 @@ static bool map_is_map_of_progs(__u32 type)
 	return type == BPF_MAP_TYPE_PROG_ARRAY;
 }
 
-static int map_type_from_str(const char *type)
-{
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(map_type_name); i++)
-		/* Don't allow prefixing in case of possible future shadowing */
-		if (map_type_name[i] && !strcmp(map_type_name[i], type))
-			return i;
-	return -1;
-}
-
 static void *alloc_value(struct bpf_map_info *info)
 {
 	if (map_is_per_cpu(info->type))
@@ -481,6 +440,8 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 
 static int show_map_close_json(int fd, struct bpf_map_info *info)
 {
+	const char *ptype_name, *mtype_name;
+	enum bpf_prog_type prog_type;
 	char *memlock, *frozen_str;
 	int frozen = 0;
 
@@ -488,11 +449,10 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
 	frozen_str = get_fdinfo(fd, "frozen");
 
 	jsonw_start_object(json_wtr);
-
 	jsonw_uint_field(json_wtr, "id", info->id);
-	if (info->type < ARRAY_SIZE(map_type_name))
-		jsonw_string_field(json_wtr, "type",
-				   map_type_name[info->type]);
+
+	if (!libbpf_map_type_to_str(info->type, &mtype_name))
+		jsonw_string_field(json_wtr, "type", mtype_name);
 	else
 		jsonw_uint_field(json_wtr, "type", info->type);
 
@@ -517,11 +477,11 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
 		char *owner_jited = get_fdinfo(fd, "owner_jited");
 
 		if (owner_prog_type) {
-			unsigned int prog_type = atoi(owner_prog_type);
+			prog_type = atoi(owner_prog_type);
 
-			if (prog_type < ARRAY_SIZE(prog_type_name))
+			if (!libbpf_prog_type_to_str(prog_type, &ptype_name))
 				jsonw_string_field(json_wtr, "owner_prog_type",
-						   prog_type_name[prog_type]);
+						   ptype_name);
 			else
 				jsonw_uint_field(json_wtr, "owner_prog_type",
 						 prog_type);
@@ -563,6 +523,7 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
 
 static int show_map_close_plain(int fd, struct bpf_map_info *info)
 {
+	const char *mtype_name, *ptype_name;
 	char *memlock, *frozen_str;
 	int frozen = 0;
 
@@ -570,8 +531,9 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
 	frozen_str = get_fdinfo(fd, "frozen");
 
 	printf("%u: ", info->id);
-	if (info->type < ARRAY_SIZE(map_type_name))
-		printf("%s  ", map_type_name[info->type]);
+
+	if (!libbpf_map_type_to_str(info->type, &mtype_name))
+		printf("%s  ", mtype_name);
 	else
 		printf("type %u  ", info->type);
 
@@ -596,10 +558,8 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
 			printf("\n\t");
 		if (owner_prog_type) {
 			unsigned int prog_type = atoi(owner_prog_type);
-
-			if (prog_type < ARRAY_SIZE(prog_type_name))
-				printf("owner_prog_type %s  ",
-				       prog_type_name[prog_type]);
+			if (!libbpf_prog_type_to_str(prog_type, &ptype_name))
+				printf("owner_prog_type %s  ", ptype_name);
 			else
 				printf("owner_prog_type %d  ", prog_type);
 		}
@@ -772,6 +732,7 @@ static int do_dump(int argc, char **argv)
 	unsigned int num_elems = 0;
 	__u32 len = sizeof(info);
 	json_writer_t *btf_wtr;
+	const char *mtype_name;
 	struct btf *btf = NULL;
 	int err;
 	int fd;
@@ -813,10 +774,14 @@ static int do_dump(int argc, char **argv)
 			}
 		}
 
+		if (libbpf_map_type_to_str(info.type, &mtype_name)) {
+			p_info("map type name does not exist");
+			goto exit_free;
+		}
 	if (info.type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY &&
 	    info.value_size != 8)
 		p_info("Warning: cannot read values from %s map with value_size != 8",
-		       map_type_name[info.type]);
+		       mtype_name);
 	while (true) {
 		err = bpf_map_get_next_key(fd, prev_key, key);
 		if (err) {
@@ -1150,8 +1115,7 @@ static int do_create(int argc, char **argv)
 				return -1;
 			}
 
-			attr.map_type = map_type_from_str(*argv);
-			if ((int)attr.map_type < 0) {
+			if (libbpf_map_type_from_str(*argv, &attr.map_type)) {
 				p_err("unrecognized map type: %s", *argv);
 				return -1;
 			}
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 66f04a4846a5..8bbb24339a52 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -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);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH bpf-next 10/10] tools/bpftool: use libbpf_attach_type_to_str helper
  2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
                   ` (8 preceding siblings ...)
  2019-08-28 21:03 ` [PATCH bpf-next 09/10] tools/bpftool: use libbpf_(prog|map)_type_to_str helpers Julia Kartseva
@ 2019-08-28 21:03 ` Julia Kartseva
  9 siblings, 0 replies; 21+ messages in thread
From: Julia Kartseva @ 2019-08-28 21:03 UTC (permalink / raw)
  To: rdna, bpf, ast, daniel, netdev, kernel-team; +Cc: Julia Kartseva

Replace lookup in `attach_type_strings` with
libbpf_attach_type_to_str helper for cgroup (cgroup.c)
and non-cgroup (prog.c) attach types.

Signed-off-by: Julia Kartseva <hex@fb.com>
---
 tools/bpf/bpftool/cgroup.c | 60 +++++++++++++++++++++-----------------
 tools/bpf/bpftool/prog.c   | 20 +++++++------
 2 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index 1ef45e55039e..1b53218b06e7 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -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;
 }
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 8bbb24339a52..4dfec67e22fa 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -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;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements
  2019-08-28 21:03 ` [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements Julia Kartseva
@ 2019-08-28 21:19   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-28 21:19 UTC (permalink / raw)
  To: Julia Kartseva; +Cc: rdna, bpf, ast, daniel, netdev, kernel-team

Em Wed, Aug 28, 2019 at 02:03:06PM -0700, Julia Kartseva escreveu:
> Add 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.

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
 
> Signed-off-by: Julia Kartseva <hex@fb.com>
> ---
>  tools/lib/bpf/libbpf.c        | 1 +
>  tools/lib/bpf/libbpf_probes.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2233f919dd88..72e6e5eb397f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -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:
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index 4b0b0364f5fc..8f2ba6a457ac 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -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;
>  	}
> -- 
> 2.17.1

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values
  2019-08-28 21:03 ` [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values Julia Kartseva
@ 2019-08-28 21:33   ` Alexei Starovoitov
  0 siblings, 0 replies; 21+ messages in thread
From: Alexei Starovoitov @ 2019-08-28 21:33 UTC (permalink / raw)
  To: Julia Kartseva; +Cc: rdna, bpf, ast, daniel, netdev, kernel-team

On Wed, Aug 28, 2019 at 02:03:04PM -0700, Julia Kartseva wrote:
> 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.
> 
> Signed-off-by: Julia Kartseva <hex@fb.com>
> ---
>  include/uapi/linux/bpf.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 5d2fb183ee2d..9b681bb82211 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -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
> +

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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-28 21:03 ` [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers Julia Kartseva
@ 2019-08-28 23:34   ` Jakub Kicinski
  2019-08-28 23:46     ` auto-split of commit. Was: " Alexei Starovoitov
  0 siblings, 1 reply; 21+ messages in thread
From: Jakub Kicinski @ 2019-08-28 23:34 UTC (permalink / raw)
  To: Julia Kartseva, ast, Greg Kroah-Hartman, Thomas Gleixner
  Cc: rdna, bpf, daniel, netdev, kernel-team

On Wed, 28 Aug 2019 14:03:07 -0700, Julia Kartseva wrote:
> Standardize 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
> 
> Signed-off-by: Julia Kartseva <hex@fb.com>

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..)

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 72e6e5eb397f..946a4d41f223 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -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,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e8f70977d137..6846c488d8a2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -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 int
>  libbpf_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,
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 664ce8e7a60e..2ea7c99f1579 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -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;


^ permalink raw reply	[flat|nested] 21+ messages in thread

* auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-28 23:34   ` Jakub Kicinski
@ 2019-08-28 23:46     ` Alexei Starovoitov
  2019-08-29  6:51       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Alexei Starovoitov @ 2019-08-28 23:46 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Julia Kartseva, ast, Greg Kroah-Hartman, Thomas Gleixner, rdna,
	bpf, daniel, netdev, kernel-team

On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> 
> 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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-28 23:46     ` auto-split of commit. Was: " Alexei Starovoitov
@ 2019-08-29  6:51       ` Greg Kroah-Hartman
  2019-08-29 17:16         ` Alexei Starovoitov
  0 siblings, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-29  6:51 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jakub Kicinski, Julia Kartseva, ast, Thomas Gleixner, rdna, bpf,
	daniel, netdev, kernel-team

On Wed, Aug 28, 2019 at 04:46:28PM -0700, Alexei Starovoitov wrote:
> On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> > 
> > 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.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-29  6:51       ` Greg Kroah-Hartman
@ 2019-08-29 17:16         ` Alexei Starovoitov
  2019-08-29 18:10           ` Arnaldo Carvalho de Melo
  2019-08-30  6:47           ` Greg Kroah-Hartman
  0 siblings, 2 replies; 21+ messages in thread
From: Alexei Starovoitov @ 2019-08-29 17:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jakub Kicinski, Julia Kartseva, ast, Thomas Gleixner, rdna, bpf,
	daniel, netdev, kernel-team

On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Aug 28, 2019 at 04:46:28PM -0700, Alexei Starovoitov wrote:
> > On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> > > 
> > > 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.
> 
> 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.

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.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-29 17:16         ` Alexei Starovoitov
@ 2019-08-29 18:10           ` Arnaldo Carvalho de Melo
  2019-08-29 18:50             ` Jakub Kicinski
  2019-08-30  6:47           ` Greg Kroah-Hartman
  1 sibling, 1 reply; 21+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-29 18:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Greg Kroah-Hartman, Jakub Kicinski, Julia Kartseva, ast,
	Thomas Gleixner, rdna, bpf, daniel, netdev, kernel-team

Em Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov escreveu:
> On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:
> > 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?

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-29 18:10           ` Arnaldo Carvalho de Melo
@ 2019-08-29 18:50             ` Jakub Kicinski
  0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2019-08-29 18:50 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Alexei Starovoitov, Greg Kroah-Hartman, Julia Kartseva, ast,
	Thomas Gleixner, rdna, bpf, daniel, netdev, kernel-team

On Thu, 29 Aug 2019 15:10:39 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov escreveu:
> > On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:  
> > > 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?  
> 
> 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.

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.
 
> 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?

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.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-29 17:16         ` Alexei Starovoitov
  2019-08-29 18:10           ` Arnaldo Carvalho de Melo
@ 2019-08-30  6:47           ` Greg Kroah-Hartman
  2019-08-30  9:01             ` Toke Høiland-Jørgensen
  1 sibling, 1 reply; 21+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-30  6:47 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jakub Kicinski, Julia Kartseva, ast, Thomas Gleixner, rdna, bpf,
	daniel, netdev, kernel-team

On Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov wrote:
> On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Aug 28, 2019 at 04:46:28PM -0700, Alexei Starovoitov wrote:
> > > On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
> > > > 
> > > > 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.
> > 
> > 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.
> 
> I think you're describing 'git cherry-pick -x'.

Well, my scripts don't use git, but yes, it's much the same thing :)

> 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.

> 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.

> 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.

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: auto-split of commit. Was: [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers
  2019-08-30  6:47           ` Greg Kroah-Hartman
@ 2019-08-30  9:01             ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 21+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-08-30  9:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Alexei Starovoitov
  Cc: Jakub Kicinski, Julia Kartseva, ast, Thomas Gleixner, rdna, bpf,
	daniel, netdev, kernel-team

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:

> On Thu, Aug 29, 2019 at 10:16:56AM -0700, Alexei Starovoitov wrote:
>> On Thu, Aug 29, 2019 at 08:51:51AM +0200, Greg Kroah-Hartman wrote:
>> > On Wed, Aug 28, 2019 at 04:46:28PM -0700, Alexei Starovoitov wrote:
>> > > On Wed, Aug 28, 2019 at 04:34:22PM -0700, Jakub Kicinski wrote:
>> > > > 
>> > > > 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.
>> > 
>> > 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.
>> 
>> I think you're describing 'git cherry-pick -x'.
>
> Well, my scripts don't use git, but yes, it's much the same thing :)
>
>> 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.
>
>> 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.
>
>> 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.
>
> 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2019-08-30  9:01 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 21:03 [PATCH bpf-next 00/10] bpf: bpf_(prog|map|attach)_type_(from|to)_str helpers Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 01/10] bpf: introduce __MAX_BPF_PROG_TYPE and __MAX_BPF_MAP_TYPE enum values Julia Kartseva
2019-08-28 21:33   ` Alexei Starovoitov
2019-08-28 21:03 ` [PATCH bpf-next 02/10] tools/bpf: sync bpf.h to tools/ Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 03/10] tools/bpf: handle __MAX_BPF_(PROG|MAP)_TYPE in switch statements Julia Kartseva
2019-08-28 21:19   ` Arnaldo Carvalho de Melo
2019-08-28 21:03 ` [PATCH bpf-next 04/10] tools/bpf: add libbpf_prog_type_(from|to)_str helpers Julia Kartseva
2019-08-28 23:34   ` Jakub Kicinski
2019-08-28 23:46     ` auto-split of commit. Was: " Alexei Starovoitov
2019-08-29  6:51       ` Greg Kroah-Hartman
2019-08-29 17:16         ` Alexei Starovoitov
2019-08-29 18:10           ` Arnaldo Carvalho de Melo
2019-08-29 18:50             ` Jakub Kicinski
2019-08-30  6:47           ` Greg Kroah-Hartman
2019-08-30  9:01             ` Toke Høiland-Jørgensen
2019-08-28 21:03 ` [PATCH bpf-next 05/10] tools/bpf: add libbpf_map_type_(from|to)_str helpers Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 06/10] tools/bpf: add libbpf_attach_type_(from|to)_str Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 07/10] selftests/bpf: extend test_section_names with type_(from|to)_str Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 08/10] selftests/bpf: rename test_section_names to test_section_and_type_names Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 09/10] tools/bpftool: use libbpf_(prog|map)_type_to_str helpers Julia Kartseva
2019-08-28 21:03 ` [PATCH bpf-next 10/10] tools/bpftool: use libbpf_attach_type_to_str helper Julia Kartseva

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).