* [PATCH bpf-next 1/6] bpf: btf: Avoid WARN_ON when CONFIG_REFCOUNT_FULL=y
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
@ 2018-05-04 21:49 ` Martin KaFai Lau
2018-05-08 21:09 ` Song Liu
2018-05-04 21:49 ` [PATCH bpf-next 2/6] bpf: btf: Introduce BTF ID Martin KaFai Lau
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2018-05-04 21:49 UTC (permalink / raw)
To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
If CONFIG_REFCOUNT_FULL=y, refcount_inc() WARN when refcount is 0.
When creating a new btf, the initial btf->refcnt is 0 and
triggered the following:
[ 34.855452] refcount_t: increment on 0; use-after-free.
[ 34.856252] WARNING: CPU: 6 PID: 1857 at lib/refcount.c:153 refcount_inc+0x26/0x30
....
[ 34.868809] Call Trace:
[ 34.869168] btf_new_fd+0x1af6/0x24d0
[ 34.869645] ? btf_type_seq_show+0x200/0x200
[ 34.870212] ? lock_acquire+0x3b0/0x3b0
[ 34.870726] ? security_capable+0x54/0x90
[ 34.871247] __x64_sys_bpf+0x1b2/0x310
[ 34.871761] ? __ia32_sys_bpf+0x310/0x310
[ 34.872285] ? bad_area_access_error+0x310/0x310
[ 34.872894] do_syscall_64+0x95/0x3f0
This patch uses refcount_set() instead.
Reported-by: Yonghong Song <yhs@fb.com>
Tested-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/btf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 22e1046a1a86..fa0dce0452e7 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1977,7 +1977,7 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
if (!err) {
btf_verifier_env_free(env);
- btf_get(btf);
+ refcount_set(&btf->refcnt, 1);
return btf;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH bpf-next 1/6] bpf: btf: Avoid WARN_ON when CONFIG_REFCOUNT_FULL=y
2018-05-04 21:49 ` [PATCH bpf-next 1/6] bpf: btf: Avoid WARN_ON when CONFIG_REFCOUNT_FULL=y Martin KaFai Lau
@ 2018-05-08 21:09 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2018-05-08 21:09 UTC (permalink / raw)
To: Martin Lau; +Cc: Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team
> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>
> If CONFIG_REFCOUNT_FULL=y, refcount_inc() WARN when refcount is 0.
> When creating a new btf, the initial btf->refcnt is 0 and
> triggered the following:
>
> [ 34.855452] refcount_t: increment on 0; use-after-free.
> [ 34.856252] WARNING: CPU: 6 PID: 1857 at lib/refcount.c:153 refcount_inc+0x26/0x30
> ....
> [ 34.868809] Call Trace:
> [ 34.869168] btf_new_fd+0x1af6/0x24d0
> [ 34.869645] ? btf_type_seq_show+0x200/0x200
> [ 34.870212] ? lock_acquire+0x3b0/0x3b0
> [ 34.870726] ? security_capable+0x54/0x90
> [ 34.871247] __x64_sys_bpf+0x1b2/0x310
> [ 34.871761] ? __ia32_sys_bpf+0x310/0x310
> [ 34.872285] ? bad_area_access_error+0x310/0x310
> [ 34.872894] do_syscall_64+0x95/0x3f0
>
> This patch uses refcount_set() instead.
>
> Reported-by: Yonghong Song <yhs@fb.com>
> Tested-by: Yonghong Song <yhs@fb.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> kernel/bpf/btf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 22e1046a1a86..fa0dce0452e7 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1977,7 +1977,7 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>
> if (!err) {
> btf_verifier_env_free(env);
> - btf_get(btf);
> + refcount_set(&btf->refcnt, 1);
> return btf;
> }
>
> --
> 2.9.5
>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf-next 2/6] bpf: btf: Introduce BTF ID
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
2018-05-04 21:49 ` [PATCH bpf-next 1/6] bpf: btf: Avoid WARN_ON when CONFIG_REFCOUNT_FULL=y Martin KaFai Lau
@ 2018-05-04 21:49 ` Martin KaFai Lau
2018-05-08 21:09 ` Song Liu
2018-05-04 21:49 ` [PATCH bpf-next 3/6] bpf: btf: Add struct bpf_btf_info Martin KaFai Lau
` (4 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2018-05-04 21:49 UTC (permalink / raw)
To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
This patch gives an ID to each loaded BTF. The ID is allocated by
the idr like the existing prog-id and map-id.
The bpf_put(map->btf) is moved to __bpf_map_put() so that the
userspace can stop seeing the BTF ID ASAP when the last BTF
refcnt is gone.
It also makes BTF accessible from userspace through the
1. new BPF_BTF_GET_FD_BY_ID command. It is limited to CAP_SYS_ADMIN
which is inline with the BPF_BTF_LOAD cmd and the existing
BPF_[MAP|PROG]_GET_FD_BY_ID cmd.
2. new btf_id (and btf_key_id + btf_value_id) in "struct bpf_map_info"
Once the BTF ID handler is accessible from userspace, freeing a BTF
object has to go through a rcu period. The BPF_BTF_GET_FD_BY_ID cmd
can then be done under a rcu_read_lock() instead of taking
spin_lock.
[Note: A similar rcu usage can be done to the existing
bpf_prog_get_fd_by_id() in a follow up patch]
When processing the BPF_BTF_GET_FD_BY_ID cmd,
refcount_inc_not_zero() is needed because the BTF object
could be already in the rcu dead row . btf_get() is
removed since its usage is currently limited to btf.c
alone. refcount_inc() is used directly instead.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
---
include/linux/btf.h | 2 +
include/uapi/linux/bpf.h | 5 +++
kernel/bpf/btf.c | 108 ++++++++++++++++++++++++++++++++++++++++++-----
kernel/bpf/syscall.c | 24 ++++++++++-
4 files changed, 128 insertions(+), 11 deletions(-)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index a966dc6d61ee..e076c4697049 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -44,5 +44,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
u32 *ret_size);
void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
struct seq_file *m);
+int btf_get_fd_by_id(u32 id);
+u32 btf_id(const struct btf *btf);
#endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 93d5a4eeec2a..6106f23a9a8a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -96,6 +96,7 @@ enum bpf_cmd {
BPF_PROG_QUERY,
BPF_RAW_TRACEPOINT_OPEN,
BPF_BTF_LOAD,
+ BPF_BTF_GET_FD_BY_ID,
};
enum bpf_map_type {
@@ -344,6 +345,7 @@ union bpf_attr {
__u32 start_id;
__u32 prog_id;
__u32 map_id;
+ __u32 btf_id;
};
__u32 next_id;
__u32 open_flags;
@@ -2130,6 +2132,9 @@ struct bpf_map_info {
__u32 ifindex;
__u64 netns_dev;
__u64 netns_ino;
+ __u32 btf_id;
+ __u32 btf_key_id;
+ __u32 btf_value_id;
} __attribute__((aligned(8)));
/* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index fa0dce0452e7..40950b6bf395 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -11,6 +11,7 @@
#include <linux/file.h>
#include <linux/uaccess.h>
#include <linux/kernel.h>
+#include <linux/idr.h>
#include <linux/bpf_verifier.h>
#include <linux/btf.h>
@@ -179,6 +180,9 @@
i < btf_type_vlen(struct_type); \
i++, member++)
+static DEFINE_IDR(btf_idr);
+static DEFINE_SPINLOCK(btf_idr_lock);
+
struct btf {
union {
struct btf_header *hdr;
@@ -193,6 +197,8 @@ struct btf {
u32 types_size;
u32 data_size;
refcount_t refcnt;
+ u32 id;
+ struct rcu_head rcu;
};
enum verifier_phase {
@@ -598,6 +604,42 @@ static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
return 0;
}
+static int btf_alloc_id(struct btf *btf)
+{
+ int id;
+
+ idr_preload(GFP_KERNEL);
+ spin_lock_bh(&btf_idr_lock);
+ id = idr_alloc_cyclic(&btf_idr, btf, 1, INT_MAX, GFP_ATOMIC);
+ if (id > 0)
+ btf->id = id;
+ spin_unlock_bh(&btf_idr_lock);
+ idr_preload_end();
+
+ if (WARN_ON_ONCE(!id))
+ return -ENOSPC;
+
+ return id > 0 ? 0 : id;
+}
+
+static void btf_free_id(struct btf *btf)
+{
+ unsigned long flags;
+
+ /*
+ * In map-in-map, calling map_delete_elem() on outer
+ * map will call bpf_map_put on the inner map.
+ * It will then eventually call btf_free_id()
+ * on the inner map. Some of the map_delete_elem()
+ * implementation may have irq disabled, so
+ * we need to use the _irqsave() version instead
+ * of the _bh() version.
+ */
+ spin_lock_irqsave(&btf_idr_lock, flags);
+ idr_remove(&btf_idr, btf->id);
+ spin_unlock_irqrestore(&btf_idr_lock, flags);
+}
+
static void btf_free(struct btf *btf)
{
kvfree(btf->types);
@@ -607,15 +649,19 @@ static void btf_free(struct btf *btf)
kfree(btf);
}
-static void btf_get(struct btf *btf)
+static void btf_free_rcu(struct rcu_head *rcu)
{
- refcount_inc(&btf->refcnt);
+ struct btf *btf = container_of(rcu, struct btf, rcu);
+
+ btf_free(btf);
}
void btf_put(struct btf *btf)
{
- if (btf && refcount_dec_and_test(&btf->refcnt))
- btf_free(btf);
+ if (btf && refcount_dec_and_test(&btf->refcnt)) {
+ btf_free_id(btf);
+ call_rcu(&btf->rcu, btf_free_rcu);
+ }
}
static int env_resolve_init(struct btf_verifier_env *env)
@@ -2006,10 +2052,15 @@ const struct file_operations btf_fops = {
.release = btf_release,
};
+static int __btf_new_fd(struct btf *btf)
+{
+ return anon_inode_getfd("btf", &btf_fops, btf, O_RDONLY | O_CLOEXEC);
+}
+
int btf_new_fd(const union bpf_attr *attr)
{
struct btf *btf;
- int fd;
+ int ret;
btf = btf_parse(u64_to_user_ptr(attr->btf),
attr->btf_size, attr->btf_log_level,
@@ -2018,12 +2069,23 @@ int btf_new_fd(const union bpf_attr *attr)
if (IS_ERR(btf))
return PTR_ERR(btf);
- fd = anon_inode_getfd("btf", &btf_fops, btf,
- O_RDONLY | O_CLOEXEC);
- if (fd < 0)
+ ret = btf_alloc_id(btf);
+ if (ret) {
+ btf_free(btf);
+ return ret;
+ }
+
+ /*
+ * The BTF ID is published to the userspace.
+ * All BTF free must go through call_rcu() from
+ * now on (i.e. free by calling btf_put()).
+ */
+
+ ret = __btf_new_fd(btf);
+ if (ret < 0)
btf_put(btf);
- return fd;
+ return ret;
}
struct btf *btf_get_by_fd(int fd)
@@ -2042,7 +2104,7 @@ struct btf *btf_get_by_fd(int fd)
}
btf = f.file->private_data;
- btf_get(btf);
+ refcount_inc(&btf->refcnt);
fdput(f);
return btf;
@@ -2062,3 +2124,29 @@ int btf_get_info_by_fd(const struct btf *btf,
return 0;
}
+
+int btf_get_fd_by_id(u32 id)
+{
+ struct btf *btf;
+ int fd;
+
+ rcu_read_lock();
+ btf = idr_find(&btf_idr, id);
+ if (!btf || !refcount_inc_not_zero(&btf->refcnt))
+ btf = ERR_PTR(-ENOENT);
+ rcu_read_unlock();
+
+ if (IS_ERR(btf))
+ return PTR_ERR(btf);
+
+ fd = __btf_new_fd(btf);
+ if (fd < 0)
+ btf_put(btf);
+
+ return fd;
+}
+
+u32 btf_id(const struct btf *btf)
+{
+ return btf->id;
+}
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 263e13ede029..8b0a45d65454 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -252,7 +252,6 @@ static void bpf_map_free_deferred(struct work_struct *work)
bpf_map_uncharge_memlock(map);
security_bpf_map_free(map);
- btf_put(map->btf);
/* implementation dependent freeing */
map->ops->map_free(map);
}
@@ -273,6 +272,7 @@ static void __bpf_map_put(struct bpf_map *map, bool do_idr_lock)
if (atomic_dec_and_test(&map->refcnt)) {
/* bpf_map_free_id() must be called first */
bpf_map_free_id(map, do_idr_lock);
+ btf_put(map->btf);
INIT_WORK(&map->work, bpf_map_free_deferred);
schedule_work(&map->work);
}
@@ -2000,6 +2000,12 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
info.map_flags = map->map_flags;
memcpy(info.name, map->name, sizeof(map->name));
+ if (map->btf) {
+ info.btf_id = btf_id(map->btf);
+ info.btf_key_id = map->btf_key_id;
+ info.btf_value_id = map->btf_value_id;
+ }
+
if (bpf_map_is_dev_bound(map)) {
err = bpf_map_offload_info_fill(&info, map);
if (err)
@@ -2057,6 +2063,19 @@ static int bpf_btf_load(const union bpf_attr *attr)
return btf_new_fd(attr);
}
+#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id
+
+static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
+{
+ if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
+ return -EINVAL;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ return btf_get_fd_by_id(attr->btf_id);
+}
+
SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
{
union bpf_attr attr = {};
@@ -2140,6 +2159,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_BTF_LOAD:
err = bpf_btf_load(&attr);
break;
+ case BPF_BTF_GET_FD_BY_ID:
+ err = bpf_btf_get_fd_by_id(&attr);
+ break;
default:
err = -EINVAL;
break;
--
2.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH bpf-next 2/6] bpf: btf: Introduce BTF ID
2018-05-04 21:49 ` [PATCH bpf-next 2/6] bpf: btf: Introduce BTF ID Martin KaFai Lau
@ 2018-05-08 21:09 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2018-05-08 21:09 UTC (permalink / raw)
To: Martin Lau
Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Kernel Team
> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>
> This patch gives an ID to each loaded BTF. The ID is allocated by
> the idr like the existing prog-id and map-id.
>
> The bpf_put(map->btf) is moved to __bpf_map_put() so that the
> userspace can stop seeing the BTF ID ASAP when the last BTF
> refcnt is gone.
>
> It also makes BTF accessible from userspace through the
> 1. new BPF_BTF_GET_FD_BY_ID command. It is limited to CAP_SYS_ADMIN
> which is inline with the BPF_BTF_LOAD cmd and the existing
> BPF_[MAP|PROG]_GET_FD_BY_ID cmd.
> 2. new btf_id (and btf_key_id + btf_value_id) in "struct bpf_map_info"
>
> Once the BTF ID handler is accessible from userspace, freeing a BTF
> object has to go through a rcu period. The BPF_BTF_GET_FD_BY_ID cmd
> can then be done under a rcu_read_lock() instead of taking
> spin_lock.
> [Note: A similar rcu usage can be done to the existing
> bpf_prog_get_fd_by_id() in a follow up patch]
>
> When processing the BPF_BTF_GET_FD_BY_ID cmd,
> refcount_inc_not_zero() is needed because the BTF object
> could be already in the rcu dead row . btf_get() is
> removed since its usage is currently limited to btf.c
> alone. refcount_inc() is used directly instead.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> include/linux/btf.h | 2 +
> include/uapi/linux/bpf.h | 5 +++
> kernel/bpf/btf.c | 108 ++++++++++++++++++++++++++++++++++++++++++-----
> kernel/bpf/syscall.c | 24 ++++++++++-
> 4 files changed, 128 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index a966dc6d61ee..e076c4697049 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -44,5 +44,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
> u32 *ret_size);
> void btf_type_seq_show(const struct btf *btf, u32 type_id, void *obj,
> struct seq_file *m);
> +int btf_get_fd_by_id(u32 id);
> +u32 btf_id(const struct btf *btf);
>
> #endif
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 93d5a4eeec2a..6106f23a9a8a 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -96,6 +96,7 @@ enum bpf_cmd {
> BPF_PROG_QUERY,
> BPF_RAW_TRACEPOINT_OPEN,
> BPF_BTF_LOAD,
> + BPF_BTF_GET_FD_BY_ID,
> };
>
> enum bpf_map_type {
> @@ -344,6 +345,7 @@ union bpf_attr {
> __u32 start_id;
> __u32 prog_id;
> __u32 map_id;
> + __u32 btf_id;
> };
> __u32 next_id;
> __u32 open_flags;
> @@ -2130,6 +2132,9 @@ struct bpf_map_info {
> __u32 ifindex;
> __u64 netns_dev;
> __u64 netns_ino;
> + __u32 btf_id;
> + __u32 btf_key_id;
> + __u32 btf_value_id;
> } __attribute__((aligned(8)));
>
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index fa0dce0452e7..40950b6bf395 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -11,6 +11,7 @@
> #include <linux/file.h>
> #include <linux/uaccess.h>
> #include <linux/kernel.h>
> +#include <linux/idr.h>
> #include <linux/bpf_verifier.h>
> #include <linux/btf.h>
>
> @@ -179,6 +180,9 @@
> i < btf_type_vlen(struct_type); \
> i++, member++)
>
> +static DEFINE_IDR(btf_idr);
> +static DEFINE_SPINLOCK(btf_idr_lock);
> +
> struct btf {
> union {
> struct btf_header *hdr;
> @@ -193,6 +197,8 @@ struct btf {
> u32 types_size;
> u32 data_size;
> refcount_t refcnt;
> + u32 id;
> + struct rcu_head rcu;
> };
>
> enum verifier_phase {
> @@ -598,6 +604,42 @@ static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
> return 0;
> }
>
> +static int btf_alloc_id(struct btf *btf)
> +{
> + int id;
> +
> + idr_preload(GFP_KERNEL);
> + spin_lock_bh(&btf_idr_lock);
> + id = idr_alloc_cyclic(&btf_idr, btf, 1, INT_MAX, GFP_ATOMIC);
> + if (id > 0)
> + btf->id = id;
> + spin_unlock_bh(&btf_idr_lock);
> + idr_preload_end();
> +
> + if (WARN_ON_ONCE(!id))
> + return -ENOSPC;
> +
> + return id > 0 ? 0 : id;
> +}
> +
> +static void btf_free_id(struct btf *btf)
> +{
> + unsigned long flags;
> +
> + /*
> + * In map-in-map, calling map_delete_elem() on outer
> + * map will call bpf_map_put on the inner map.
> + * It will then eventually call btf_free_id()
> + * on the inner map. Some of the map_delete_elem()
> + * implementation may have irq disabled, so
> + * we need to use the _irqsave() version instead
> + * of the _bh() version.
> + */
> + spin_lock_irqsave(&btf_idr_lock, flags);
> + idr_remove(&btf_idr, btf->id);
> + spin_unlock_irqrestore(&btf_idr_lock, flags);
> +}
> +
> static void btf_free(struct btf *btf)
> {
> kvfree(btf->types);
> @@ -607,15 +649,19 @@ static void btf_free(struct btf *btf)
> kfree(btf);
> }
>
> -static void btf_get(struct btf *btf)
> +static void btf_free_rcu(struct rcu_head *rcu)
> {
> - refcount_inc(&btf->refcnt);
> + struct btf *btf = container_of(rcu, struct btf, rcu);
> +
> + btf_free(btf);
> }
>
> void btf_put(struct btf *btf)
> {
> - if (btf && refcount_dec_and_test(&btf->refcnt))
> - btf_free(btf);
> + if (btf && refcount_dec_and_test(&btf->refcnt)) {
> + btf_free_id(btf);
> + call_rcu(&btf->rcu, btf_free_rcu);
> + }
> }
>
> static int env_resolve_init(struct btf_verifier_env *env)
> @@ -2006,10 +2052,15 @@ const struct file_operations btf_fops = {
> .release = btf_release,
> };
>
> +static int __btf_new_fd(struct btf *btf)
> +{
> + return anon_inode_getfd("btf", &btf_fops, btf, O_RDONLY | O_CLOEXEC);
> +}
> +
> int btf_new_fd(const union bpf_attr *attr)
> {
> struct btf *btf;
> - int fd;
> + int ret;
>
> btf = btf_parse(u64_to_user_ptr(attr->btf),
> attr->btf_size, attr->btf_log_level,
> @@ -2018,12 +2069,23 @@ int btf_new_fd(const union bpf_attr *attr)
> if (IS_ERR(btf))
> return PTR_ERR(btf);
>
> - fd = anon_inode_getfd("btf", &btf_fops, btf,
> - O_RDONLY | O_CLOEXEC);
> - if (fd < 0)
> + ret = btf_alloc_id(btf);
> + if (ret) {
> + btf_free(btf);
> + return ret;
> + }
> +
> + /*
> + * The BTF ID is published to the userspace.
> + * All BTF free must go through call_rcu() from
> + * now on (i.e. free by calling btf_put()).
> + */
> +
> + ret = __btf_new_fd(btf);
> + if (ret < 0)
> btf_put(btf);
>
> - return fd;
> + return ret;
> }
>
> struct btf *btf_get_by_fd(int fd)
> @@ -2042,7 +2104,7 @@ struct btf *btf_get_by_fd(int fd)
> }
>
> btf = f.file->private_data;
> - btf_get(btf);
> + refcount_inc(&btf->refcnt);
> fdput(f);
>
> return btf;
> @@ -2062,3 +2124,29 @@ int btf_get_info_by_fd(const struct btf *btf,
>
> return 0;
> }
> +
> +int btf_get_fd_by_id(u32 id)
> +{
> + struct btf *btf;
> + int fd;
> +
> + rcu_read_lock();
> + btf = idr_find(&btf_idr, id);
> + if (!btf || !refcount_inc_not_zero(&btf->refcnt))
> + btf = ERR_PTR(-ENOENT);
> + rcu_read_unlock();
> +
> + if (IS_ERR(btf))
> + return PTR_ERR(btf);
> +
> + fd = __btf_new_fd(btf);
> + if (fd < 0)
> + btf_put(btf);
> +
> + return fd;
> +}
> +
> +u32 btf_id(const struct btf *btf)
> +{
> + return btf->id;
> +}
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 263e13ede029..8b0a45d65454 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -252,7 +252,6 @@ static void bpf_map_free_deferred(struct work_struct *work)
>
> bpf_map_uncharge_memlock(map);
> security_bpf_map_free(map);
> - btf_put(map->btf);
> /* implementation dependent freeing */
> map->ops->map_free(map);
> }
> @@ -273,6 +272,7 @@ static void __bpf_map_put(struct bpf_map *map, bool do_idr_lock)
> if (atomic_dec_and_test(&map->refcnt)) {
> /* bpf_map_free_id() must be called first */
> bpf_map_free_id(map, do_idr_lock);
> + btf_put(map->btf);
> INIT_WORK(&map->work, bpf_map_free_deferred);
> schedule_work(&map->work);
> }
> @@ -2000,6 +2000,12 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
> info.map_flags = map->map_flags;
> memcpy(info.name, map->name, sizeof(map->name));
>
> + if (map->btf) {
> + info.btf_id = btf_id(map->btf);
> + info.btf_key_id = map->btf_key_id;
> + info.btf_value_id = map->btf_value_id;
> + }
> +
> if (bpf_map_is_dev_bound(map)) {
> err = bpf_map_offload_info_fill(&info, map);
> if (err)
> @@ -2057,6 +2063,19 @@ static int bpf_btf_load(const union bpf_attr *attr)
> return btf_new_fd(attr);
> }
>
> +#define BPF_BTF_GET_FD_BY_ID_LAST_FIELD btf_id
> +
> +static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
> +{
> + if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
> + return -EINVAL;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return btf_get_fd_by_id(attr->btf_id);
> +}
> +
> SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
> {
> union bpf_attr attr = {};
> @@ -2140,6 +2159,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
> case BPF_BTF_LOAD:
> err = bpf_btf_load(&attr);
> break;
> + case BPF_BTF_GET_FD_BY_ID:
> + err = bpf_btf_get_fd_by_id(&attr);
> + break;
> default:
> err = -EINVAL;
> break;
> --
> 2.9.5
>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf-next 3/6] bpf: btf: Add struct bpf_btf_info
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
2018-05-04 21:49 ` [PATCH bpf-next 1/6] bpf: btf: Avoid WARN_ON when CONFIG_REFCOUNT_FULL=y Martin KaFai Lau
2018-05-04 21:49 ` [PATCH bpf-next 2/6] bpf: btf: Introduce BTF ID Martin KaFai Lau
@ 2018-05-04 21:49 ` Martin KaFai Lau
2018-05-08 21:09 ` Song Liu
2018-05-04 21:49 ` [PATCH bpf-next 4/6] bpf: btf: Some test_btf clean up Martin KaFai Lau
` (3 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2018-05-04 21:49 UTC (permalink / raw)
To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
During BPF_OBJ_GET_INFO_BY_FD on a btf_fd, the current bpf_attr's
info.info is directly filled with the BTF binary data. It is
not extensible. In this case, we want to add BTF ID.
This patch adds "struct bpf_btf_info" which has the BTF ID as
one of its member. The BTF binary data itself is exposed through
the "btf" and "btf_size" members.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
---
include/uapi/linux/bpf.h | 6 ++++++
kernel/bpf/btf.c | 26 +++++++++++++++++++++-----
kernel/bpf/syscall.c | 17 ++++++++++++++++-
3 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 6106f23a9a8a..d615c777b573 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2137,6 +2137,12 @@ struct bpf_map_info {
__u32 btf_value_id;
} __attribute__((aligned(8)));
+struct bpf_btf_info {
+ __aligned_u64 btf;
+ __u32 btf_size;
+ __u32 id;
+} __attribute__((aligned(8)));
+
/* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
* by user and intended to be used by socket (e.g. to bind to, depends on
* attach attach type).
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 40950b6bf395..ded10ab47b8a 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2114,12 +2114,28 @@ int btf_get_info_by_fd(const struct btf *btf,
const union bpf_attr *attr,
union bpf_attr __user *uattr)
{
- void __user *udata = u64_to_user_ptr(attr->info.info);
- u32 copy_len = min_t(u32, btf->data_size,
- attr->info.info_len);
+ struct bpf_btf_info __user *uinfo;
+ struct bpf_btf_info info = {};
+ u32 info_copy, btf_copy;
+ void __user *ubtf;
+ u32 uinfo_len;
- if (copy_to_user(udata, btf->data, copy_len) ||
- put_user(btf->data_size, &uattr->info.info_len))
+ uinfo = u64_to_user_ptr(attr->info.info);
+ uinfo_len = attr->info.info_len;
+
+ info_copy = min_t(u32, uinfo_len, sizeof(info));
+ if (copy_from_user(&info, uinfo, info_copy))
+ return -EFAULT;
+
+ info.id = btf->id;
+ ubtf = u64_to_user_ptr(info.btf);
+ btf_copy = min_t(u32, btf->data_size, info.btf_size);
+ if (copy_to_user(ubtf, btf->data, btf_copy))
+ return -EFAULT;
+ info.btf_size = btf->data_size;
+
+ if (copy_to_user(uinfo, &info, info_copy) ||
+ put_user(info_copy, &uattr->info.info_len))
return -EFAULT;
return 0;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8b0a45d65454..d2895e3e5cbf 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2019,6 +2019,21 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
return 0;
}
+static int bpf_btf_get_info_by_fd(struct btf *btf,
+ const union bpf_attr *attr,
+ union bpf_attr __user *uattr)
+{
+ struct bpf_btf_info __user *uinfo = u64_to_user_ptr(attr->info.info);
+ u32 info_len = attr->info.info_len;
+ int err;
+
+ err = check_uarg_tail_zero(uinfo, sizeof(*uinfo), info_len);
+ if (err)
+ return err;
+
+ return btf_get_info_by_fd(btf, attr, uattr);
+}
+
#define BPF_OBJ_GET_INFO_BY_FD_LAST_FIELD info.info
static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
@@ -2042,7 +2057,7 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
err = bpf_map_get_info_by_fd(f.file->private_data, attr,
uattr);
else if (f.file->f_op == &btf_fops)
- err = btf_get_info_by_fd(f.file->private_data, attr, uattr);
+ err = bpf_btf_get_info_by_fd(f.file->private_data, attr, uattr);
else
err = -EINVAL;
--
2.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH bpf-next 3/6] bpf: btf: Add struct bpf_btf_info
2018-05-04 21:49 ` [PATCH bpf-next 3/6] bpf: btf: Add struct bpf_btf_info Martin KaFai Lau
@ 2018-05-08 21:09 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2018-05-08 21:09 UTC (permalink / raw)
To: Martin Lau
Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Kernel Team
> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>
> During BPF_OBJ_GET_INFO_BY_FD on a btf_fd, the current bpf_attr's
> info.info is directly filled with the BTF binary data. It is
> not extensible. In this case, we want to add BTF ID.
>
> This patch adds "struct bpf_btf_info" which has the BTF ID as
> one of its member. The BTF binary data itself is exposed through
> the "btf" and "btf_size" members.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> include/uapi/linux/bpf.h | 6 ++++++
> kernel/bpf/btf.c | 26 +++++++++++++++++++++-----
> kernel/bpf/syscall.c | 17 ++++++++++++++++-
> 3 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 6106f23a9a8a..d615c777b573 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2137,6 +2137,12 @@ struct bpf_map_info {
> __u32 btf_value_id;
> } __attribute__((aligned(8)));
>
> +struct bpf_btf_info {
> + __aligned_u64 btf;
> + __u32 btf_size;
> + __u32 id;
> +} __attribute__((aligned(8)));
> +
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> * by user and intended to be used by socket (e.g. to bind to, depends on
> * attach attach type).
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 40950b6bf395..ded10ab47b8a 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2114,12 +2114,28 @@ int btf_get_info_by_fd(const struct btf *btf,
> const union bpf_attr *attr,
> union bpf_attr __user *uattr)
> {
> - void __user *udata = u64_to_user_ptr(attr->info.info);
> - u32 copy_len = min_t(u32, btf->data_size,
> - attr->info.info_len);
> + struct bpf_btf_info __user *uinfo;
> + struct bpf_btf_info info = {};
> + u32 info_copy, btf_copy;
> + void __user *ubtf;
> + u32 uinfo_len;
>
> - if (copy_to_user(udata, btf->data, copy_len) ||
> - put_user(btf->data_size, &uattr->info.info_len))
> + uinfo = u64_to_user_ptr(attr->info.info);
> + uinfo_len = attr->info.info_len;
> +
> + info_copy = min_t(u32, uinfo_len, sizeof(info));
> + if (copy_from_user(&info, uinfo, info_copy))
> + return -EFAULT;
> +
> + info.id = btf->id;
> + ubtf = u64_to_user_ptr(info.btf);
> + btf_copy = min_t(u32, btf->data_size, info.btf_size);
> + if (copy_to_user(ubtf, btf->data, btf_copy))
> + return -EFAULT;
> + info.btf_size = btf->data_size;
> +
> + if (copy_to_user(uinfo, &info, info_copy) ||
> + put_user(info_copy, &uattr->info.info_len))
> return -EFAULT;
>
> return 0;
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 8b0a45d65454..d2895e3e5cbf 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2019,6 +2019,21 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
> return 0;
> }
>
> +static int bpf_btf_get_info_by_fd(struct btf *btf,
> + const union bpf_attr *attr,
> + union bpf_attr __user *uattr)
> +{
> + struct bpf_btf_info __user *uinfo = u64_to_user_ptr(attr->info.info);
> + u32 info_len = attr->info.info_len;
> + int err;
> +
> + err = check_uarg_tail_zero(uinfo, sizeof(*uinfo), info_len);
> + if (err)
> + return err;
> +
> + return btf_get_info_by_fd(btf, attr, uattr);
> +}
> +
> #define BPF_OBJ_GET_INFO_BY_FD_LAST_FIELD info.info
>
> static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
> @@ -2042,7 +2057,7 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr,
> err = bpf_map_get_info_by_fd(f.file->private_data, attr,
> uattr);
> else if (f.file->f_op == &btf_fops)
> - err = btf_get_info_by_fd(f.file->private_data, attr, uattr);
> + err = bpf_btf_get_info_by_fd(f.file->private_data, attr, uattr);
> else
> err = -EINVAL;
>
> --
> 2.9.5
>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf-next 4/6] bpf: btf: Some test_btf clean up
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
` (2 preceding siblings ...)
2018-05-04 21:49 ` [PATCH bpf-next 3/6] bpf: btf: Add struct bpf_btf_info Martin KaFai Lau
@ 2018-05-04 21:49 ` Martin KaFai Lau
2018-05-08 21:18 ` Song Liu
2018-05-04 21:49 ` [PATCH bpf-next 5/6] bpf: btf: Update tools/include/uapi/linux/btf.h with BTF ID Martin KaFai Lau
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2018-05-04 21:49 UTC (permalink / raw)
To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
This patch adds a CHECK() macro for condition checking
and error report purpose. Something similar to test_progs.c
It also counts the number of tests passed/skipped/failed and
print them at the end of the test run.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
---
tools/testing/selftests/bpf/test_btf.c | 201 ++++++++++++++++-----------------
1 file changed, 99 insertions(+), 102 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index 7b39b1f712a1..b7880a20fad1 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -20,6 +20,30 @@
#include "bpf_rlimit.h"
+static uint32_t pass_cnt;
+static uint32_t error_cnt;
+static uint32_t skip_cnt;
+
+#define CHECK(condition, format...) ({ \
+ int __ret = !!(condition); \
+ if (__ret) { \
+ fprintf(stderr, "%s:%d:FAIL ", __func__, __LINE__); \
+ fprintf(stderr, format); \
+ } \
+ __ret; \
+})
+
+static int count_result(int err)
+{
+ if (err)
+ error_cnt++;
+ else
+ pass_cnt++;
+
+ fprintf(stderr, "\n");
+ return err;
+}
+
#define min(a, b) ((a) < (b) ? (a) : (b))
#define __printf(a, b) __attribute__((format(printf, a, b)))
@@ -894,17 +918,13 @@ static void *btf_raw_create(const struct btf_header *hdr,
void *raw_btf;
type_sec_size = get_type_sec_size(raw_types);
- if (type_sec_size < 0) {
- fprintf(stderr, "Cannot get nr_raw_types\n");
+ if (CHECK(type_sec_size < 0, "Cannot get nr_raw_types"))
return NULL;
- }
size_needed = sizeof(*hdr) + type_sec_size + str_sec_size;
raw_btf = malloc(size_needed);
- if (!raw_btf) {
- fprintf(stderr, "Cannot allocate memory for raw_btf\n");
+ if (CHECK(!raw_btf, "Cannot allocate memory for raw_btf"))
return NULL;
- }
/* Copy header */
memcpy(raw_btf, hdr, sizeof(*hdr));
@@ -915,8 +935,7 @@ static void *btf_raw_create(const struct btf_header *hdr,
for (i = 0; i < type_sec_size / sizeof(raw_types[0]); i++) {
if (raw_types[i] == NAME_TBD) {
next_str = get_next_str(next_str, end_str);
- if (!next_str) {
- fprintf(stderr, "Error in getting next_str\n");
+ if (CHECK(!next_str, "Error in getting next_str")) {
free(raw_btf);
return NULL;
}
@@ -973,9 +992,8 @@ static int do_test_raw(unsigned int test_num)
free(raw_btf);
err = ((btf_fd == -1) != test->btf_load_err);
- if (err)
- fprintf(stderr, "btf_load_err:%d btf_fd:%d\n",
- test->btf_load_err, btf_fd);
+ CHECK(err, "btf_fd:%d test->btf_load_err:%u",
+ btf_fd, test->btf_load_err);
if (err || btf_fd == -1)
goto done;
@@ -992,16 +1010,15 @@ static int do_test_raw(unsigned int test_num)
map_fd = bpf_create_map_xattr(&create_attr);
err = ((map_fd == -1) != test->map_create_err);
- if (err)
- fprintf(stderr, "map_create_err:%d map_fd:%d\n",
- test->map_create_err, map_fd);
+ CHECK(err, "map_fd:%d test->map_create_err:%u",
+ map_fd, test->map_create_err);
done:
if (!err)
- fprintf(stderr, "OK\n");
+ fprintf(stderr, "OK");
if (*btf_log_buf && (err || args.always_log))
- fprintf(stderr, "%s\n", btf_log_buf);
+ fprintf(stderr, "\n%s", btf_log_buf);
if (btf_fd != -1)
close(btf_fd);
@@ -1017,10 +1034,10 @@ static int test_raw(void)
int err = 0;
if (args.raw_test_num)
- return do_test_raw(args.raw_test_num);
+ return count_result(do_test_raw(args.raw_test_num));
for (i = 1; i <= ARRAY_SIZE(raw_tests); i++)
- err |= do_test_raw(i);
+ err |= count_result(do_test_raw(i));
return err;
}
@@ -1080,8 +1097,7 @@ static int do_test_get_info(unsigned int test_num)
*btf_log_buf = '\0';
user_btf = malloc(raw_btf_size);
- if (!user_btf) {
- fprintf(stderr, "Cannot allocate memory for user_btf\n");
+ if (CHECK(!user_btf, "!user_btf")) {
err = -1;
goto done;
}
@@ -1089,9 +1105,7 @@ static int do_test_get_info(unsigned int test_num)
btf_fd = bpf_load_btf(raw_btf, raw_btf_size,
btf_log_buf, BTF_LOG_BUF_SIZE,
args.always_log);
- if (btf_fd == -1) {
- fprintf(stderr, "bpf_load_btf:%s(%d)\n",
- strerror(errno), errno);
+ if (CHECK(btf_fd == -1, "errno:%d", errno)) {
err = -1;
goto done;
}
@@ -1103,31 +1117,31 @@ static int do_test_get_info(unsigned int test_num)
raw_btf_size - expected_nbytes);
err = bpf_obj_get_info_by_fd(btf_fd, user_btf, &user_btf_size);
- if (err || user_btf_size != raw_btf_size ||
- memcmp(raw_btf, user_btf, expected_nbytes)) {
- fprintf(stderr,
- "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d\n",
- err, errno,
- raw_btf_size, user_btf_size, expected_nbytes,
- memcmp(raw_btf, user_btf, expected_nbytes));
+ if (CHECK(err || user_btf_size != raw_btf_size ||
+ memcmp(raw_btf, user_btf, expected_nbytes),
+ "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d",
+ err, errno,
+ raw_btf_size, user_btf_size, expected_nbytes,
+ memcmp(raw_btf, user_btf, expected_nbytes))) {
err = -1;
goto done;
}
while (expected_nbytes < raw_btf_size) {
fprintf(stderr, "%u...", expected_nbytes);
- if (user_btf[expected_nbytes++] != 0xff) {
- fprintf(stderr, "!= 0xff\n");
+ if (CHECK(user_btf[expected_nbytes++] != 0xff,
+ "user_btf[%u]:%x != 0xff", expected_nbytes - 1,
+ user_btf[expected_nbytes - 1])) {
err = -1;
goto done;
}
}
- fprintf(stderr, "OK\n");
+ fprintf(stderr, "OK");
done:
if (*btf_log_buf && (err || args.always_log))
- fprintf(stderr, "%s\n", btf_log_buf);
+ fprintf(stderr, "\n%s", btf_log_buf);
free(raw_btf);
free(user_btf);
@@ -1144,10 +1158,10 @@ static int test_get_info(void)
int err = 0;
if (args.get_info_test_num)
- return do_test_get_info(args.get_info_test_num);
+ return count_result(do_test_get_info(args.get_info_test_num));
for (i = 1; i <= ARRAY_SIZE(get_info_tests); i++)
- err |= do_test_get_info(i);
+ err |= count_result(do_test_get_info(i));
return err;
}
@@ -1175,28 +1189,21 @@ static int file_has_btf_elf(const char *fn)
Elf *elf;
int ret;
- if (elf_version(EV_CURRENT) == EV_NONE) {
- fprintf(stderr, "Failed to init libelf\n");
+ if (CHECK(elf_version(EV_CURRENT) == EV_NONE,
+ "elf_version(EV_CURRENT) == EV_NONE"))
return -1;
- }
elf_fd = open(fn, O_RDONLY);
- if (elf_fd == -1) {
- fprintf(stderr, "Cannot open file %s: %s(%d)\n",
- fn, strerror(errno), errno);
+ if (CHECK(elf_fd == -1, "open(%s): errno:%d", fn, errno))
return -1;
- }
elf = elf_begin(elf_fd, ELF_C_READ, NULL);
- if (!elf) {
- fprintf(stderr, "Failed to read ELF from %s. %s\n", fn,
- elf_errmsg(elf_errno()));
+ if (CHECK(!elf, "elf_begin(%s): %s", fn, elf_errmsg(elf_errno()))) {
ret = -1;
goto done;
}
- if (!gelf_getehdr(elf, &ehdr)) {
- fprintf(stderr, "Failed to get EHDR from %s\n", fn);
+ if (CHECK(!gelf_getehdr(elf, &ehdr), "!gelf_getehdr(%s)", fn)) {
ret = -1;
goto done;
}
@@ -1205,9 +1212,8 @@ static int file_has_btf_elf(const char *fn)
const char *sh_name;
GElf_Shdr sh;
- if (gelf_getshdr(scn, &sh) != &sh) {
- fprintf(stderr,
- "Failed to get section header from %s\n", fn);
+ if (CHECK(gelf_getshdr(scn, &sh) != &sh,
+ "file:%s gelf_getshdr != &sh", fn)) {
ret = -1;
goto done;
}
@@ -1243,53 +1249,44 @@ static int do_test_file(unsigned int test_num)
return err;
if (err == 0) {
- fprintf(stderr, "SKIP. No ELF %s found\n", BTF_ELF_SEC);
+ fprintf(stderr, "SKIP. No ELF %s found", BTF_ELF_SEC);
+ skip_cnt++;
return 0;
}
obj = bpf_object__open(test->file);
- if (IS_ERR(obj))
+ if (CHECK(IS_ERR(obj), "obj: %ld", PTR_ERR(obj)))
return PTR_ERR(obj);
err = bpf_object__btf_fd(obj);
- if (err == -1) {
- fprintf(stderr, "bpf_object__btf_fd: -1\n");
+ if (CHECK(err == -1, "bpf_object__btf_fd: -1"))
goto done;
- }
prog = bpf_program__next(NULL, obj);
- if (!prog) {
- fprintf(stderr, "Cannot find bpf_prog\n");
+ if (CHECK(!prog, "Cannot find bpf_prog")) {
err = -1;
goto done;
}
bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
err = bpf_object__load(obj);
- if (err < 0) {
- fprintf(stderr, "bpf_object__load: %d\n", err);
+ if (CHECK(err < 0, "bpf_object__load: %d", err))
goto done;
- }
map = bpf_object__find_map_by_name(obj, "btf_map");
- if (!map) {
- fprintf(stderr, "btf_map not found\n");
+ if (CHECK(!map, "btf_map not found")) {
err = -1;
goto done;
}
err = (bpf_map__btf_key_id(map) == 0 || bpf_map__btf_value_id(map) == 0)
!= test->btf_kv_notfound;
- if (err) {
- fprintf(stderr,
- "btf_kv_notfound:%u btf_key_id:%u btf_value_id:%u\n",
- test->btf_kv_notfound,
- bpf_map__btf_key_id(map),
- bpf_map__btf_value_id(map));
+ if (CHECK(err, "btf_key_id:%u btf_value_id:%u test->btf_kv_notfound:%u",
+ bpf_map__btf_key_id(map), bpf_map__btf_value_id(map),
+ test->btf_kv_notfound))
goto done;
- }
- fprintf(stderr, "OK\n");
+ fprintf(stderr, "OK");
done:
bpf_object__close(obj);
@@ -1302,10 +1299,10 @@ static int test_file(void)
int err = 0;
if (args.file_test_num)
- return do_test_file(args.file_test_num);
+ return count_result(do_test_file(args.file_test_num));
for (i = 1; i <= ARRAY_SIZE(file_tests); i++)
- err |= do_test_file(i);
+ err |= count_result(do_test_file(i));
return err;
}
@@ -1425,7 +1422,7 @@ static int test_pprint(void)
unsigned int key;
uint8_t *raw_btf;
ssize_t nread;
- int err;
+ int err, ret;
fprintf(stderr, "%s......", test->descr);
raw_btf = btf_raw_create(&hdr_tmpl, test->raw_types,
@@ -1441,10 +1438,8 @@ static int test_pprint(void)
args.always_log);
free(raw_btf);
- if (btf_fd == -1) {
+ if (CHECK(btf_fd == -1, "errno:%d", errno)) {
err = -1;
- fprintf(stderr, "bpf_load_btf: %s(%d)\n",
- strerror(errno), errno);
goto done;
}
@@ -1458,26 +1453,23 @@ static int test_pprint(void)
create_attr.btf_value_id = test->value_id;
map_fd = bpf_create_map_xattr(&create_attr);
- if (map_fd == -1) {
+ if (CHECK(map_fd == -1, "errno:%d", errno)) {
err = -1;
- fprintf(stderr, "bpf_creat_map_btf: %s(%d)\n",
- strerror(errno), errno);
goto done;
}
- if (snprintf(pin_path, sizeof(pin_path), "%s/%s",
- "/sys/fs/bpf", test->map_name) == sizeof(pin_path)) {
+ ret = snprintf(pin_path, sizeof(pin_path), "%s/%s",
+ "/sys/fs/bpf", test->map_name);
+
+ if (CHECK(ret == sizeof(pin_path), "pin_path %s/%s is too long",
+ "/sys/fs/bpf", test->map_name)) {
err = -1;
- fprintf(stderr, "pin_path is too long\n");
goto done;
}
err = bpf_obj_pin(map_fd, pin_path);
- if (err) {
- fprintf(stderr, "Cannot pin to %s. %s(%d).\n", pin_path,
- strerror(errno), errno);
+ if (CHECK(err, "bpf_obj_pin(%s): errno:%d.", pin_path, errno))
goto done;
- }
for (key = 0; key < test->max_entries; key++) {
set_pprint_mapv(&mapv, key);
@@ -1485,10 +1477,8 @@ static int test_pprint(void)
}
pin_file = fopen(pin_path, "r");
- if (!pin_file) {
+ if (CHECK(!pin_file, "fopen(%s): errno:%d", pin_path, errno)) {
err = -1;
- fprintf(stderr, "fopen(%s): %s(%d)\n", pin_path,
- strerror(errno), errno);
goto done;
}
@@ -1497,9 +1487,8 @@ static int test_pprint(void)
*line == '#')
;
- if (nread <= 0) {
+ if (CHECK(nread <= 0, "Unexpected EOF")) {
err = -1;
- fprintf(stderr, "Unexpected EOF\n");
goto done;
}
@@ -1518,9 +1507,9 @@ static int test_pprint(void)
mapv.ui8a[4], mapv.ui8a[5], mapv.ui8a[6], mapv.ui8a[7],
pprint_enum_str[mapv.aenum]);
- if (nexpected_line == sizeof(expected_line)) {
+ if (CHECK(nexpected_line == sizeof(expected_line),
+ "expected_line is too long")) {
err = -1;
- fprintf(stderr, "expected_line is too long\n");
goto done;
}
@@ -1535,15 +1524,15 @@ static int test_pprint(void)
nread = getline(&line, &line_len, pin_file);
} while (++key < test->max_entries && nread > 0);
- if (key < test->max_entries) {
+ if (CHECK(key < test->max_entries,
+ "Unexpected EOF. key:%u test->max_entries:%u",
+ key, test->max_entries)) {
err = -1;
- fprintf(stderr, "Unexpected EOF\n");
goto done;
}
- if (nread > 0) {
+ if (CHECK(nread > 0, "Unexpected extra pprint output: %s", line)) {
err = -1;
- fprintf(stderr, "Unexpected extra pprint output: %s\n", line);
goto done;
}
@@ -1551,9 +1540,9 @@ static int test_pprint(void)
done:
if (!err)
- fprintf(stderr, "OK\n");
+ fprintf(stderr, "OK");
if (*btf_log_buf && (err || args.always_log))
- fprintf(stderr, "%s\n", btf_log_buf);
+ fprintf(stderr, "\n%s", btf_log_buf);
if (btf_fd != -1)
close(btf_fd);
if (map_fd != -1)
@@ -1634,6 +1623,12 @@ static int parse_args(int argc, char **argv)
return 0;
}
+static void print_summary(void)
+{
+ fprintf(stderr, "PASS:%u SKIP:%u FAIL:%u\n",
+ pass_cnt - skip_cnt, skip_cnt, error_cnt);
+}
+
int main(int argc, char **argv)
{
int err = 0;
@@ -1655,15 +1650,17 @@ int main(int argc, char **argv)
err |= test_file();
if (args.pprint_test)
- err |= test_pprint();
+ err |= count_result(test_pprint());
if (args.raw_test || args.get_info_test || args.file_test ||
args.pprint_test)
- return err;
+ goto done;
err |= test_raw();
err |= test_get_info();
err |= test_file();
+done:
+ print_summary();
return err;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH bpf-next 4/6] bpf: btf: Some test_btf clean up
2018-05-04 21:49 ` [PATCH bpf-next 4/6] bpf: btf: Some test_btf clean up Martin KaFai Lau
@ 2018-05-08 21:18 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2018-05-08 21:18 UTC (permalink / raw)
To: Martin Lau
Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Kernel Team
> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>
> This patch adds a CHECK() macro for condition checking
> and error report purpose. Something similar to test_progs.c
>
> It also counts the number of tests passed/skipped/failed and
> print them at the end of the test run.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> tools/testing/selftests/bpf/test_btf.c | 201 ++++++++++++++++-----------------
> 1 file changed, 99 insertions(+), 102 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index 7b39b1f712a1..b7880a20fad1 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -20,6 +20,30 @@
>
> #include "bpf_rlimit.h"
>
> +static uint32_t pass_cnt;
> +static uint32_t error_cnt;
> +static uint32_t skip_cnt;
> +
> +#define CHECK(condition, format...) ({ \
> + int __ret = !!(condition); \
> + if (__ret) { \
> + fprintf(stderr, "%s:%d:FAIL ", __func__, __LINE__); \
> + fprintf(stderr, format); \
> + } \
> + __ret; \
> +})
> +
> +static int count_result(int err)
> +{
> + if (err)
> + error_cnt++;
> + else
> + pass_cnt++;
> +
> + fprintf(stderr, "\n");
> + return err;
> +}
> +
> #define min(a, b) ((a) < (b) ? (a) : (b))
> #define __printf(a, b) __attribute__((format(printf, a, b)))
>
> @@ -894,17 +918,13 @@ static void *btf_raw_create(const struct btf_header *hdr,
> void *raw_btf;
>
> type_sec_size = get_type_sec_size(raw_types);
> - if (type_sec_size < 0) {
> - fprintf(stderr, "Cannot get nr_raw_types\n");
> + if (CHECK(type_sec_size < 0, "Cannot get nr_raw_types"))
> return NULL;
> - }
>
> size_needed = sizeof(*hdr) + type_sec_size + str_sec_size;
> raw_btf = malloc(size_needed);
> - if (!raw_btf) {
> - fprintf(stderr, "Cannot allocate memory for raw_btf\n");
> + if (CHECK(!raw_btf, "Cannot allocate memory for raw_btf"))
> return NULL;
> - }
>
> /* Copy header */
> memcpy(raw_btf, hdr, sizeof(*hdr));
> @@ -915,8 +935,7 @@ static void *btf_raw_create(const struct btf_header *hdr,
> for (i = 0; i < type_sec_size / sizeof(raw_types[0]); i++) {
> if (raw_types[i] == NAME_TBD) {
> next_str = get_next_str(next_str, end_str);
> - if (!next_str) {
> - fprintf(stderr, "Error in getting next_str\n");
> + if (CHECK(!next_str, "Error in getting next_str")) {
> free(raw_btf);
> return NULL;
> }
> @@ -973,9 +992,8 @@ static int do_test_raw(unsigned int test_num)
> free(raw_btf);
>
> err = ((btf_fd == -1) != test->btf_load_err);
> - if (err)
> - fprintf(stderr, "btf_load_err:%d btf_fd:%d\n",
> - test->btf_load_err, btf_fd);
> + CHECK(err, "btf_fd:%d test->btf_load_err:%u",
> + btf_fd, test->btf_load_err);
>
> if (err || btf_fd == -1)
> goto done;
> @@ -992,16 +1010,15 @@ static int do_test_raw(unsigned int test_num)
> map_fd = bpf_create_map_xattr(&create_attr);
>
> err = ((map_fd == -1) != test->map_create_err);
> - if (err)
> - fprintf(stderr, "map_create_err:%d map_fd:%d\n",
> - test->map_create_err, map_fd);
> + CHECK(err, "map_fd:%d test->map_create_err:%u",
> + map_fd, test->map_create_err);
>
> done:
> if (!err)
> - fprintf(stderr, "OK\n");
> + fprintf(stderr, "OK");
>
> if (*btf_log_buf && (err || args.always_log))
> - fprintf(stderr, "%s\n", btf_log_buf);
> + fprintf(stderr, "\n%s", btf_log_buf);
>
> if (btf_fd != -1)
> close(btf_fd);
> @@ -1017,10 +1034,10 @@ static int test_raw(void)
> int err = 0;
>
> if (args.raw_test_num)
> - return do_test_raw(args.raw_test_num);
> + return count_result(do_test_raw(args.raw_test_num));
>
> for (i = 1; i <= ARRAY_SIZE(raw_tests); i++)
> - err |= do_test_raw(i);
> + err |= count_result(do_test_raw(i));
>
> return err;
> }
> @@ -1080,8 +1097,7 @@ static int do_test_get_info(unsigned int test_num)
> *btf_log_buf = '\0';
>
> user_btf = malloc(raw_btf_size);
> - if (!user_btf) {
> - fprintf(stderr, "Cannot allocate memory for user_btf\n");
> + if (CHECK(!user_btf, "!user_btf")) {
> err = -1;
> goto done;
> }
> @@ -1089,9 +1105,7 @@ static int do_test_get_info(unsigned int test_num)
> btf_fd = bpf_load_btf(raw_btf, raw_btf_size,
> btf_log_buf, BTF_LOG_BUF_SIZE,
> args.always_log);
> - if (btf_fd == -1) {
> - fprintf(stderr, "bpf_load_btf:%s(%d)\n",
> - strerror(errno), errno);
> + if (CHECK(btf_fd == -1, "errno:%d", errno)) {
> err = -1;
> goto done;
> }
> @@ -1103,31 +1117,31 @@ static int do_test_get_info(unsigned int test_num)
> raw_btf_size - expected_nbytes);
>
> err = bpf_obj_get_info_by_fd(btf_fd, user_btf, &user_btf_size);
> - if (err || user_btf_size != raw_btf_size ||
> - memcmp(raw_btf, user_btf, expected_nbytes)) {
> - fprintf(stderr,
> - "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d\n",
> - err, errno,
> - raw_btf_size, user_btf_size, expected_nbytes,
> - memcmp(raw_btf, user_btf, expected_nbytes));
> + if (CHECK(err || user_btf_size != raw_btf_size ||
> + memcmp(raw_btf, user_btf, expected_nbytes),
> + "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d",
> + err, errno,
> + raw_btf_size, user_btf_size, expected_nbytes,
> + memcmp(raw_btf, user_btf, expected_nbytes))) {
> err = -1;
> goto done;
> }
>
> while (expected_nbytes < raw_btf_size) {
> fprintf(stderr, "%u...", expected_nbytes);
> - if (user_btf[expected_nbytes++] != 0xff) {
> - fprintf(stderr, "!= 0xff\n");
> + if (CHECK(user_btf[expected_nbytes++] != 0xff,
> + "user_btf[%u]:%x != 0xff", expected_nbytes - 1,
> + user_btf[expected_nbytes - 1])) {
> err = -1;
> goto done;
> }
> }
>
> - fprintf(stderr, "OK\n");
> + fprintf(stderr, "OK");
>
> done:
> if (*btf_log_buf && (err || args.always_log))
> - fprintf(stderr, "%s\n", btf_log_buf);
> + fprintf(stderr, "\n%s", btf_log_buf);
>
> free(raw_btf);
> free(user_btf);
> @@ -1144,10 +1158,10 @@ static int test_get_info(void)
> int err = 0;
>
> if (args.get_info_test_num)
> - return do_test_get_info(args.get_info_test_num);
> + return count_result(do_test_get_info(args.get_info_test_num));
>
> for (i = 1; i <= ARRAY_SIZE(get_info_tests); i++)
> - err |= do_test_get_info(i);
> + err |= count_result(do_test_get_info(i));
>
> return err;
> }
> @@ -1175,28 +1189,21 @@ static int file_has_btf_elf(const char *fn)
> Elf *elf;
> int ret;
>
> - if (elf_version(EV_CURRENT) == EV_NONE) {
> - fprintf(stderr, "Failed to init libelf\n");
> + if (CHECK(elf_version(EV_CURRENT) == EV_NONE,
> + "elf_version(EV_CURRENT) == EV_NONE"))
> return -1;
> - }
>
> elf_fd = open(fn, O_RDONLY);
> - if (elf_fd == -1) {
> - fprintf(stderr, "Cannot open file %s: %s(%d)\n",
> - fn, strerror(errno), errno);
> + if (CHECK(elf_fd == -1, "open(%s): errno:%d", fn, errno))
> return -1;
> - }
>
> elf = elf_begin(elf_fd, ELF_C_READ, NULL);
> - if (!elf) {
> - fprintf(stderr, "Failed to read ELF from %s. %s\n", fn,
> - elf_errmsg(elf_errno()));
> + if (CHECK(!elf, "elf_begin(%s): %s", fn, elf_errmsg(elf_errno()))) {
> ret = -1;
> goto done;
> }
>
> - if (!gelf_getehdr(elf, &ehdr)) {
> - fprintf(stderr, "Failed to get EHDR from %s\n", fn);
> + if (CHECK(!gelf_getehdr(elf, &ehdr), "!gelf_getehdr(%s)", fn)) {
> ret = -1;
> goto done;
> }
> @@ -1205,9 +1212,8 @@ static int file_has_btf_elf(const char *fn)
> const char *sh_name;
> GElf_Shdr sh;
>
> - if (gelf_getshdr(scn, &sh) != &sh) {
> - fprintf(stderr,
> - "Failed to get section header from %s\n", fn);
> + if (CHECK(gelf_getshdr(scn, &sh) != &sh,
> + "file:%s gelf_getshdr != &sh", fn)) {
> ret = -1;
> goto done;
> }
> @@ -1243,53 +1249,44 @@ static int do_test_file(unsigned int test_num)
> return err;
>
> if (err == 0) {
> - fprintf(stderr, "SKIP. No ELF %s found\n", BTF_ELF_SEC);
> + fprintf(stderr, "SKIP. No ELF %s found", BTF_ELF_SEC);
> + skip_cnt++;
> return 0;
> }
>
> obj = bpf_object__open(test->file);
> - if (IS_ERR(obj))
> + if (CHECK(IS_ERR(obj), "obj: %ld", PTR_ERR(obj)))
> return PTR_ERR(obj);
>
> err = bpf_object__btf_fd(obj);
> - if (err == -1) {
> - fprintf(stderr, "bpf_object__btf_fd: -1\n");
> + if (CHECK(err == -1, "bpf_object__btf_fd: -1"))
> goto done;
> - }
>
> prog = bpf_program__next(NULL, obj);
> - if (!prog) {
> - fprintf(stderr, "Cannot find bpf_prog\n");
> + if (CHECK(!prog, "Cannot find bpf_prog")) {
> err = -1;
> goto done;
> }
>
> bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> err = bpf_object__load(obj);
> - if (err < 0) {
> - fprintf(stderr, "bpf_object__load: %d\n", err);
> + if (CHECK(err < 0, "bpf_object__load: %d", err))
> goto done;
> - }
>
> map = bpf_object__find_map_by_name(obj, "btf_map");
> - if (!map) {
> - fprintf(stderr, "btf_map not found\n");
> + if (CHECK(!map, "btf_map not found")) {
> err = -1;
> goto done;
> }
>
> err = (bpf_map__btf_key_id(map) == 0 || bpf_map__btf_value_id(map) == 0)
> != test->btf_kv_notfound;
> - if (err) {
> - fprintf(stderr,
> - "btf_kv_notfound:%u btf_key_id:%u btf_value_id:%u\n",
> - test->btf_kv_notfound,
> - bpf_map__btf_key_id(map),
> - bpf_map__btf_value_id(map));
> + if (CHECK(err, "btf_key_id:%u btf_value_id:%u test->btf_kv_notfound:%u",
> + bpf_map__btf_key_id(map), bpf_map__btf_value_id(map),
> + test->btf_kv_notfound))
> goto done;
> - }
>
> - fprintf(stderr, "OK\n");
> + fprintf(stderr, "OK");
>
> done:
> bpf_object__close(obj);
> @@ -1302,10 +1299,10 @@ static int test_file(void)
> int err = 0;
>
> if (args.file_test_num)
> - return do_test_file(args.file_test_num);
> + return count_result(do_test_file(args.file_test_num));
>
> for (i = 1; i <= ARRAY_SIZE(file_tests); i++)
> - err |= do_test_file(i);
> + err |= count_result(do_test_file(i));
>
> return err;
> }
> @@ -1425,7 +1422,7 @@ static int test_pprint(void)
> unsigned int key;
> uint8_t *raw_btf;
> ssize_t nread;
> - int err;
> + int err, ret;
>
> fprintf(stderr, "%s......", test->descr);
> raw_btf = btf_raw_create(&hdr_tmpl, test->raw_types,
> @@ -1441,10 +1438,8 @@ static int test_pprint(void)
> args.always_log);
> free(raw_btf);
>
> - if (btf_fd == -1) {
> + if (CHECK(btf_fd == -1, "errno:%d", errno)) {
> err = -1;
> - fprintf(stderr, "bpf_load_btf: %s(%d)\n",
> - strerror(errno), errno);
> goto done;
> }
>
> @@ -1458,26 +1453,23 @@ static int test_pprint(void)
> create_attr.btf_value_id = test->value_id;
>
> map_fd = bpf_create_map_xattr(&create_attr);
> - if (map_fd == -1) {
> + if (CHECK(map_fd == -1, "errno:%d", errno)) {
> err = -1;
> - fprintf(stderr, "bpf_creat_map_btf: %s(%d)\n",
> - strerror(errno), errno);
> goto done;
> }
>
> - if (snprintf(pin_path, sizeof(pin_path), "%s/%s",
> - "/sys/fs/bpf", test->map_name) == sizeof(pin_path)) {
> + ret = snprintf(pin_path, sizeof(pin_path), "%s/%s",
> + "/sys/fs/bpf", test->map_name);
> +
> + if (CHECK(ret == sizeof(pin_path), "pin_path %s/%s is too long",
> + "/sys/fs/bpf", test->map_name)) {
> err = -1;
> - fprintf(stderr, "pin_path is too long\n");
> goto done;
> }
>
> err = bpf_obj_pin(map_fd, pin_path);
> - if (err) {
> - fprintf(stderr, "Cannot pin to %s. %s(%d).\n", pin_path,
> - strerror(errno), errno);
> + if (CHECK(err, "bpf_obj_pin(%s): errno:%d.", pin_path, errno))
> goto done;
> - }
>
> for (key = 0; key < test->max_entries; key++) {
> set_pprint_mapv(&mapv, key);
> @@ -1485,10 +1477,8 @@ static int test_pprint(void)
> }
>
> pin_file = fopen(pin_path, "r");
> - if (!pin_file) {
> + if (CHECK(!pin_file, "fopen(%s): errno:%d", pin_path, errno)) {
> err = -1;
> - fprintf(stderr, "fopen(%s): %s(%d)\n", pin_path,
> - strerror(errno), errno);
> goto done;
> }
>
> @@ -1497,9 +1487,8 @@ static int test_pprint(void)
> *line == '#')
> ;
>
> - if (nread <= 0) {
> + if (CHECK(nread <= 0, "Unexpected EOF")) {
> err = -1;
> - fprintf(stderr, "Unexpected EOF\n");
> goto done;
> }
>
> @@ -1518,9 +1507,9 @@ static int test_pprint(void)
> mapv.ui8a[4], mapv.ui8a[5], mapv.ui8a[6], mapv.ui8a[7],
> pprint_enum_str[mapv.aenum]);
>
> - if (nexpected_line == sizeof(expected_line)) {
> + if (CHECK(nexpected_line == sizeof(expected_line),
> + "expected_line is too long")) {
> err = -1;
> - fprintf(stderr, "expected_line is too long\n");
> goto done;
> }
>
> @@ -1535,15 +1524,15 @@ static int test_pprint(void)
> nread = getline(&line, &line_len, pin_file);
> } while (++key < test->max_entries && nread > 0);
>
> - if (key < test->max_entries) {
> + if (CHECK(key < test->max_entries,
> + "Unexpected EOF. key:%u test->max_entries:%u",
> + key, test->max_entries)) {
> err = -1;
> - fprintf(stderr, "Unexpected EOF\n");
> goto done;
> }
>
> - if (nread > 0) {
> + if (CHECK(nread > 0, "Unexpected extra pprint output: %s", line)) {
> err = -1;
> - fprintf(stderr, "Unexpected extra pprint output: %s\n", line);
> goto done;
> }
>
> @@ -1551,9 +1540,9 @@ static int test_pprint(void)
>
> done:
> if (!err)
> - fprintf(stderr, "OK\n");
> + fprintf(stderr, "OK");
> if (*btf_log_buf && (err || args.always_log))
> - fprintf(stderr, "%s\n", btf_log_buf);
> + fprintf(stderr, "\n%s", btf_log_buf);
> if (btf_fd != -1)
> close(btf_fd);
> if (map_fd != -1)
> @@ -1634,6 +1623,12 @@ static int parse_args(int argc, char **argv)
> return 0;
> }
>
> +static void print_summary(void)
> +{
> + fprintf(stderr, "PASS:%u SKIP:%u FAIL:%u\n",
> + pass_cnt - skip_cnt, skip_cnt, error_cnt);
> +}
> +
> int main(int argc, char **argv)
> {
> int err = 0;
> @@ -1655,15 +1650,17 @@ int main(int argc, char **argv)
> err |= test_file();
>
> if (args.pprint_test)
> - err |= test_pprint();
> + err |= count_result(test_pprint());
>
> if (args.raw_test || args.get_info_test || args.file_test ||
> args.pprint_test)
> - return err;
> + goto done;
>
> err |= test_raw();
> err |= test_get_info();
> err |= test_file();
>
> +done:
> + print_summary();
> return err;
> }
> --
> 2.9.5
>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf-next 5/6] bpf: btf: Update tools/include/uapi/linux/btf.h with BTF ID
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
` (3 preceding siblings ...)
2018-05-04 21:49 ` [PATCH bpf-next 4/6] bpf: btf: Some test_btf clean up Martin KaFai Lau
@ 2018-05-04 21:49 ` Martin KaFai Lau
2018-05-08 21:18 ` Song Liu
2018-05-04 21:49 ` [PATCH bpf-next 6/6] bpf: btf: Tests for BPF_OBJ_GET_INFO_BY_FD and BPF_BTF_GET_FD_BY_ID Martin KaFai Lau
2018-05-09 16:08 ` [PATCH bpf-next 0/6] Introduce BTF ID Daniel Borkmann
6 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2018-05-04 21:49 UTC (permalink / raw)
To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
This patch sync the tools/include/uapi/linux/btf.h with
the newly introduced BTF ID support.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
---
tools/include/uapi/linux/bpf.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 83a95ae388dd..fff51c187d1e 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -96,6 +96,7 @@ enum bpf_cmd {
BPF_PROG_QUERY,
BPF_RAW_TRACEPOINT_OPEN,
BPF_BTF_LOAD,
+ BPF_BTF_GET_FD_BY_ID,
};
enum bpf_map_type {
@@ -343,6 +344,7 @@ union bpf_attr {
__u32 start_id;
__u32 prog_id;
__u32 map_id;
+ __u32 btf_id;
};
__u32 next_id;
__u32 open_flags;
@@ -2129,6 +2131,15 @@ struct bpf_map_info {
__u32 ifindex;
__u64 netns_dev;
__u64 netns_ino;
+ __u32 btf_id;
+ __u32 btf_key_id;
+ __u32 btf_value_id;
+} __attribute__((aligned(8)));
+
+struct bpf_btf_info {
+ __aligned_u64 btf;
+ __u32 btf_size;
+ __u32 id;
} __attribute__((aligned(8)));
/* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
--
2.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH bpf-next 5/6] bpf: btf: Update tools/include/uapi/linux/btf.h with BTF ID
2018-05-04 21:49 ` [PATCH bpf-next 5/6] bpf: btf: Update tools/include/uapi/linux/btf.h with BTF ID Martin KaFai Lau
@ 2018-05-08 21:18 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2018-05-08 21:18 UTC (permalink / raw)
To: Martin Lau
Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Kernel Team
> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>
> This patch sync the tools/include/uapi/linux/btf.h with
> the newly introduced BTF ID support.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> tools/include/uapi/linux/bpf.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 83a95ae388dd..fff51c187d1e 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -96,6 +96,7 @@ enum bpf_cmd {
> BPF_PROG_QUERY,
> BPF_RAW_TRACEPOINT_OPEN,
> BPF_BTF_LOAD,
> + BPF_BTF_GET_FD_BY_ID,
> };
>
> enum bpf_map_type {
> @@ -343,6 +344,7 @@ union bpf_attr {
> __u32 start_id;
> __u32 prog_id;
> __u32 map_id;
> + __u32 btf_id;
> };
> __u32 next_id;
> __u32 open_flags;
> @@ -2129,6 +2131,15 @@ struct bpf_map_info {
> __u32 ifindex;
> __u64 netns_dev;
> __u64 netns_ino;
> + __u32 btf_id;
> + __u32 btf_key_id;
> + __u32 btf_value_id;
> +} __attribute__((aligned(8)));
> +
> +struct bpf_btf_info {
> + __aligned_u64 btf;
> + __u32 btf_size;
> + __u32 id;
> } __attribute__((aligned(8)));
>
> /* User bpf_sock_addr struct to access socket fields and sockaddr struct passed
> --
> 2.9.5
>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf-next 6/6] bpf: btf: Tests for BPF_OBJ_GET_INFO_BY_FD and BPF_BTF_GET_FD_BY_ID
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
` (4 preceding siblings ...)
2018-05-04 21:49 ` [PATCH bpf-next 5/6] bpf: btf: Update tools/include/uapi/linux/btf.h with BTF ID Martin KaFai Lau
@ 2018-05-04 21:49 ` Martin KaFai Lau
2018-05-08 21:18 ` Song Liu
2018-05-09 16:08 ` [PATCH bpf-next 0/6] Introduce BTF ID Daniel Borkmann
6 siblings, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2018-05-04 21:49 UTC (permalink / raw)
To: netdev; +Cc: Alexei Starovoitov, Daniel Borkmann, kernel-team
This patch adds test for BPF_BTF_GET_FD_BY_ID and the new
btf_id/btf_key_id/btf_value_id in the "struct bpf_map_info".
It also modifies the existing BPF_OBJ_GET_INFO_BY_FD test
to reflect the new "struct bpf_btf_info".
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Alexei Starovoitov <ast@fb.com>
---
tools/lib/bpf/bpf.c | 10 ++
tools/lib/bpf/bpf.h | 1 +
tools/testing/selftests/bpf/test_btf.c | 289 +++++++++++++++++++++++++++++++--
3 files changed, 287 insertions(+), 13 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 76b36cc16e7f..a3a8fb2ac697 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -458,6 +458,16 @@ int bpf_map_get_fd_by_id(__u32 id)
return sys_bpf(BPF_MAP_GET_FD_BY_ID, &attr, sizeof(attr));
}
+int bpf_btf_get_fd_by_id(__u32 id)
+{
+ union bpf_attr attr;
+
+ bzero(&attr, sizeof(attr));
+ attr.btf_id = id;
+
+ return sys_bpf(BPF_BTF_GET_FD_BY_ID, &attr, sizeof(attr));
+}
+
int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len)
{
union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 553b11ad52b3..fb3a146d92ff 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -98,6 +98,7 @@ int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id);
int bpf_map_get_next_id(__u32 start_id, __u32 *next_id);
int bpf_prog_get_fd_by_id(__u32 id);
int bpf_map_get_fd_by_id(__u32 id);
+int bpf_btf_get_fd_by_id(__u32 id);
int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len);
int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
__u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt);
diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index b7880a20fad1..c8bceae7ec02 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -1047,9 +1047,13 @@ struct btf_get_info_test {
const char *str_sec;
__u32 raw_types[MAX_NR_RAW_TYPES];
__u32 str_sec_size;
- int info_size_delta;
+ int btf_size_delta;
+ int (*special_test)(unsigned int test_num);
};
+static int test_big_btf_info(unsigned int test_num);
+static int test_btf_id(unsigned int test_num);
+
const struct btf_get_info_test get_info_tests[] = {
{
.descr = "== raw_btf_size+1",
@@ -1060,7 +1064,7 @@ const struct btf_get_info_test get_info_tests[] = {
},
.str_sec = "",
.str_sec_size = sizeof(""),
- .info_size_delta = 1,
+ .btf_size_delta = 1,
},
{
.descr = "== raw_btf_size-3",
@@ -1071,20 +1075,274 @@ const struct btf_get_info_test get_info_tests[] = {
},
.str_sec = "",
.str_sec_size = sizeof(""),
- .info_size_delta = -3,
+ .btf_size_delta = -3,
+},
+{
+ .descr = "Large bpf_btf_info",
+ .raw_types = {
+ /* int */ /* [1] */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+ BTF_END_RAW,
+ },
+ .str_sec = "",
+ .str_sec_size = sizeof(""),
+ .special_test = test_big_btf_info,
+},
+{
+ .descr = "BTF ID",
+ .raw_types = {
+ /* int */ /* [1] */
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+ /* unsigned int */ /* [2] */
+ BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
+ BTF_END_RAW,
+ },
+ .str_sec = "",
+ .str_sec_size = sizeof(""),
+ .special_test = test_btf_id,
},
};
+static inline __u64 ptr_to_u64(const void *ptr)
+{
+ return (__u64)(unsigned long)ptr;
+}
+
+static int test_big_btf_info(unsigned int test_num)
+{
+ const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
+ uint8_t *raw_btf = NULL, *user_btf = NULL;
+ unsigned int raw_btf_size;
+ struct {
+ struct bpf_btf_info info;
+ uint64_t garbage;
+ } info_garbage;
+ struct bpf_btf_info *info;
+ int btf_fd = -1, err;
+ uint32_t info_len;
+
+ raw_btf = btf_raw_create(&hdr_tmpl,
+ test->raw_types,
+ test->str_sec,
+ test->str_sec_size,
+ &raw_btf_size);
+
+ if (!raw_btf)
+ return -1;
+
+ *btf_log_buf = '\0';
+
+ user_btf = malloc(raw_btf_size);
+ if (CHECK(!user_btf, "!user_btf")) {
+ err = -1;
+ goto done;
+ }
+
+ btf_fd = bpf_load_btf(raw_btf, raw_btf_size,
+ btf_log_buf, BTF_LOG_BUF_SIZE,
+ args.always_log);
+ if (CHECK(btf_fd == -1, "errno:%d", errno)) {
+ err = -1;
+ goto done;
+ }
+
+ /*
+ * GET_INFO should error out if the userspace info
+ * has non zero tailing bytes.
+ */
+ info = &info_garbage.info;
+ memset(info, 0, sizeof(*info));
+ info_garbage.garbage = 0xdeadbeef;
+ info_len = sizeof(info_garbage);
+ info->btf = ptr_to_u64(user_btf);
+ info->btf_size = raw_btf_size;
+
+ err = bpf_obj_get_info_by_fd(btf_fd, info, &info_len);
+ if (CHECK(!err, "!err")) {
+ err = -1;
+ goto done;
+ }
+
+ /*
+ * GET_INFO should succeed even info_len is larger than
+ * the kernel supported as long as tailing bytes are zero.
+ * The kernel supported info len should also be returned
+ * to userspace.
+ */
+ info_garbage.garbage = 0;
+ err = bpf_obj_get_info_by_fd(btf_fd, info, &info_len);
+ if (CHECK(err || info_len != sizeof(*info),
+ "err:%d errno:%d info_len:%u sizeof(*info):%lu",
+ err, errno, info_len, sizeof(*info))) {
+ err = -1;
+ goto done;
+ }
+
+ fprintf(stderr, "OK");
+
+done:
+ if (*btf_log_buf && (err || args.always_log))
+ fprintf(stderr, "\n%s", btf_log_buf);
+
+ free(raw_btf);
+ free(user_btf);
+
+ if (btf_fd != -1)
+ close(btf_fd);
+
+ return err;
+}
+
+static int test_btf_id(unsigned int test_num)
+{
+ const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
+ struct bpf_create_map_attr create_attr = {};
+ uint8_t *raw_btf = NULL, *user_btf[2] = {};
+ int btf_fd[2] = {-1, -1}, map_fd = -1;
+ struct bpf_map_info map_info = {};
+ struct bpf_btf_info info[2] = {};
+ unsigned int raw_btf_size;
+ uint32_t info_len;
+ int err, i, ret;
+
+ raw_btf = btf_raw_create(&hdr_tmpl,
+ test->raw_types,
+ test->str_sec,
+ test->str_sec_size,
+ &raw_btf_size);
+
+ if (!raw_btf)
+ return -1;
+
+ *btf_log_buf = '\0';
+
+ for (i = 0; i < 2; i++) {
+ user_btf[i] = malloc(raw_btf_size);
+ if (CHECK(!user_btf[i], "!user_btf[%d]", i)) {
+ err = -1;
+ goto done;
+ }
+ info[i].btf = ptr_to_u64(user_btf[i]);
+ info[i].btf_size = raw_btf_size;
+ }
+
+ btf_fd[0] = bpf_load_btf(raw_btf, raw_btf_size,
+ btf_log_buf, BTF_LOG_BUF_SIZE,
+ args.always_log);
+ if (CHECK(btf_fd[0] == -1, "errno:%d", errno)) {
+ err = -1;
+ goto done;
+ }
+
+ /* Test BPF_OBJ_GET_INFO_BY_ID on btf_id */
+ info_len = sizeof(info[0]);
+ err = bpf_obj_get_info_by_fd(btf_fd[0], &info[0], &info_len);
+ if (CHECK(err, "errno:%d", errno)) {
+ err = -1;
+ goto done;
+ }
+
+ btf_fd[1] = bpf_btf_get_fd_by_id(info[0].id);
+ if (CHECK(btf_fd[1] == -1, "errno:%d", errno)) {
+ err = -1;
+ goto done;
+ }
+
+ ret = 0;
+ err = bpf_obj_get_info_by_fd(btf_fd[1], &info[1], &info_len);
+ if (CHECK(err || info[0].id != info[1].id ||
+ info[0].btf_size != info[1].btf_size ||
+ (ret = memcmp(user_btf[0], user_btf[1], info[0].btf_size)),
+ "err:%d errno:%d id0:%u id1:%u btf_size0:%u btf_size1:%u memcmp:%d",
+ err, errno, info[0].id, info[1].id,
+ info[0].btf_size, info[1].btf_size, ret)) {
+ err = -1;
+ goto done;
+ }
+
+ /* Test btf members in struct bpf_map_info */
+ create_attr.name = "test_btf_id";
+ create_attr.map_type = BPF_MAP_TYPE_ARRAY;
+ create_attr.key_size = sizeof(int);
+ create_attr.value_size = sizeof(unsigned int);
+ create_attr.max_entries = 4;
+ create_attr.btf_fd = btf_fd[0];
+ create_attr.btf_key_id = 1;
+ create_attr.btf_value_id = 2;
+
+ map_fd = bpf_create_map_xattr(&create_attr);
+ if (CHECK(map_fd == -1, "errno:%d", errno)) {
+ err = -1;
+ goto done;
+ }
+
+ info_len = sizeof(map_info);
+ err = bpf_obj_get_info_by_fd(map_fd, &map_info, &info_len);
+ if (CHECK(err || map_info.btf_id != info[0].id ||
+ map_info.btf_key_id != 1 || map_info.btf_value_id != 2,
+ "err:%d errno:%d info.id:%u btf_id:%u btf_key_id:%u btf_value_id:%u",
+ err, errno, info[0].id, map_info.btf_id, map_info.btf_key_id,
+ map_info.btf_value_id)) {
+ err = -1;
+ goto done;
+ }
+
+ for (i = 0; i < 2; i++) {
+ close(btf_fd[i]);
+ btf_fd[i] = -1;
+ }
+
+ /* Test BTF ID is removed from the kernel */
+ btf_fd[0] = bpf_btf_get_fd_by_id(map_info.btf_id);
+ if (CHECK(btf_fd[0] == -1, "errno:%d", errno)) {
+ err = -1;
+ goto done;
+ }
+ close(btf_fd[0]);
+ btf_fd[0] = -1;
+
+ /* The map holds the last ref to BTF and its btf_id */
+ close(map_fd);
+ map_fd = -1;
+ btf_fd[0] = bpf_btf_get_fd_by_id(map_info.btf_id);
+ if (CHECK(btf_fd[0] != -1, "BTF lingers")) {
+ err = -1;
+ goto done;
+ }
+
+ fprintf(stderr, "OK");
+
+done:
+ if (*btf_log_buf && (err || args.always_log))
+ fprintf(stderr, "\n%s", btf_log_buf);
+
+ free(raw_btf);
+ if (map_fd != -1)
+ close(map_fd);
+ for (i = 0; i < 2; i++) {
+ free(user_btf[i]);
+ if (btf_fd[i] != -1)
+ close(btf_fd[i]);
+ }
+
+ return err;
+}
+
static int do_test_get_info(unsigned int test_num)
{
const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
unsigned int raw_btf_size, user_btf_size, expected_nbytes;
uint8_t *raw_btf = NULL, *user_btf = NULL;
- int btf_fd = -1, err;
+ struct bpf_btf_info info = {};
+ int btf_fd = -1, err, ret;
+ uint32_t info_len;
- fprintf(stderr, "BTF GET_INFO_BY_ID test[%u] (%s): ",
+ fprintf(stderr, "BTF GET_INFO test[%u] (%s): ",
test_num, test->descr);
+ if (test->special_test)
+ return test->special_test(test_num);
+
raw_btf = btf_raw_create(&hdr_tmpl,
test->raw_types,
test->str_sec,
@@ -1110,19 +1368,24 @@ static int do_test_get_info(unsigned int test_num)
goto done;
}
- user_btf_size = (int)raw_btf_size + test->info_size_delta;
+ user_btf_size = (int)raw_btf_size + test->btf_size_delta;
expected_nbytes = min(raw_btf_size, user_btf_size);
if (raw_btf_size > expected_nbytes)
memset(user_btf + expected_nbytes, 0xff,
raw_btf_size - expected_nbytes);
- err = bpf_obj_get_info_by_fd(btf_fd, user_btf, &user_btf_size);
- if (CHECK(err || user_btf_size != raw_btf_size ||
- memcmp(raw_btf, user_btf, expected_nbytes),
- "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d",
- err, errno,
- raw_btf_size, user_btf_size, expected_nbytes,
- memcmp(raw_btf, user_btf, expected_nbytes))) {
+ info_len = sizeof(info);
+ info.btf = ptr_to_u64(user_btf);
+ info.btf_size = user_btf_size;
+
+ ret = 0;
+ err = bpf_obj_get_info_by_fd(btf_fd, &info, &info_len);
+ if (CHECK(err || !info.id || info_len != sizeof(info) ||
+ info.btf_size != raw_btf_size ||
+ (ret = memcmp(raw_btf, user_btf, expected_nbytes)),
+ "err:%d errno:%d info.id:%u info_len:%u sizeof(info):%lu raw_btf_size:%u info.btf_size:%u expected_nbytes:%u memcmp:%d",
+ err, errno, info.id, info_len, sizeof(info),
+ raw_btf_size, info.btf_size, expected_nbytes, ret)) {
err = -1;
goto done;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH bpf-next 6/6] bpf: btf: Tests for BPF_OBJ_GET_INFO_BY_FD and BPF_BTF_GET_FD_BY_ID
2018-05-04 21:49 ` [PATCH bpf-next 6/6] bpf: btf: Tests for BPF_OBJ_GET_INFO_BY_FD and BPF_BTF_GET_FD_BY_ID Martin KaFai Lau
@ 2018-05-08 21:18 ` Song Liu
0 siblings, 0 replies; 14+ messages in thread
From: Song Liu @ 2018-05-08 21:18 UTC (permalink / raw)
To: Martin Lau
Cc: netdev@vger.kernel.org, Alexei Starovoitov, Daniel Borkmann,
Kernel Team
> On May 4, 2018, at 2:49 PM, Martin KaFai Lau <kafai@fb.com> wrote:
>
> This patch adds test for BPF_BTF_GET_FD_BY_ID and the new
> btf_id/btf_key_id/btf_value_id in the "struct bpf_map_info".
>
> It also modifies the existing BPF_OBJ_GET_INFO_BY_FD test
> to reflect the new "struct bpf_btf_info".
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Alexei Starovoitov <ast@fb.com>
> ---
> tools/lib/bpf/bpf.c | 10 ++
> tools/lib/bpf/bpf.h | 1 +
> tools/testing/selftests/bpf/test_btf.c | 289 +++++++++++++++++++++++++++++++--
> 3 files changed, 287 insertions(+), 13 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 76b36cc16e7f..a3a8fb2ac697 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -458,6 +458,16 @@ int bpf_map_get_fd_by_id(__u32 id)
> return sys_bpf(BPF_MAP_GET_FD_BY_ID, &attr, sizeof(attr));
> }
>
> +int bpf_btf_get_fd_by_id(__u32 id)
> +{
> + union bpf_attr attr;
> +
> + bzero(&attr, sizeof(attr));
> + attr.btf_id = id;
> +
> + return sys_bpf(BPF_BTF_GET_FD_BY_ID, &attr, sizeof(attr));
> +}
> +
> int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len)
> {
> union bpf_attr attr;
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 553b11ad52b3..fb3a146d92ff 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -98,6 +98,7 @@ int bpf_prog_get_next_id(__u32 start_id, __u32 *next_id);
> int bpf_map_get_next_id(__u32 start_id, __u32 *next_id);
> int bpf_prog_get_fd_by_id(__u32 id);
> int bpf_map_get_fd_by_id(__u32 id);
> +int bpf_btf_get_fd_by_id(__u32 id);
> int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len);
> int bpf_prog_query(int target_fd, enum bpf_attach_type type, __u32 query_flags,
> __u32 *attach_flags, __u32 *prog_ids, __u32 *prog_cnt);
> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index b7880a20fad1..c8bceae7ec02 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -1047,9 +1047,13 @@ struct btf_get_info_test {
> const char *str_sec;
> __u32 raw_types[MAX_NR_RAW_TYPES];
> __u32 str_sec_size;
> - int info_size_delta;
> + int btf_size_delta;
> + int (*special_test)(unsigned int test_num);
> };
>
> +static int test_big_btf_info(unsigned int test_num);
> +static int test_btf_id(unsigned int test_num);
> +
> const struct btf_get_info_test get_info_tests[] = {
> {
> .descr = "== raw_btf_size+1",
> @@ -1060,7 +1064,7 @@ const struct btf_get_info_test get_info_tests[] = {
> },
> .str_sec = "",
> .str_sec_size = sizeof(""),
> - .info_size_delta = 1,
> + .btf_size_delta = 1,
> },
> {
> .descr = "== raw_btf_size-3",
> @@ -1071,20 +1075,274 @@ const struct btf_get_info_test get_info_tests[] = {
> },
> .str_sec = "",
> .str_sec_size = sizeof(""),
> - .info_size_delta = -3,
> + .btf_size_delta = -3,
> +},
> +{
> + .descr = "Large bpf_btf_info",
> + .raw_types = {
> + /* int */ /* [1] */
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
> + BTF_END_RAW,
> + },
> + .str_sec = "",
> + .str_sec_size = sizeof(""),
> + .special_test = test_big_btf_info,
> +},
> +{
> + .descr = "BTF ID",
> + .raw_types = {
> + /* int */ /* [1] */
> + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
> + /* unsigned int */ /* [2] */
> + BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
> + BTF_END_RAW,
> + },
> + .str_sec = "",
> + .str_sec_size = sizeof(""),
> + .special_test = test_btf_id,
> },
> };
>
> +static inline __u64 ptr_to_u64(const void *ptr)
> +{
> + return (__u64)(unsigned long)ptr;
> +}
> +
> +static int test_big_btf_info(unsigned int test_num)
> +{
> + const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
> + uint8_t *raw_btf = NULL, *user_btf = NULL;
> + unsigned int raw_btf_size;
> + struct {
> + struct bpf_btf_info info;
> + uint64_t garbage;
> + } info_garbage;
> + struct bpf_btf_info *info;
> + int btf_fd = -1, err;
> + uint32_t info_len;
> +
> + raw_btf = btf_raw_create(&hdr_tmpl,
> + test->raw_types,
> + test->str_sec,
> + test->str_sec_size,
> + &raw_btf_size);
> +
> + if (!raw_btf)
> + return -1;
> +
> + *btf_log_buf = '\0';
> +
> + user_btf = malloc(raw_btf_size);
> + if (CHECK(!user_btf, "!user_btf")) {
> + err = -1;
> + goto done;
> + }
> +
> + btf_fd = bpf_load_btf(raw_btf, raw_btf_size,
> + btf_log_buf, BTF_LOG_BUF_SIZE,
> + args.always_log);
> + if (CHECK(btf_fd == -1, "errno:%d", errno)) {
> + err = -1;
> + goto done;
> + }
> +
> + /*
> + * GET_INFO should error out if the userspace info
> + * has non zero tailing bytes.
> + */
> + info = &info_garbage.info;
> + memset(info, 0, sizeof(*info));
> + info_garbage.garbage = 0xdeadbeef;
> + info_len = sizeof(info_garbage);
> + info->btf = ptr_to_u64(user_btf);
> + info->btf_size = raw_btf_size;
> +
> + err = bpf_obj_get_info_by_fd(btf_fd, info, &info_len);
> + if (CHECK(!err, "!err")) {
> + err = -1;
> + goto done;
> + }
> +
> + /*
> + * GET_INFO should succeed even info_len is larger than
> + * the kernel supported as long as tailing bytes are zero.
> + * The kernel supported info len should also be returned
> + * to userspace.
> + */
> + info_garbage.garbage = 0;
> + err = bpf_obj_get_info_by_fd(btf_fd, info, &info_len);
> + if (CHECK(err || info_len != sizeof(*info),
> + "err:%d errno:%d info_len:%u sizeof(*info):%lu",
> + err, errno, info_len, sizeof(*info))) {
> + err = -1;
> + goto done;
> + }
> +
> + fprintf(stderr, "OK");
> +
> +done:
> + if (*btf_log_buf && (err || args.always_log))
> + fprintf(stderr, "\n%s", btf_log_buf);
> +
> + free(raw_btf);
> + free(user_btf);
> +
> + if (btf_fd != -1)
> + close(btf_fd);
> +
> + return err;
> +}
> +
> +static int test_btf_id(unsigned int test_num)
> +{
> + const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
> + struct bpf_create_map_attr create_attr = {};
> + uint8_t *raw_btf = NULL, *user_btf[2] = {};
> + int btf_fd[2] = {-1, -1}, map_fd = -1;
> + struct bpf_map_info map_info = {};
> + struct bpf_btf_info info[2] = {};
> + unsigned int raw_btf_size;
> + uint32_t info_len;
> + int err, i, ret;
> +
> + raw_btf = btf_raw_create(&hdr_tmpl,
> + test->raw_types,
> + test->str_sec,
> + test->str_sec_size,
> + &raw_btf_size);
> +
> + if (!raw_btf)
> + return -1;
> +
> + *btf_log_buf = '\0';
> +
> + for (i = 0; i < 2; i++) {
> + user_btf[i] = malloc(raw_btf_size);
> + if (CHECK(!user_btf[i], "!user_btf[%d]", i)) {
> + err = -1;
> + goto done;
> + }
> + info[i].btf = ptr_to_u64(user_btf[i]);
> + info[i].btf_size = raw_btf_size;
> + }
> +
> + btf_fd[0] = bpf_load_btf(raw_btf, raw_btf_size,
> + btf_log_buf, BTF_LOG_BUF_SIZE,
> + args.always_log);
> + if (CHECK(btf_fd[0] == -1, "errno:%d", errno)) {
> + err = -1;
> + goto done;
> + }
> +
> + /* Test BPF_OBJ_GET_INFO_BY_ID on btf_id */
> + info_len = sizeof(info[0]);
> + err = bpf_obj_get_info_by_fd(btf_fd[0], &info[0], &info_len);
> + if (CHECK(err, "errno:%d", errno)) {
> + err = -1;
> + goto done;
> + }
> +
> + btf_fd[1] = bpf_btf_get_fd_by_id(info[0].id);
> + if (CHECK(btf_fd[1] == -1, "errno:%d", errno)) {
> + err = -1;
> + goto done;
> + }
> +
> + ret = 0;
> + err = bpf_obj_get_info_by_fd(btf_fd[1], &info[1], &info_len);
> + if (CHECK(err || info[0].id != info[1].id ||
> + info[0].btf_size != info[1].btf_size ||
> + (ret = memcmp(user_btf[0], user_btf[1], info[0].btf_size)),
> + "err:%d errno:%d id0:%u id1:%u btf_size0:%u btf_size1:%u memcmp:%d",
> + err, errno, info[0].id, info[1].id,
> + info[0].btf_size, info[1].btf_size, ret)) {
> + err = -1;
> + goto done;
> + }
> +
> + /* Test btf members in struct bpf_map_info */
> + create_attr.name = "test_btf_id";
> + create_attr.map_type = BPF_MAP_TYPE_ARRAY;
> + create_attr.key_size = sizeof(int);
> + create_attr.value_size = sizeof(unsigned int);
> + create_attr.max_entries = 4;
> + create_attr.btf_fd = btf_fd[0];
> + create_attr.btf_key_id = 1;
> + create_attr.btf_value_id = 2;
> +
> + map_fd = bpf_create_map_xattr(&create_attr);
> + if (CHECK(map_fd == -1, "errno:%d", errno)) {
> + err = -1;
> + goto done;
> + }
> +
> + info_len = sizeof(map_info);
> + err = bpf_obj_get_info_by_fd(map_fd, &map_info, &info_len);
> + if (CHECK(err || map_info.btf_id != info[0].id ||
> + map_info.btf_key_id != 1 || map_info.btf_value_id != 2,
> + "err:%d errno:%d info.id:%u btf_id:%u btf_key_id:%u btf_value_id:%u",
> + err, errno, info[0].id, map_info.btf_id, map_info.btf_key_id,
> + map_info.btf_value_id)) {
> + err = -1;
> + goto done;
> + }
> +
> + for (i = 0; i < 2; i++) {
> + close(btf_fd[i]);
> + btf_fd[i] = -1;
> + }
> +
> + /* Test BTF ID is removed from the kernel */
> + btf_fd[0] = bpf_btf_get_fd_by_id(map_info.btf_id);
> + if (CHECK(btf_fd[0] == -1, "errno:%d", errno)) {
> + err = -1;
> + goto done;
> + }
> + close(btf_fd[0]);
> + btf_fd[0] = -1;
> +
> + /* The map holds the last ref to BTF and its btf_id */
> + close(map_fd);
> + map_fd = -1;
> + btf_fd[0] = bpf_btf_get_fd_by_id(map_info.btf_id);
> + if (CHECK(btf_fd[0] != -1, "BTF lingers")) {
> + err = -1;
> + goto done;
> + }
> +
> + fprintf(stderr, "OK");
> +
> +done:
> + if (*btf_log_buf && (err || args.always_log))
> + fprintf(stderr, "\n%s", btf_log_buf);
> +
> + free(raw_btf);
> + if (map_fd != -1)
> + close(map_fd);
> + for (i = 0; i < 2; i++) {
> + free(user_btf[i]);
> + if (btf_fd[i] != -1)
> + close(btf_fd[i]);
> + }
> +
> + return err;
> +}
> +
> static int do_test_get_info(unsigned int test_num)
> {
> const struct btf_get_info_test *test = &get_info_tests[test_num - 1];
> unsigned int raw_btf_size, user_btf_size, expected_nbytes;
> uint8_t *raw_btf = NULL, *user_btf = NULL;
> - int btf_fd = -1, err;
> + struct bpf_btf_info info = {};
> + int btf_fd = -1, err, ret;
> + uint32_t info_len;
>
> - fprintf(stderr, "BTF GET_INFO_BY_ID test[%u] (%s): ",
> + fprintf(stderr, "BTF GET_INFO test[%u] (%s): ",
> test_num, test->descr);
>
> + if (test->special_test)
> + return test->special_test(test_num);
> +
> raw_btf = btf_raw_create(&hdr_tmpl,
> test->raw_types,
> test->str_sec,
> @@ -1110,19 +1368,24 @@ static int do_test_get_info(unsigned int test_num)
> goto done;
> }
>
> - user_btf_size = (int)raw_btf_size + test->info_size_delta;
> + user_btf_size = (int)raw_btf_size + test->btf_size_delta;
> expected_nbytes = min(raw_btf_size, user_btf_size);
> if (raw_btf_size > expected_nbytes)
> memset(user_btf + expected_nbytes, 0xff,
> raw_btf_size - expected_nbytes);
>
> - err = bpf_obj_get_info_by_fd(btf_fd, user_btf, &user_btf_size);
> - if (CHECK(err || user_btf_size != raw_btf_size ||
> - memcmp(raw_btf, user_btf, expected_nbytes),
> - "err:%d(errno:%d) raw_btf_size:%u user_btf_size:%u expected_nbytes:%u memcmp:%d",
> - err, errno,
> - raw_btf_size, user_btf_size, expected_nbytes,
> - memcmp(raw_btf, user_btf, expected_nbytes))) {
> + info_len = sizeof(info);
> + info.btf = ptr_to_u64(user_btf);
> + info.btf_size = user_btf_size;
> +
> + ret = 0;
> + err = bpf_obj_get_info_by_fd(btf_fd, &info, &info_len);
> + if (CHECK(err || !info.id || info_len != sizeof(info) ||
> + info.btf_size != raw_btf_size ||
> + (ret = memcmp(raw_btf, user_btf, expected_nbytes)),
> + "err:%d errno:%d info.id:%u info_len:%u sizeof(info):%lu raw_btf_size:%u info.btf_size:%u expected_nbytes:%u memcmp:%d",
> + err, errno, info.id, info_len, sizeof(info),
> + raw_btf_size, info.btf_size, expected_nbytes, ret)) {
> err = -1;
> goto done;
> }
> --
> 2.9.5
>
Acked-by: Song Liu <songliubraving@fb.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next 0/6] Introduce BTF ID
2018-05-04 21:49 [PATCH bpf-next 0/6] Introduce BTF ID Martin KaFai Lau
` (5 preceding siblings ...)
2018-05-04 21:49 ` [PATCH bpf-next 6/6] bpf: btf: Tests for BPF_OBJ_GET_INFO_BY_FD and BPF_BTF_GET_FD_BY_ID Martin KaFai Lau
@ 2018-05-09 16:08 ` Daniel Borkmann
6 siblings, 0 replies; 14+ messages in thread
From: Daniel Borkmann @ 2018-05-09 16:08 UTC (permalink / raw)
To: Martin KaFai Lau, netdev; +Cc: Alexei Starovoitov, kernel-team
On 05/04/2018 11:49 PM, Martin KaFai Lau wrote:
> This series introduces BTF ID which is exposed through
> the new BPF_BTF_GET_FD_BY_ID cmd, new "struct bpf_btf_info"
> and new members in the "struct bpf_map_info".
>
> Please see individual patch for details.
Applied to bpf-next, thanks Martin!
^ permalink raw reply [flat|nested] 14+ messages in thread