Skip to contentSkip to conversation

bpf: libbpf: retry map creation without the name

PATCH Superseded Stanislav Fomichev 3 participants 5 messages Nov 20, 2018

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 bpf: libbpf: retry map creation without the name

Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name for maps. Pre v4.14 kernels don't know about map names and return an error about unexpected non-zero data. Retry sys_bpf without a map name to cover older kernels.

---
 tools/lib/bpf/bpf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
tools/lib/bpf/bpf.c +10 -1
index 03f9bcc4ef50..673175bc06ee 100644
@@ -69,6 +69,7 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 {
 	__u32 name_len = create_attr->name ? strlen(create_attr->name) : 0;
 	union bpf_attr attr;
+	int ret;
 
 	memset(&attr, '\0', sizeof(attr));
 
@@ -86,7 +87,15 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 	attr.map_ifindex = create_attr->map_ifindex;
 	attr.inner_map_fd = create_attr->inner_map_fd;
 
-	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	ret = sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	if (ret < 0 && create_attr->name) {
+		/* Retry the same syscall, but without the name.
+		 * Pre v4.14 kernels don't support map names.
+		 */
+		memset(attr.map_name, 0, sizeof(attr.map_name));
+		return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	}
+	return ret;
 }
 
 int bpf_create_map_node(enum bpf_map_type map_type, const char *name,
Signature · 1 line
2.19.1.1215.g8438c0b245-goog
maintainer
#1 lore raw
Quoted 5 lines · Stanislav Fomichev

Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name for maps. Pre v4.14 kernels don't know about map names and return an error about unexpected non-zero data. Retry sys_bpf without a map name to cover older kernels.

Looks good, small nit below:

Quoted 18 lines
---
 tools/lib/bpf/bpf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
tools/lib/bpf/bpf.c +1 -0
index 03f9bcc4ef50..673175bc06ee 100644
@@ -69,6 +69,7 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 {
 	__u32 name_len = create_attr->name ? strlen(create_attr->name) : 0;
 	union bpf_attr attr;
+	int ret;
 
 	memset(&attr, '\0', sizeof(attr));
 

tools/lib/bpf/bpf.c int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr) · line 91

Quoted 6 lines
tools/lib/bpf/bpf.c +2 -1
@@ -86,7 +87,15 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 	attr.map_ifindex = create_attr->map_ifindex;
 	attr.inner_map_fd = create_attr->inner_map_fd;
 
-	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	ret = sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	if (ret < 0 && create_attr->name) {

Could you also check errno being EINVAL here so that we do not try to confuse it with an error coming from insufficient memlock which is EPERM in that case.

Quoted 11 lines
tools/lib/bpf/bpf.c +7 -0
+		/* Retry the same syscall, but without the name.
+		 * Pre v4.14 kernels don't support map names.
+		 */
+		memset(attr.map_name, 0, sizeof(attr.map_name));
+		return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	}
+	return ret;
 }
 
 int bpf_create_map_node(enum bpf_map_type map_type, const char *name,
#2 lore raw

PATCH v2 1/1 bpf: libbpf: retry map creation without the name

Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name for maps. Pre v4.14 kernels don't know about map names and return an error about unexpected non-zero data. Retry sys_bpf without a map name to cover older kernels.

v2 changes:
* check for errno == EINVAL as suggested by Daniel Borkmann

---
 tools/lib/bpf/bpf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
tools/lib/bpf/bpf.c +10 -1
index 03f9bcc4ef50..961e1b9fc592 100644
@@ -69,6 +69,7 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 {
 	__u32 name_len = create_attr->name ? strlen(create_attr->name) : 0;
 	union bpf_attr attr;
+	int ret;
 
 	memset(&attr, '\0', sizeof(attr));
 
@@ -86,7 +87,15 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 	attr.map_ifindex = create_attr->map_ifindex;
 	attr.inner_map_fd = create_attr->inner_map_fd;
 
-	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	ret = sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	if (ret < 0 && errno == EINVAL && create_attr->name) {
+		/* Retry the same syscall, but without the name.
+		 * Pre v4.14 kernels don't support map names.
+		 */
+		memset(attr.map_name, 0, sizeof(attr.map_name));
+		return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	}
+	return ret;
 }
 
 int bpf_create_map_node(enum bpf_map_type map_type, const char *name,
Signature · 1 line
2.19.1.1215.g8438c0b245-goog
maintainer Applied
#3 lore raw
Quoted 10 lines · Stanislav Fomichev

Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name for maps. Pre v4.14 kernels don't know about map names and return an error about unexpected non-zero data. Retry sys_bpf without a map name to cover older kernels.

v2 changes:
* check for errno == EINVAL as suggested by Daniel Borkmann

Applied to bpf-next, thanks!

#4 lore raw
Quoted 7 lines · Daniel Borkmann
Quoted 6 lines · Stanislav Fomichev

Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name for maps. Pre v4.14 kernels don't know about map names and return an error about unexpected non-zero data. Retry sys_bpf without a map name to cover older kernels.

Quoted 6 lines
Quoted 4 lines

v2 changes:
* check for errno == EINVAL as suggested by Daniel Borkmann

Applied to bpf-next, thanks!

Great, thanks! It looks like there might be the same problem with the program's name. I'll probably follow up with another patch soon.