rename _info_cnt to nr__info in bpf_prog_info

PATCH Applied Yonghong Song 2 participants 5 messages Dec 10, 2018

Subsystems (8)

Networking Network drivers Kselftest BPF +4 more subsystems

lore

Share & tools

lore

Yonghong Song

author

2018-12-10 22:14 UTC #0 lore raw

cover rename *_info_cnt to nr_*_info in bpf_prog_info

Before func_info and line_info are added to the kernel, there are several fields in structure bpf_prog_info specifying the "count" of a user buffer, e.g.,

        __u32 nr_jited_ksyms;
        __u32 nr_jited_func_lens;

The naming convention has the prefix "nr_".

The func_info and line_info support added several fields

        __u32 func_info_cnt;
        __u32 line_info_cnt;
        __u32 jited_line_info_cnt;

to indicate the "count" of buffers func_info, line_info and jited_line_info. The original intention is to keep the field names the same as those in structure bpf_attr, so it will be clear that the "count" returned to user space will be the same as the one passed to the kernel during prog load.

Unfortunately, the field names *_info_cnt are not consistent with other existing fields in bpf_prog_info.
This patch set renamed the fields *_info_cnt to nr_*_info to keep naming convention consistent.

Yonghong Song (3):
bpf: rename *_info_cnt to nr_*_info in bpf_prog_info tools/bpf: sync kernel uapi bpf.h to tools directory tools/bpf: rename *_info_cnt to nr_*_info

 include/uapi/linux/bpf.h               |  6 +--
 kernel/bpf/syscall.c                   | 38 ++++++++--------
 tools/bpf/bpftool/prog.c               | 48 ++++++++++-----------
 tools/include/uapi/linux/bpf.h         |  6 +--
 tools/lib/bpf/bpf_prog_linfo.c         |  4 +-
 tools/testing/selftests/bpf/test_btf.c | 60 +++++++++++++-------------
 6 files changed, 81 insertions(+), 81 deletions(-)

Signature · 1 line

2.17.1

Yonghong Song

author

2018-12-10 22:14 UTC #1 lore raw

PATCH 1/3 bpf: rename *_info_cnt to nr_*_info in bpf_prog_info

In uapi bpf.h, currently we have the following fields in the struct bpf_prog_info:

	__u32 func_info_cnt;
	__u32 line_info_cnt;
	__u32 jited_line_info_cnt;

The above field names "func_info_cnt" and "line_info_cnt" also appear in union bpf_attr for program loading.

The original intention is to keep the names the same between bpf_prog_info and bpf_attr
so it will imply what we returned to user space will be the same as what the user space passed to the kernel.

Such a naming convention in bpf_prog_info is not consistent with other fields like:

        __u32 nr_jited_ksyms;
        __u32 nr_jited_func_lens;

This patch made this adjustment so in bpf_prog_info newly introduced *_info_cnt becomes nr_*_info.

Acked-by Song Liu <[email protected]>

Acked-by Martin KaFai Lau <[email protected]>

Signed-off-by Yonghong Song <[email protected]>

---
 include/uapi/linux/bpf.h |  6 +++---
 kernel/bpf/syscall.c     | 38 +++++++++++++++++++-------------------
 2 files changed, 22 insertions(+), 22 deletions(-)

include/uapi/linux/bpf.h +3 -3

index 1bee1135866a..f943ed803309 100644
@@ -2696,11 +2696,11 @@ struct bpf_prog_info {
 	__u32 btf_id;
 	__u32 func_info_rec_size;
 	__aligned_u64 func_info;
-	__u32 func_info_cnt;
-	__u32 line_info_cnt;
+	__u32 nr_func_info;
+	__u32 nr_line_info;
 	__aligned_u64 line_info;
 	__aligned_u64 jited_line_info;
-	__u32 jited_line_info_cnt;
+	__u32 nr_jited_line_info;
 	__u32 line_info_rec_size;
 	__u32 jited_line_info_rec_size;
 } __attribute__((aligned(8)));

kernel/bpf/syscall.c +19 ~~-19~~

index 19c88cff7880..318a592c6925 100644
@@ -2055,15 +2055,15 @@ static int set_info_rec_size(struct bpf_prog_info *info)
 	 * _rec_size back to the info.
 	 */
 
-	if ((info->func_info_cnt || info->func_info_rec_size) &&
+	if ((info->nr_func_info || info->func_info_rec_size) &&
 	    info->func_info_rec_size != sizeof(struct bpf_func_info))
 		return -EINVAL;
 
-	if ((info->line_info_cnt || info->line_info_rec_size) &&
+	if ((info->nr_line_info || info->line_info_rec_size) &&
 	    info->line_info_rec_size != sizeof(struct bpf_line_info))
 		return -EINVAL;
 
-	if ((info->jited_line_info_cnt || info->jited_line_info_rec_size) &&
+	if ((info->nr_jited_line_info || info->jited_line_info_rec_size) &&
 	    info->jited_line_info_rec_size != sizeof(__u64))
 		return -EINVAL;

@@ -2125,9 +2125,9 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 		info.xlated_prog_len = 0;
 		info.nr_jited_ksyms = 0;
 		info.nr_jited_func_lens = 0;
-		info.func_info_cnt = 0;
-		info.line_info_cnt = 0;
-		info.jited_line_info_cnt = 0;
+		info.nr_func_info = 0;
+		info.nr_line_info = 0;
+		info.nr_jited_line_info = 0;
 		goto done;
 	}

@@ -2268,14 +2268,14 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 	if (prog->aux->btf)
 		info.btf_id = btf_id(prog->aux->btf);
 
-	ulen = info.func_info_cnt;
-	info.func_info_cnt = prog->aux->func_info_cnt;
-	if (info.func_info_cnt && ulen) {
+	ulen = info.nr_func_info;
+	info.nr_func_info = prog->aux->func_info_cnt;
+	if (info.nr_func_info && ulen) {
 		if (bpf_dump_raw_ok()) {
 			char __user *user_finfo;
 
 			user_finfo = u64_to_user_ptr(info.func_info);
-			ulen = min_t(u32, info.func_info_cnt, ulen);
+			ulen = min_t(u32, info.nr_func_info, ulen);
 			if (copy_to_user(user_finfo, prog->aux->func_info,
 					 info.func_info_rec_size * ulen))
 				return -EFAULT;

@@ -2284,14 +2284,14 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 		}
 	}
 
-	ulen = info.line_info_cnt;
-	info.line_info_cnt = prog->aux->nr_linfo;
-	if (info.line_info_cnt && ulen) {
+	ulen = info.nr_line_info;
+	info.nr_line_info = prog->aux->nr_linfo;
+	if (info.nr_line_info && ulen) {
 		if (bpf_dump_raw_ok()) {
 			__u8 __user *user_linfo;
 
 			user_linfo = u64_to_user_ptr(info.line_info);
-			ulen = min_t(u32, info.line_info_cnt, ulen);
+			ulen = min_t(u32, info.nr_line_info, ulen);
 			if (copy_to_user(user_linfo, prog->aux->linfo,
 					 info.line_info_rec_size * ulen))
 				return -EFAULT;

@@ -2300,18 +2300,18 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 		}
 	}
 
-	ulen = info.jited_line_info_cnt;
+	ulen = info.nr_jited_line_info;
 	if (prog->aux->jited_linfo)
-		info.jited_line_info_cnt = prog->aux->nr_linfo;
+		info.nr_jited_line_info = prog->aux->nr_linfo;
 	else
-		info.jited_line_info_cnt = 0;
-	if (info.jited_line_info_cnt && ulen) {
+		info.nr_jited_line_info = 0;
+	if (info.nr_jited_line_info && ulen) {
 		if (bpf_dump_raw_ok()) {
 			__u64 __user *user_linfo;
 			u32 i;
 
 			user_linfo = u64_to_user_ptr(info.jited_line_info);
-			ulen = min_t(u32, info.jited_line_info_cnt, ulen);
+			ulen = min_t(u32, info.nr_jited_line_info, ulen);
 			for (i = 0; i < ulen; i++) {
 				if (put_user((__u64)(long)prog->aux->jited_linfo[i],
 					     &user_linfo[i]))

Signature · 1 line

2.17.1

Yonghong Song

author

2018-12-10 22:14 UTC #2 lore raw

PATCH 2/3 tools/bpf: sync kernel uapi bpf.h to tools directory

In reply to Yonghong Song

Sync kernel uapi bpf.h "*_info_cnt => nr_*_info"
changes to tools directory.

Acked-by Martin KaFai Lau <[email protected]>

Signed-off-by Yonghong Song <[email protected]>

---
 tools/include/uapi/linux/bpf.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

tools/include/uapi/linux/bpf.h +3 -3

index 6ad50b6471d3..620ee1f919cf 100644
@@ -2695,11 +2695,11 @@ struct bpf_prog_info {
 	__u32 btf_id;
 	__u32 func_info_rec_size;
 	__aligned_u64 func_info;
-	__u32 func_info_cnt;
-	__u32 line_info_cnt;
+	__u32 nr_func_info;
+	__u32 nr_line_info;
 	__aligned_u64 line_info;
 	__aligned_u64 jited_line_info;
-	__u32 jited_line_info_cnt;
+	__u32 nr_jited_line_info;
 	__u32 line_info_rec_size;
 	__u32 jited_line_info_rec_size;
 } __attribute__((aligned(8)));

Signature · 1 line

2.17.1

Yonghong Song

author

2018-12-10 22:14 UTC #3 lore raw

PATCH 3/3 tools/bpf: rename *_info_cnt to nr_*_info

In reply to Yonghong Song

Rename all occurances of *_info_cnt field access to nr_*_info in tools directory.

The local variables finfo_cnt, linfo_cnt and jited_linfo_cnt in function do_dump() of tools/bpf/bpftool/prog.c are also changed to nr_finfo, nr_linfo and nr_jited_linfo to keep naming convention consistent.

Acked-by Martin KaFai Lau <[email protected]>

Signed-off-by Yonghong Song <[email protected]>

---
 tools/bpf/bpftool/prog.c               | 48 ++++++++++-----------
 tools/lib/bpf/bpf_prog_linfo.c         |  4 +-
 tools/testing/selftests/bpf/test_btf.c | 60 +++++++++++++-------------
 3 files changed, 56 insertions(+), 56 deletions(-)

tools/bpf/bpftool/prog.c +24 ~~-24~~

index 9a78ebbcea1d..b73b4e473948 100644
@@ -425,7 +425,7 @@ static int do_dump(int argc, char **argv)
 {
 	unsigned int finfo_rec_size, linfo_rec_size, jited_linfo_rec_size;
 	void *func_info = NULL, *linfo = NULL, *jited_linfo = NULL;
-	unsigned int finfo_cnt, linfo_cnt = 0, jited_linfo_cnt = 0;
+	unsigned int nr_finfo, nr_linfo = 0, nr_jited_linfo = 0;
 	struct bpf_prog_linfo *prog_linfo = NULL;
 	unsigned long *func_ksyms = NULL;
 	struct bpf_prog_info info = {};

@@ -537,10 +537,10 @@ static int do_dump(int argc, char **argv)
 		}
 	}
 
-	finfo_cnt = info.func_info_cnt;
+	nr_finfo = info.nr_func_info;
 	finfo_rec_size = info.func_info_rec_size;
-	if (finfo_cnt && finfo_rec_size) {
-		func_info = malloc(finfo_cnt * finfo_rec_size);
+	if (nr_finfo && finfo_rec_size) {
+		func_info = malloc(nr_finfo * finfo_rec_size);
 		if (!func_info) {
 			p_err("mem alloc failed");
 			close(fd);

@@ -549,9 +549,9 @@ static int do_dump(int argc, char **argv)
 	}
 
 	linfo_rec_size = info.line_info_rec_size;
-	if (info.line_info_cnt && linfo_rec_size && info.btf_id) {
-		linfo_cnt = info.line_info_cnt;
-		linfo = malloc(linfo_cnt * linfo_rec_size);
+	if (info.nr_line_info && linfo_rec_size && info.btf_id) {
+		nr_linfo = info.nr_line_info;
+		linfo = malloc(nr_linfo * linfo_rec_size);
 		if (!linfo) {
 			p_err("mem alloc failed");
 			close(fd);

@@ -560,13 +560,13 @@ static int do_dump(int argc, char **argv)
 	}
 
 	jited_linfo_rec_size = info.jited_line_info_rec_size;
-	if (info.jited_line_info_cnt &&
+	if (info.nr_jited_line_info &&
 	    jited_linfo_rec_size &&
 	    info.nr_jited_ksyms &&
 	    info.nr_jited_func_lens &&
 	    info.btf_id) {
-		jited_linfo_cnt = info.jited_line_info_cnt;
-		jited_linfo = malloc(jited_linfo_cnt * jited_linfo_rec_size);
+		nr_jited_linfo = info.nr_jited_line_info;
+		jited_linfo = malloc(nr_jited_linfo * jited_linfo_rec_size);
 		if (!jited_linfo) {
 			p_err("mem alloc failed");
 			close(fd);

@@ -582,13 +582,13 @@ static int do_dump(int argc, char **argv)
 	info.nr_jited_ksyms = nr_func_ksyms;
 	info.jited_func_lens = ptr_to_u64(func_lens);
 	info.nr_jited_func_lens = nr_func_lens;
-	info.func_info_cnt = finfo_cnt;
+	info.nr_func_info = nr_finfo;
 	info.func_info_rec_size = finfo_rec_size;
 	info.func_info = ptr_to_u64(func_info);
-	info.line_info_cnt = linfo_cnt;
+	info.nr_line_info = nr_linfo;
 	info.line_info_rec_size = linfo_rec_size;
 	info.line_info = ptr_to_u64(linfo);
-	info.jited_line_info_cnt = jited_linfo_cnt;
+	info.nr_jited_line_info = nr_jited_linfo;
 	info.jited_line_info_rec_size = jited_linfo_rec_size;
 	info.jited_line_info = ptr_to_u64(jited_linfo);

@@ -614,9 +614,9 @@ static int do_dump(int argc, char **argv)
 		goto err_free;
 	}
 
-	if (info.func_info_cnt != finfo_cnt) {
-		p_err("incorrect func_info_cnt %d vs. expected %d",
-		      info.func_info_cnt, finfo_cnt);
+	if (info.nr_func_info != nr_finfo) {
+		p_err("incorrect nr_func_info %d vs. expected %d",
+		      info.nr_func_info, nr_finfo);
 		goto err_free;
 	}

@@ -630,12 +630,12 @@ static int do_dump(int argc, char **argv)
 		/* kernel.kptr_restrict is set.  No func_info available. */
 		free(func_info);
 		func_info = NULL;
-		finfo_cnt = 0;
+		nr_finfo = 0;
 	}
 
-	if (linfo && info.line_info_cnt != linfo_cnt) {
-		p_err("incorrect line_info_cnt %u vs. expected %u",
-		      info.line_info_cnt, linfo_cnt);
+	if (linfo && info.nr_line_info != nr_linfo) {
+		p_err("incorrect nr_line_info %u vs. expected %u",
+		      info.nr_line_info, nr_linfo);
 		goto err_free;
 	}

@@ -645,9 +645,9 @@ static int do_dump(int argc, char **argv)
 		goto err_free;
 	}
 
-	if (jited_linfo && info.jited_line_info_cnt != jited_linfo_cnt) {
-		p_err("incorrect jited_line_info_cnt %u vs. expected %u",
-		      info.jited_line_info_cnt, jited_linfo_cnt);
+	if (jited_linfo && info.nr_jited_line_info != nr_jited_linfo) {
+		p_err("incorrect nr_jited_line_info %u vs. expected %u",
+		      info.nr_jited_line_info, nr_jited_linfo);
 		goto err_free;
 	}

@@ -670,7 +670,7 @@ static int do_dump(int argc, char **argv)
 		goto err_free;
 	}
 
-	if (linfo_cnt) {
+	if (nr_linfo) {
 		prog_linfo = bpf_prog_linfo__new(&info);
 		if (!prog_linfo)
 			p_info("error in processing bpf_line_info.  continue without it.");

tools/lib/bpf/bpf_prog_linfo.c +2 -2

index b8af65145408..addd6e9971cc 100644
@@ -105,7 +105,7 @@ struct bpf_prog_linfo *bpf_prog_linfo__new(const struct bpf_prog_info *info)
 	struct bpf_prog_linfo *prog_linfo;
 	__u32 nr_linfo, nr_jited_func;
 
-	nr_linfo = info->line_info_cnt;
+	nr_linfo = info->nr_line_info;
 
 	/*
 	 * Test !info->line_info because the kernel may NULL

@@ -138,7 +138,7 @@ struct bpf_prog_linfo *bpf_prog_linfo__new(const struct bpf_prog_info *info)
 	nr_jited_func = info->nr_jited_ksyms;
 	if (!nr_jited_func ||
 	    !info->jited_line_info ||
-	    info->jited_line_info_cnt != nr_linfo ||
+	    info->nr_jited_line_info != nr_linfo ||
 	    info->jited_line_info_rec_size < sizeof(__u64) ||
 	    info->nr_jited_func_lens != nr_jited_func ||
 	    !info->jited_ksyms ||

tools/testing/selftests/bpf/test_btf.c +30 ~~-30~~

139 lines not on this page · raw message

Signature · 1 line

2.17.1

Alexei Starovoitov

Applied

2018-12-10 23:02 UTC #4 lore raw

In reply to Yonghong Song

Quoted 15 lines · Yonghong Song

Before func_info and line_info are added to the kernel, there are several fields in structure bpf_prog_info specifying the "count" of a user buffer, e.g.,
         __u32 nr_jited_ksyms;
         __u32 nr_jited_func_lens;
The naming convention has the prefix "nr_".
The func_info and line_info support added several fields
         __u32 func_info_cnt;
         __u32 line_info_cnt;
         __u32 jited_line_info_cnt;
to indicate the "count" of buffers func_info, line_info and jited_line_info. The original intention is to keep the field names the same as those in structure bpf_attr, so it will be clear that the "count" returned to user space will be the same as the one passed to the kernel during prog load.

Quoted 4 lines

Unfortunately, the field names *_info_cnt are not consistent with other existing fields in bpf_prog_info.
This patch set renamed the fields *_info_cnt to nr_*_info to keep naming convention consistent.

Applied to bpf-next. Thanks