* [PATCH bpf-next v4 06/13] tools/bpf: sync kernel uapi bpf.h header to tools directory
From: Yonghong Song @ 2018-11-08 20:37 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20181108203718.3139024-1-yhs@fb.com>
The kernel uapi bpf.h is synced to tools directory.
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/include/uapi/linux/bpf.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 852dc17ab47a..28db552a1eed 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -335,6 +335,10 @@ union bpf_attr {
* (context accesses, allowed helpers, etc).
*/
__u32 expected_attach_type;
+ __u32 prog_btf_fd; /* fd pointing to BTF type data */
+ __u32 func_info_rec_size; /* userspace bpf_func_info size */
+ __aligned_u64 func_info; /* func info */
+ __u32 func_info_cnt; /* number of bpf_func_info records */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -2631,6 +2635,10 @@ struct bpf_prog_info {
__u32 nr_jited_func_lens;
__aligned_u64 jited_ksyms;
__aligned_u64 jited_func_lens;
+ __u32 btf_id;
+ __u32 func_info_rec_size;
+ __aligned_u64 func_info;
+ __u32 func_info_cnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
@@ -2942,4 +2950,9 @@ struct bpf_flow_keys {
};
};
+struct bpf_func_info {
+ __u32 insn_offset;
+ __u32 type_id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v4 05/13] bpf: get better bpf_prog ksyms based on btf func type_id
From: Yonghong Song @ 2018-11-08 20:37 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20181108203718.3139024-1-yhs@fb.com>
This patch added interface to load a program with the following
additional information:
. prog_btf_fd
. func_info, func_info_rec_size and func_info_cnt
where func_info will provide function range and type_id
corresponding to each function.
The func_info_rec_size is introduced in the UAPI to specify
struct bpf_func_info size passed from user space. This
intends to make bpf_func_info structure growable in the future.
If the kernel gets a different bpf_func_info size from userspace,
it will try to handle user request with part of bpf_func_info
it can understand. In this patch, kernel can understand
struct bpf_func_info {
__u32 insn_offset;
__u32 type_id;
};
If user passed a bpf func_info record size of 16 bytes, the
kernel can still handle part of records with the above definition.
If verifier agrees with function range provided by the user,
the bpf_prog ksym for each function will use the func name
provided in the type_id, which is supposed to provide better
encoding as it is not limited by 16 bytes program name
limitation and this is better for bpf program which contains
multiple subprograms.
The bpf_prog_info interface is also extended to
return btf_id, func_info, func_info_rec_size and func_info_cnt
to userspace, so userspace can print out the function prototype
for each xlated function. The insn_offset in the returned
func_info corresponds to the insn offset for xlated functions.
With other jit related fields in bpf_prog_info, userspace can also
print out function prototypes for each jited function.
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/bpf.h | 5 +-
include/linux/bpf_verifier.h | 1 +
include/linux/btf.h | 2 +
include/uapi/linux/bpf.h | 13 ++++
kernel/bpf/btf.c | 4 +-
kernel/bpf/core.c | 13 ++++
kernel/bpf/syscall.c | 59 +++++++++++++++--
kernel/bpf/verifier.c | 121 ++++++++++++++++++++++++++++++++++-
8 files changed, 210 insertions(+), 8 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 33014ae73103..5835aa70e301 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -312,6 +312,8 @@ struct bpf_prog_aux {
void *security;
#endif
struct bpf_prog_offload *offload;
+ struct btf *btf;
+ u32 type_id; /* type id for this prog/func */
union {
struct work_struct work;
struct rcu_head rcu;
@@ -523,7 +525,8 @@ static inline void bpf_long_memcpy(void *dst, const void *src, u32 size)
}
/* verify correctness of eBPF program */
-int bpf_check(struct bpf_prog **fp, union bpf_attr *attr);
+int bpf_check(struct bpf_prog **fp, union bpf_attr *attr,
+ union bpf_attr __user *uattr);
void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth);
/* Map specifics */
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index d93e89761a8b..7f52528e6a06 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -204,6 +204,7 @@ static inline bool bpf_verifier_log_needed(const struct bpf_verifier_log *log)
struct bpf_subprog_info {
u32 start; /* insn idx of function entry point */
u16 stack_depth; /* max. stack depth used by this function */
+ u32 type_id; /* btf type_id for this subprog */
};
/* single container for all structs
diff --git a/include/linux/btf.h b/include/linux/btf.h
index e076c4697049..7f2c0a4a45ea 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -46,5 +46,7 @@ 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);
+const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
+const char *btf_name_by_offset(const struct btf *btf, u32 offset);
#endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 852dc17ab47a..28db552a1eed 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -335,6 +335,10 @@ union bpf_attr {
* (context accesses, allowed helpers, etc).
*/
__u32 expected_attach_type;
+ __u32 prog_btf_fd; /* fd pointing to BTF type data */
+ __u32 func_info_rec_size; /* userspace bpf_func_info size */
+ __aligned_u64 func_info; /* func info */
+ __u32 func_info_cnt; /* number of bpf_func_info records */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -2631,6 +2635,10 @@ struct bpf_prog_info {
__u32 nr_jited_func_lens;
__aligned_u64 jited_ksyms;
__aligned_u64 jited_func_lens;
+ __u32 btf_id;
+ __u32 func_info_rec_size;
+ __aligned_u64 func_info;
+ __u32 func_info_cnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
@@ -2942,4 +2950,9 @@ struct bpf_flow_keys {
};
};
+struct bpf_func_info {
+ __u32 insn_offset;
+ __u32 type_id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 62140da0c10d..4ff0a4593aab 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -477,7 +477,7 @@ static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
return !*src;
}
-static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
+const char *btf_name_by_offset(const struct btf *btf, u32 offset)
{
if (!offset)
return "(anon)";
@@ -487,7 +487,7 @@ static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
return "(invalid-name-offset)";
}
-static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
+const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
{
if (type_id > btf->nr_types)
return NULL;
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 1a796e0799ec..16d77012ad3e 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -21,12 +21,14 @@
* Kris Katterjohn - Added many additional checks in bpf_check_classic()
*/
+#include <uapi/linux/btf.h>
#include <linux/filter.h>
#include <linux/skbuff.h>
#include <linux/vmalloc.h>
#include <linux/random.h>
#include <linux/moduleloader.h>
#include <linux/bpf.h>
+#include <linux/btf.h>
#include <linux/frame.h>
#include <linux/rbtree_latch.h>
#include <linux/kallsyms.h>
@@ -390,6 +392,8 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
{
const char *end = sym + KSYM_NAME_LEN;
+ const struct btf_type *type;
+ const char *func_name;
BUILD_BUG_ON(sizeof("bpf_prog_") +
sizeof(prog->tag) * 2 +
@@ -404,6 +408,15 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
sym = bin2hex(sym, prog->tag, sizeof(prog->tag));
+
+ /* prog->aux->name will be ignored if full btf name is available */
+ if (prog->aux->btf) {
+ type = btf_type_by_id(prog->aux->btf, prog->aux->type_id);
+ func_name = btf_name_by_offset(prog->aux->btf, type->name_off);
+ snprintf(sym, (size_t)(end - sym), "_%s", func_name);
+ return;
+ }
+
if (prog->aux->name[0])
snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
else
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index cf5040fd5434..998377808102 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1213,6 +1213,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
/* bpf_prog_free_id() must be called first */
bpf_prog_free_id(prog, do_idr_lock);
bpf_prog_kallsyms_del_all(prog);
+ btf_put(prog->aux->btf);
call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
}
@@ -1437,9 +1438,9 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
}
/* last field in 'union bpf_attr' used by this command */
-#define BPF_PROG_LOAD_LAST_FIELD expected_attach_type
+#define BPF_PROG_LOAD_LAST_FIELD func_info_cnt
-static int bpf_prog_load(union bpf_attr *attr)
+static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
{
enum bpf_prog_type type = attr->prog_type;
struct bpf_prog *prog;
@@ -1525,7 +1526,7 @@ static int bpf_prog_load(union bpf_attr *attr)
goto free_prog;
/* run eBPF verifier */
- err = bpf_check(&prog, attr);
+ err = bpf_check(&prog, attr, uattr);
if (err < 0)
goto free_used_maps;
@@ -2079,6 +2080,7 @@ 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;
goto done;
}
@@ -2216,6 +2218,55 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
}
}
+ if (prog->aux->btf) {
+ u32 ucnt, urec_size;
+
+ info.btf_id = btf_id(prog->aux->btf);
+
+ ucnt = info.func_info_cnt;
+ info.func_info_cnt = prog->aux->func_cnt ? : 1;
+ urec_size = info.func_info_rec_size;
+ info.func_info_rec_size = sizeof(struct bpf_func_info);
+ if (ucnt) {
+ /* expect passed-in urec_size is what the kernel expects */
+ if (urec_size != info.func_info_rec_size)
+ return -EINVAL;
+
+ if (bpf_dump_raw_ok()) {
+ struct bpf_func_info kern_finfo;
+ char __user *user_finfo;
+ u32 i, insn_offset;
+
+ user_finfo = u64_to_user_ptr(info.func_info);
+ if (prog->aux->func_cnt) {
+ ucnt = min_t(u32, info.func_info_cnt, ucnt);
+ insn_offset = 0;
+ for (i = 0; i < ucnt; i++) {
+ kern_finfo.insn_offset = insn_offset;
+ kern_finfo.type_id = prog->aux->func[i]->aux->type_id;
+ if (copy_to_user(user_finfo, &kern_finfo,
+ sizeof(kern_finfo)))
+ return -EFAULT;
+
+ /* func[i]->len holds the prog len */
+ insn_offset += prog->aux->func[i]->len;
+ user_finfo += urec_size;
+ }
+ } else {
+ kern_finfo.insn_offset = 0;
+ kern_finfo.type_id = prog->aux->type_id;
+ if (copy_to_user(user_finfo, &kern_finfo,
+ sizeof(kern_finfo)))
+ return -EFAULT;
+ }
+ } else {
+ info.func_info_cnt = 0;
+ }
+ }
+ } else {
+ info.func_info_cnt = 0;
+ }
+
done:
if (copy_to_user(uinfo, &info, info_len) ||
put_user(info_len, &uattr->info.info_len))
@@ -2501,7 +2552,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
err = map_get_next_key(&attr);
break;
case BPF_PROG_LOAD:
- err = bpf_prog_load(&attr);
+ err = bpf_prog_load(&attr, uattr);
break;
case BPF_OBJ_PIN:
err = bpf_obj_pin(&attr);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1971ca325fb4..1b01a1bce147 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11,10 +11,12 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* General Public License for more details.
*/
+#include <uapi/linux/btf.h>
#include <linux/kernel.h>
#include <linux/types.h>
#include <linux/slab.h>
#include <linux/bpf.h>
+#include <linux/btf.h>
#include <linux/bpf_verifier.h>
#include <linux/filter.h>
#include <net/netlink.h>
@@ -4628,6 +4630,115 @@ static int check_cfg(struct bpf_verifier_env *env)
return ret;
}
+/* The minimum supported BTF func info size */
+#define MIN_BPF_FUNCINFO_SIZE 8
+#define MAX_FUNCINFO_REC_SIZE 252
+
+static int check_btf_func(struct bpf_prog *prog, struct bpf_verifier_env *env,
+ union bpf_attr *attr, union bpf_attr __user *uattr)
+{
+ u32 i, nfuncs, urec_size, min_size, prev_offset;
+ u32 krec_size = sizeof(struct bpf_func_info);
+ struct bpf_func_info krecord = {};
+ const struct btf_type *type;
+ void __user *urecord;
+ struct btf *btf;
+ int ret = 0;
+
+ nfuncs = attr->func_info_cnt;
+ if (!nfuncs)
+ return 0;
+
+ if (nfuncs != env->subprog_cnt) {
+ verbose(env, "number of funcs in func_info doesn't match number of subprogs\n");
+ return -EINVAL;
+ }
+
+ urec_size = attr->func_info_rec_size;
+ if (urec_size < MIN_BPF_FUNCINFO_SIZE ||
+ urec_size > MAX_FUNCINFO_REC_SIZE ||
+ urec_size % sizeof(u32)) {
+ verbose(env, "invalid func info rec size %u\n", urec_size);
+ return -EINVAL;
+ }
+
+ btf = btf_get_by_fd(attr->prog_btf_fd);
+ if (IS_ERR(btf)) {
+ verbose(env, "unable to get btf from fd\n");
+ return PTR_ERR(btf);
+ }
+
+ urecord = u64_to_user_ptr(attr->func_info);
+ min_size = min_t(u32, krec_size, urec_size);
+
+ for (i = 0; i < nfuncs; i++) {
+ ret = bpf_check_uarg_tail_zero(urecord, krec_size, urec_size);
+ if (ret) {
+ if (ret == -E2BIG) {
+ verbose(env, "nonzero tailing record in func info");
+ /* set the size kernel expects so loader can zero
+ * out the rest of the record.
+ */
+ if (put_user(min_size, &uattr->func_info_rec_size))
+ ret = -EFAULT;
+ }
+ goto free_btf;
+ }
+
+ if (copy_from_user(&krecord, urecord, min_size)) {
+ ret = -EFAULT;
+ goto free_btf;
+ }
+
+ /* check insn_offset */
+ if (i == 0) {
+ if (krecord.insn_offset) {
+ verbose(env,
+ "nonzero insn_offset %u for the first func info record",
+ krecord.insn_offset);
+ ret = -EINVAL;
+ goto free_btf;
+ }
+ } else if (krecord.insn_offset <= prev_offset) {
+ verbose(env,
+ "same or smaller insn offset (%u) than previous func info record (%u)",
+ krecord.insn_offset, prev_offset);
+ ret = -EINVAL;
+ goto free_btf;
+ }
+
+ if (env->subprog_info[i].start != krecord.insn_offset) {
+ verbose(env, "func_info BTF section doesn't match subprog layout in BPF program\n");
+ ret = -EINVAL;
+ goto free_btf;
+ }
+
+ /* check type_id */
+ type = btf_type_by_id(btf, krecord.type_id);
+ if (!type || !type->name_off ||
+ BTF_INFO_KIND(type->info) != BTF_KIND_FUNC) {
+ verbose(env, "invalid type id %d in func info",
+ krecord.type_id);
+ ret = -EINVAL;
+ goto free_btf;
+ }
+
+ if (i == 0)
+ prog->aux->type_id = krecord.type_id;
+ env->subprog_info[i].type_id = krecord.type_id;
+
+ prev_offset = krecord.insn_offset;
+ urecord += urec_size;
+ }
+
+ prog->aux->btf = btf;
+ return 0;
+
+free_btf:
+ btf_put(btf);
+ return ret;
+}
+
/* check %cur's range satisfies %old's */
static bool range_within(struct bpf_reg_state *old,
struct bpf_reg_state *cur)
@@ -5917,6 +6028,9 @@ static int jit_subprogs(struct bpf_verifier_env *env)
func[i]->aux->name[0] = 'F';
func[i]->aux->stack_depth = env->subprog_info[i].stack_depth;
func[i]->jit_requested = 1;
+ /* the btf will be freed only at prog->aux */
+ func[i]->aux->btf = prog->aux->btf;
+ func[i]->aux->type_id = env->subprog_info[i].type_id;
func[i] = bpf_int_jit_compile(func[i]);
if (!func[i]->jited) {
err = -ENOTSUPP;
@@ -6302,7 +6416,8 @@ static void free_states(struct bpf_verifier_env *env)
kfree(env->explored_states);
}
-int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
+int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
+ union bpf_attr __user *uattr)
{
struct bpf_verifier_env *env;
struct bpf_verifier_log *log;
@@ -6374,6 +6489,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr)
if (ret < 0)
goto skip_full_check;
+ ret = check_btf_func(env->prog, env, attr, uattr);
+ if (ret < 0)
+ goto skip_full_check;
+
ret = do_check(env);
if (env->cur_state) {
free_verifier_state(env->cur_state, true);
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v4 00/13] bpf: add btf func info support
From: Yonghong Song @ 2018-11-08 20:36 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
The BTF support was added to kernel by Commit 69b693f0aefa
("bpf: btf: Introduce BPF Type Format (BTF)"), which introduced
.BTF section into ELF file and is primarily
used for map pretty print.
pahole is used to convert dwarf to BTF for ELF files.
This patch added func info support to the kernel so we can
get better ksym's for bpf function calls. Basically,
function call types are passed to kernel and the kernel
extract function names from these types in order to contruct ksym
for these functions.
The llvm patch at https://reviews.llvm.org/D53736
will generate .BTF section and one more section .BTF.ext.
The .BTF.ext section encodes function type
information. The following is a sample output for selftests
test_btf with file test_btf_haskv.o for translated insns
and jited insns respectively.
$ bpftool prog dump xlated id 1
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
0: (85) call pc+2#bpf_prog_2dcecc18072623fc_test_long_fname_1
1: (b7) r0 = 0
2: (95) exit
int test_long_fname_1(struct dummy_tracepoint_args * arg):
3: (85) call pc+1#bpf_prog_89d64e4abf0f0126_test_long_fname_2
4: (95) exit
int test_long_fname_2(struct dummy_tracepoint_args * arg):
5: (b7) r2 = 0
6: (63) *(u32 *)(r10 -4) = r2
7: (79) r1 = *(u64 *)(r1 +8)
...
22: (07) r1 += 1
23: (63) *(u32 *)(r0 +4) = r1
24: (95) exit
$ bpftool prog dump jited id 1
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
bpf_prog_b07ccb89267cf242__dummy_tracepoint:
0: push %rbp
1: mov %rsp,%rbp
......
3c: add $0x28,%rbp
40: leaveq
41: retq
int test_long_fname_1(struct dummy_tracepoint_args * arg):
bpf_prog_2dcecc18072623fc_test_long_fname_1:
0: push %rbp
1: mov %rsp,%rbp
......
3a: add $0x28,%rbp
3e: leaveq
3f: retq
int test_long_fname_2(struct dummy_tracepoint_args * arg):
bpf_prog_89d64e4abf0f0126_test_long_fname_2:
0: push %rbp
1: mov %rsp,%rbp
......
80: add $0x28,%rbp
84: leaveq
85: retq
For the patchset,
Patch #1 refactors the code to break up btf_type_is_void().
Patch #2 introduces new BTF type BTF_KIND_FUNC.
Patch #3 syncs btf.h header to tools directory.
Patch #4 adds btf func type self tests in test_btf.
Patch #5 adds kernel interface to load func_info to kernel
and pass func_info back to userspace.
Patch #6 syncs bpf.h header to tools directory.
Patch #7 adds news btf/func_info related fields in libbpf
program load function.
Patch #8 extends selftest test_btf to test load/retrieve func_info.
Patch #9 adds .BTF.ext func info support.
Patch #10 changes Makefile to avoid using pahole if llvm is capable of
generating BTF sections.
Patch #11 refactors to have btf_get_from_id() in libbpf for reuse.
Patch #12 enhance test_btf file testing to test func info.
Patch #13 adds bpftool support for func signature dump.
Changelogs:
v3 -> v4:
. Remove BTF_KIND_FUNC_PROTO. BTF_KIND_FUNC is used for
both function pointer and subprogram. The name_off field
is used to distinguish both.
. The record size is added to the func_info subsection
in .BTF.ext to enable future extension.
. The bpf_prog_info interface change to make it similar
bpf_prog_load.
. Related kernel and libbpf changes to accommodate the
new .BTF.ext and kernel interface changes.
v2 -> v3:
. Removed kernel btf extern functions btf_type_id_func()
and btf_get_name_by_id(). Instead, exposing existing
functions btf_type_by_id() and btf_name_by_offset().
. Added comments about ELF section .BTF.ext layout.
. Better codes in btftool as suggested by Edward Cree.
v1 -> v2:
. Added missing sign-off.
. Limited the func_name/struct_member_name length for validity test.
. Removed/changed several verifier messages.
. Modified several commit messages to remove line_off reference.
Yonghong Song (13):
bpf: btf: Break up btf_type_is_void()
bpf: btf: Add BTF_KIND_FUNC
tools/bpf: sync kernel btf.h header
tools/bpf: add btf func unit tests in selftest test_btf
bpf: get better bpf_prog ksyms based on btf func type_id
tools/bpf: sync kernel uapi bpf.h header to tools directory
tools/bpf: add new fields for program load in lib/bpf
tools/bpf: extends test_btf to test load/retrieve func_type info
tools/bpf: add support to read .BTF.ext sections
tools/bpf: do not use pahole if clang/llvm can generate BTF sections
tools/bpf: refactor to implement btf_get_from_id() in lib/bpf
tools/bpf: enhance test_btf file testing to test func info
tools/bpf: bpftool: add support for func types
include/linux/bpf.h | 5 +-
include/linux/bpf_verifier.h | 1 +
include/linux/btf.h | 2 +
include/uapi/linux/bpf.h | 13 +
include/uapi/linux/btf.h | 17 +-
kernel/bpf/btf.c | 342 +++++++--
kernel/bpf/core.c | 13 +
kernel/bpf/syscall.c | 59 +-
kernel/bpf/verifier.c | 121 +++-
samples/bpf/Makefile | 8 +
tools/bpf/bpftool/btf_dumper.c | 98 +++
tools/bpf/bpftool/main.h | 2 +
tools/bpf/bpftool/map.c | 68 +-
tools/bpf/bpftool/prog.c | 56 ++
tools/bpf/bpftool/xlated_dumper.c | 33 +
tools/bpf/bpftool/xlated_dumper.h | 3 +
tools/include/uapi/linux/bpf.h | 13 +
tools/include/uapi/linux/btf.h | 17 +-
tools/lib/bpf/bpf.c | 50 +-
tools/lib/bpf/bpf.h | 4 +
tools/lib/bpf/btf.c | 346 +++++++++
tools/lib/bpf/btf.h | 51 ++
tools/lib/bpf/libbpf.c | 87 ++-
tools/testing/selftests/bpf/Makefile | 8 +
tools/testing/selftests/bpf/test_btf.c | 694 ++++++++++++++++++-
tools/testing/selftests/bpf/test_btf_haskv.c | 16 +-
tools/testing/selftests/bpf/test_btf_nokv.c | 16 +-
27 files changed, 1988 insertions(+), 155 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH bpf-next v4 01/13] bpf: btf: Break up btf_type_is_void()
From: Yonghong Song @ 2018-11-08 20:36 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team, Martin KaFai Lau
In-Reply-To: <20181108203657.3138259-1-yhs@fb.com>
This patch breaks up btf_type_is_void() into
btf_type_is_void() and btf_type_is_fwd().
It also adds btf_type_nosize() to better describe it is
testing a type has nosize info.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/btf.c | 37 ++++++++++++++++++++++---------------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ee4c82667d65..2a50d87de485 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -306,15 +306,22 @@ static bool btf_type_is_modifier(const struct btf_type *t)
static bool btf_type_is_void(const struct btf_type *t)
{
- /* void => no type and size info.
- * Hence, FWD is also treated as void.
- */
- return t == &btf_void || BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+ return t == &btf_void;
+}
+
+static bool btf_type_is_fwd(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+}
+
+static bool btf_type_nosize(const struct btf_type *t)
+{
+ return btf_type_is_void(t) || btf_type_is_fwd(t);
}
-static bool btf_type_is_void_or_null(const struct btf_type *t)
+static bool btf_type_nosize_or_null(const struct btf_type *t)
{
- return !t || btf_type_is_void(t);
+ return !t || btf_type_nosize(t);
}
/* union is only a special case of struct:
@@ -826,7 +833,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
u32 size = 0;
size_type = btf_type_by_id(btf, size_type_id);
- if (btf_type_is_void_or_null(size_type))
+ if (btf_type_nosize_or_null(size_type))
return NULL;
if (btf_type_has_size(size_type)) {
@@ -842,7 +849,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
size = btf->resolved_sizes[size_type_id];
size_type_id = btf->resolved_ids[size_type_id];
size_type = btf_type_by_id(btf, size_type_id);
- if (btf_type_is_void(size_type))
+ if (btf_type_nosize_or_null(size_type))
return NULL;
}
@@ -1164,7 +1171,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
}
/* "typedef void new_void", "const void"...etc */
- if (btf_type_is_void(next_type))
+ if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
goto resolved;
if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1178,7 +1185,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
* pretty print).
*/
if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
- !btf_type_is_void(btf_type_id_resolve(btf, &next_type_id))) {
+ !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL;
}
@@ -1205,7 +1212,7 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
}
/* "void *" */
- if (btf_type_is_void(next_type))
+ if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
goto resolved;
if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1235,7 +1242,7 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
}
if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
- !btf_type_is_void(btf_type_id_resolve(btf, &next_type_id))) {
+ !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL;
}
@@ -1396,7 +1403,7 @@ static int btf_array_resolve(struct btf_verifier_env *env,
/* Check array->index_type */
index_type_id = array->index_type;
index_type = btf_type_by_id(btf, index_type_id);
- if (btf_type_is_void_or_null(index_type)) {
+ if (btf_type_nosize_or_null(index_type)) {
btf_verifier_log_type(env, v->t, "Invalid index");
return -EINVAL;
}
@@ -1415,7 +1422,7 @@ static int btf_array_resolve(struct btf_verifier_env *env,
/* Check array->type */
elem_type_id = array->type;
elem_type = btf_type_by_id(btf, elem_type_id);
- if (btf_type_is_void_or_null(elem_type)) {
+ if (btf_type_nosize_or_null(elem_type)) {
btf_verifier_log_type(env, v->t,
"Invalid elem");
return -EINVAL;
@@ -1615,7 +1622,7 @@ static int btf_struct_resolve(struct btf_verifier_env *env,
const struct btf_type *member_type = btf_type_by_id(env->btf,
member_type_id);
- if (btf_type_is_void_or_null(member_type)) {
+ if (btf_type_nosize_or_null(member_type)) {
btf_verifier_log_member(env, v->t, member,
"Invalid member");
return -EINVAL;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next v4 02/13] bpf: btf: Add BTF_KIND_FUNC
From: Yonghong Song @ 2018-11-08 20:36 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team, Martin KaFai Lau
In-Reply-To: <20181108203657.3138259-1-yhs@fb.com>
This patch adds BTF_KIND_FUNC support to the type section.
BTF_KIND_FUNC is used to specify the signature of a
defined subprogram or the pointee of a function pointer.
In BTF, the function type related data structures are
struct bpf_param {
__u32 name_off; /* parameter name */
__u32 type; /* parameter type */
};
struct bpf_type {
__u32 name_off; /* function name */
__u32 info; /* BTF_KIND_FUNC and num of parameters (#vlen) */
__u32 type; /* return type */
}
The data layout of the function type:
struct bpf_type
#vlen number of bpf_param's
For a defined subprogram with valid function body,
. function name and all parameter names except the vararg
must be valid C identifier.
For the pointee of a function pointer,
. function name and all parameter names will
have name_off = 0 to indicate a non-existing name.
As a concrete example, for the C program below,
int foo(int (*bar)(int)) { return bar(5); }
two func types will be generated:
FuncType #1: subprogram "foo" with parameter "bar"
FuncType #2: pointee of function pointer "int (*)(int)"
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/uapi/linux/btf.h | 17 ++-
kernel/bpf/btf.c | 309 ++++++++++++++++++++++++++++++++++-----
2 files changed, 290 insertions(+), 36 deletions(-)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 972265f32871..9e7c74a83ee7 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -40,7 +40,8 @@ struct btf_type {
/* "size" is used by INT, ENUM, STRUCT and UNION.
* "size" tells the size of the type it is describing.
*
- * "type" is used by PTR, TYPEDEF, VOLATILE, CONST and RESTRICT.
+ * "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT
+ * and FUNC.
* "type" is a type_id referring to another type.
*/
union {
@@ -64,8 +65,9 @@ struct btf_type {
#define BTF_KIND_VOLATILE 9 /* Volatile */
#define BTF_KIND_CONST 10 /* Const */
#define BTF_KIND_RESTRICT 11 /* Restrict */
-#define BTF_KIND_MAX 11
-#define NR_BTF_KINDS 12
+#define BTF_KIND_FUNC 12 /* Function */
+#define BTF_KIND_MAX 12
+#define NR_BTF_KINDS 13
/* For some specific BTF_KIND, "struct btf_type" is immediately
* followed by extra data.
@@ -110,4 +112,13 @@ struct btf_member {
__u32 offset; /* offset in bits */
};
+/* BTF_KIND_FUNC is followed by multiple "struct btf_param".
+ * The exact number of btf_param is stored in the vlen (of the
+ * info in "struct btf_type").
+ */
+struct btf_param {
+ __u32 name_off;
+ __u32 type;
+};
+
#endif /* _UAPI__LINUX_BTF_H__ */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2a50d87de485..62140da0c10d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5,6 +5,7 @@
#include <uapi/linux/types.h>
#include <linux/seq_file.h>
#include <linux/compiler.h>
+#include <linux/ctype.h>
#include <linux/errno.h>
#include <linux/slab.h>
#include <linux/anon_inodes.h>
@@ -259,6 +260,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
[BTF_KIND_VOLATILE] = "VOLATILE",
[BTF_KIND_CONST] = "CONST",
[BTF_KIND_RESTRICT] = "RESTRICT",
+ [BTF_KIND_FUNC] = "FUNC",
};
struct btf_kind_operations {
@@ -281,6 +283,9 @@ struct btf_kind_operations {
static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS];
static struct btf_type btf_void;
+static int btf_resolve(struct btf_verifier_env *env,
+ const struct btf_type *t, u32 type_id);
+
static bool btf_type_is_modifier(const struct btf_type *t)
{
/* Some of them is not strictly a C modifier
@@ -314,9 +319,27 @@ static bool btf_type_is_fwd(const struct btf_type *t)
return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
}
+static bool btf_type_is_func(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC;
+}
+
+static bool btf_type_is_func_prog(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC &&
+ t->name_off;
+}
+
+static bool btf_type_is_func_proto(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC &&
+ !t->name_off;
+}
+
static bool btf_type_nosize(const struct btf_type *t)
{
- return btf_type_is_void(t) || btf_type_is_fwd(t);
+ return btf_type_is_void(t) || btf_type_is_fwd(t) ||
+ btf_type_is_func(t);
}
static bool btf_type_nosize_or_null(const struct btf_type *t)
@@ -433,6 +456,27 @@ static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
offset < btf->hdr.str_len;
}
+static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
+{
+ /* offset must be valid */
+ const char *src = &btf->strings[offset];
+ const char *src_limit;
+
+ if (!isalpha(*src) && *src != '_')
+ return false;
+
+ /* set a limit on identifier length */
+ src_limit = src + KSYM_NAME_LEN;
+ src++;
+ while (*src && src < src_limit) {
+ if (!isalnum(*src) && *src != '_')
+ return false;
+ src++;
+ }
+
+ return !*src;
+}
+
static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
{
if (!offset)
@@ -747,7 +791,9 @@ static bool env_type_is_resolve_sink(const struct btf_verifier_env *env,
/* int, enum or void is a sink */
return !btf_type_needs_resolve(next_type);
case RESOLVE_PTR:
- /* int, enum, void, struct or array is a sink for ptr */
+ /* int, enum, void, struct, array or func_proto is a sink
+ * for ptr
+ */
return !btf_type_is_modifier(next_type) &&
!btf_type_is_ptr(next_type);
case RESOLVE_STRUCT_OR_ARRAY:
@@ -1170,6 +1216,14 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (btf_type_is_func_proto(next_type)) {
+ if (env->resolve_mode == RESOLVE_PTR)
+ goto resolved;
+
+ btf_verifier_log_type(env, v->t, "Invalid type_id");
+ return -EINVAL;
+ }
+
/* "typedef void new_void", "const void"...etc */
if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
goto resolved;
@@ -1185,7 +1239,8 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
* pretty print).
*/
if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
- !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
+ (!btf_type_nosize(btf_type_id_resolve(btf, &next_type_id)) ||
+ btf_type_is_func_prog(next_type))) {
btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL;
}
@@ -1212,7 +1267,8 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
}
/* "void *" */
- if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
+ if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type) ||
+ btf_type_is_func_proto(next_type))
goto resolved;
if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1242,7 +1298,8 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
}
if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
- !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
+ (!btf_type_nosize(btf_type_id_resolve(btf, &next_type_id)) ||
+ btf_type_is_func_prog(next_type))) {
btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL;
}
@@ -1550,6 +1607,14 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
+ if (member->name_off &&
+ !btf_name_valid_identifier(btf, member->name_off)) {
+ btf_verifier_log_member(env, t, member,
+ "Invalid member name:%s",
+ btf_name_by_offset(btf, member->name_off));
+ return -EINVAL;
+ }
+
/* A member cannot be in type void */
if (!member->type || !BTF_TYPE_ID_VALID(member->type)) {
btf_verifier_log_member(env, t, member,
@@ -1787,6 +1852,174 @@ static struct btf_kind_operations enum_ops = {
.seq_show = btf_enum_seq_show,
};
+static s32 btf_func_check_meta(struct btf_verifier_env *env,
+ const struct btf_type *t,
+ u32 meta_left)
+{
+ u32 meta_needed = btf_type_vlen(t) * sizeof(struct btf_param);
+
+ if (meta_left < meta_needed) {
+ btf_verifier_log_basic(env, t,
+ "meta_left:%u meta_needed:%u",
+ meta_left, meta_needed);
+ return -EINVAL;
+ }
+
+ btf_verifier_log_type(env, t, NULL);
+
+ return meta_needed;
+}
+
+static void btf_func_log(struct btf_verifier_env *env,
+ const struct btf_type *t)
+{
+ u16 nr_args = btf_type_vlen(t), i;
+ struct btf_param *args = (struct btf_param *)(t + 1);
+
+ btf_verifier_log(env, "return=%u args=(", t->type);
+ if (!nr_args) {
+ btf_verifier_log(env, "void");
+ goto done;
+ }
+
+ if (nr_args == 1 && !args[0].type) {
+ /* Only one vararg */
+ btf_verifier_log(env, "vararg");
+ goto done;
+ }
+
+ btf_verifier_log(env, "%u %s", args[0].type,
+ btf_name_by_offset(env->btf,
+ args[0].name_off));
+ for (i = 1; i < nr_args - 1; i++)
+ btf_verifier_log(env, ", %u %s", args[i].type,
+ btf_name_by_offset(env->btf,
+ args[i].name_off));
+
+ if (nr_args > 1) {
+ struct btf_param *last_arg = &args[nr_args - 1];
+
+ if (last_arg->type)
+ btf_verifier_log(env, ", %u %s", last_arg->type,
+ btf_name_by_offset(env->btf,
+ last_arg->name_off));
+ else
+ btf_verifier_log(env, ", vararg");
+ }
+
+done:
+ btf_verifier_log(env, ")");
+}
+
+static int btf_func_check(struct btf_verifier_env *env,
+ const struct btf_type *t,
+ u32 type_id)
+{
+ u16 nr_args = btf_type_vlen(t), i;
+ const struct btf *btf = env->btf;
+ const struct btf_type *ret_type;
+ struct btf_param *args = (struct btf_param *)(t + 1);
+ u32 ret_type_id;
+ int err;
+
+ /* Check non-0 name_off must point to valid identifier */
+ if (t->name_off &&
+ !btf_name_valid_identifier(btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid type_id");
+ return -EINVAL;
+ }
+
+ /* Check func return type which could be "void" (t->type == 0) */
+ ret_type_id = t->type;
+ if (ret_type_id) {
+ /* return type is not "void" */
+ ret_type = btf_type_by_id(btf, ret_type_id);
+ if (btf_type_needs_resolve(ret_type) &&
+ !env_type_is_resolved(env, ret_type_id)) {
+ err = btf_resolve(env, ret_type, ret_type_id);
+ if (err)
+ return err;
+ }
+
+ /* Ensure the return type is a type that has a size */
+ if (!btf_type_id_size(btf, &ret_type_id, NULL)) {
+ btf_verifier_log_type(env, t, "Invalid return type");
+ return -EINVAL;
+ }
+ }
+
+ if (!nr_args)
+ return 0;
+
+ /* Last func arg type_id could be 0 if it is a vararg */
+ if (!args[nr_args - 1].type) {
+ if (args[nr_args - 1].name_off) {
+ btf_verifier_log_type(env, t, "Invalid arg#%u", nr_args);
+ return -EINVAL;
+ }
+ nr_args--;
+ }
+
+ err = 0;
+ for (i = 0; i < nr_args; i++) {
+ const struct btf_type *arg_type;
+ u32 arg_type_id;
+
+ arg_type_id = args[i].type;
+ arg_type = btf_type_by_id(btf, arg_type_id);
+ if (!arg_type ||
+ /* func has name, all its args must have names.
+ * func has no name, all its args cannot have names.
+ */
+ (!!args[i].name_off != !!t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
+ err = -EINVAL;
+ break;
+ }
+
+ if (args[i].name_off &&
+ (!btf_name_offset_valid(btf, args[i].name_off) ||
+ !btf_name_valid_identifier(btf, args[i].name_off))) {
+ btf_verifier_log_type(env, t,
+ "Invalid arg#%u", i + 1);
+ err = -EINVAL;
+ break;
+ }
+
+ if (btf_type_needs_resolve(arg_type) &&
+ !env_type_is_resolved(env, arg_type_id)) {
+ err = btf_resolve(env, arg_type, arg_type_id);
+ if (err)
+ break;
+ }
+
+ if (!btf_type_id_size(btf, &arg_type_id, NULL)) {
+ btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
+ err = -EINVAL;
+ break;
+ }
+ }
+
+ return err;
+}
+
+static struct btf_kind_operations func_ops = {
+ .check_meta = btf_func_check_meta,
+ .resolve = btf_df_resolve,
+ /*
+ * BTF_KIND_FUNC cannot be directly referred by
+ * a struct's member.
+ *
+ * It should be a funciton pointer instead.
+ * (i.e. struct's member -> BTF_KIND_PTR -> BTF_KIND_FUNC)
+ *
+ * Hence, there is no btf_func_check_member().
+ */
+ .check_member = btf_df_check_member,
+ .log_details = btf_func_log,
+ .seq_show = btf_df_seq_show,
+};
+
static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
[BTF_KIND_INT] = &int_ops,
[BTF_KIND_PTR] = &ptr_ops,
@@ -1799,6 +2032,7 @@ static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
[BTF_KIND_VOLATILE] = &modifier_ops,
[BTF_KIND_CONST] = &modifier_ops,
[BTF_KIND_RESTRICT] = &modifier_ops,
+ [BTF_KIND_FUNC] = &func_ops,
};
static s32 btf_check_meta(struct btf_verifier_env *env,
@@ -1870,30 +2104,6 @@ static int btf_check_all_metas(struct btf_verifier_env *env)
return 0;
}
-static int btf_resolve(struct btf_verifier_env *env,
- const struct btf_type *t, u32 type_id)
-{
- const struct resolve_vertex *v;
- int err = 0;
-
- env->resolve_mode = RESOLVE_TBD;
- env_stack_push(env, t, type_id);
- while (!err && (v = env_stack_peak(env))) {
- env->log_type_id = v->type_id;
- err = btf_type_ops(v->t)->resolve(env, v);
- }
-
- env->log_type_id = type_id;
- if (err == -E2BIG)
- btf_verifier_log_type(env, t,
- "Exceeded max resolving depth:%u",
- MAX_RESOLVE_DEPTH);
- else if (err == -EEXIST)
- btf_verifier_log_type(env, t, "Loop detected");
-
- return err;
-}
-
static bool btf_resolve_valid(struct btf_verifier_env *env,
const struct btf_type *t,
u32 type_id)
@@ -1927,6 +2137,39 @@ static bool btf_resolve_valid(struct btf_verifier_env *env,
return false;
}
+static int btf_resolve(struct btf_verifier_env *env,
+ const struct btf_type *t, u32 type_id)
+{
+ u32 save_log_type_id = env->log_type_id;
+ const struct resolve_vertex *v;
+ int err = 0;
+
+ env->resolve_mode = RESOLVE_TBD;
+ env_stack_push(env, t, type_id);
+ while (!err && (v = env_stack_peak(env))) {
+ env->log_type_id = v->type_id;
+ err = btf_type_ops(v->t)->resolve(env, v);
+ }
+
+ env->log_type_id = type_id;
+ if (err == -E2BIG) {
+ btf_verifier_log_type(env, t,
+ "Exceeded max resolving depth:%u",
+ MAX_RESOLVE_DEPTH);
+ } else if (err == -EEXIST) {
+ btf_verifier_log_type(env, t, "Loop detected");
+ }
+
+ /* Final sanity check */
+ if (!err && !btf_resolve_valid(env, t, type_id)) {
+ btf_verifier_log_type(env, t, "Invalid resolve state");
+ err = -EINVAL;
+ }
+
+ env->log_type_id = save_log_type_id;
+ return err;
+}
+
static int btf_check_all_types(struct btf_verifier_env *env)
{
struct btf *btf = env->btf;
@@ -1949,10 +2192,10 @@ static int btf_check_all_types(struct btf_verifier_env *env)
return err;
}
- if (btf_type_needs_resolve(t) &&
- !btf_resolve_valid(env, t, type_id)) {
- btf_verifier_log_type(env, t, "Invalid resolve state");
- return -EINVAL;
+ if (btf_type_is_func(t)) {
+ err = btf_func_check(env, t, type_id);
+ if (err)
+ return err;
}
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 2/2] net/mlx5: Fix offsets of ifc reserved fields
From: Saeed Mahameed @ 2018-11-08 20:34 UTC (permalink / raw)
To: davem@davemloft.net, pressmangal@gmail.com; +Cc: netdev@vger.kernel.org
In-Reply-To: <20181107183137.16774-2-pressmangal@gmail.com>
On Wed, 2018-11-07 at 20:31 +0200, Gal Pressman wrote:
> Fix wrong offsets of reserved fields in ifc file.
> Issues found using pahole.
>
> Signed-off-by: Gal Pressman <pressmangal@gmail.com>
> ---
Applied to mlx5-next branch will be sent later in a pull request to the
netdev maintainer.
Thanks,
Saeed.
^ permalink raw reply
* [PATCH v2 net-next 4/4] net: ethernet: ti: cpsw: fix vlan configuration while down/up
From: Ivan Khoronzhuk @ 2018-11-08 20:27 UTC (permalink / raw)
To: grygorii.strashko, davem
Cc: linux-omap, netdev, linux-kernel, alexander.h.duyck, bjorn,
Ivan Khoronzhuk
In-Reply-To: <20181108202757.30110-1-ivan.khoronzhuk@linaro.org>
The vlan configuration is not restored after interface donw/up sequence
(if dual-emac - both interfaces). Tested on am572x EVM.
Steps to check:
~# ip link add link eth1 name eth1.100 type vlan id 100
~# ifconfig eth0 down
~# ifconfig eth1 down
Try to remove vid and observe warning:
~# ip link del eth1.100
[ 739.526757] net eth1: removing vlanid 100 from vlan filter
[ 739.533322] failed to kill vid 0081/100 for device eth1
This patch fixes it, restoring only vlan ALE entries and all other
unicast/multicast entries are restored by system calling rx_mode ndo.
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/cpsw.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 0b18634d336c..9434fd5a5477 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -565,6 +565,9 @@ static const struct cpsw_stats cpsw_gstrings_ch_stats[] = {
(func)(slave++, ##arg); \
} while (0)
+static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
+ __be16 proto, u16 vid);
+
static inline int cpsw_get_slave_port(u32 slave_num)
{
return slave_num + 1;
@@ -1951,9 +1954,23 @@ static void cpsw_mqprio_resume(struct cpsw_slave *slave, struct cpsw_priv *priv)
slave_write(slave, tx_prio_map, tx_prio_rg);
}
+static int cpsw_restore_vlans(struct net_device *vdev, int vid, void *arg)
+{
+ struct cpsw_priv *priv = arg;
+
+ if (!vdev)
+ return 0;
+
+ cpsw_ndo_vlan_rx_add_vid(priv->ndev, 0, vid);
+ return 0;
+}
+
/* restore resources after port reset */
static void cpsw_restore(struct cpsw_priv *priv)
{
+ /* restore vlan configurations */
+ vlan_for_each(priv->ndev, cpsw_restore_vlans, priv);
+
/* restore MQPRIO offload */
for_each_slave(priv, cpsw_mqprio_resume, priv);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net/mlx5e: Cleanup unused defines
From: Saeed Mahameed @ 2018-11-08 20:30 UTC (permalink / raw)
To: davem@davemloft.net, pressmangal@gmail.com; +Cc: netdev@vger.kernel.org
In-Reply-To: <20181107183137.16774-1-pressmangal@gmail.com>
On Wed, 2018-11-07 at 20:31 +0200, Gal Pressman wrote:
> Remove couple of defines that are no longer used.
>
> Signed-off-by: Gal Pressman <pressmangal@gmail.com>
>
Applied to mlx5-next branch will be sent later in a pull request to the
netdev maintainer.
Thanks,
Saeed.
^ permalink raw reply
* [PATCH v2 net-next 1/4] net: core: dev_addr_lists: add auxiliary func to handle reference address updates
From: Ivan Khoronzhuk @ 2018-11-08 20:27 UTC (permalink / raw)
To: grygorii.strashko, davem
Cc: linux-omap, netdev, linux-kernel, alexander.h.duyck, bjorn,
Ivan Khoronzhuk
In-Reply-To: <20181108202757.30110-1-ivan.khoronzhuk@linaro.org>
In order to avoid all table update, and only remove or add new
address, the auxiliary function exists, named __hw_addr_sync_dev().
It allows end driver do nothing when nothing changed and add/rm when
concrete address is firstly added or lastly removed. But it doesn't
include cases when an address of real device or vlan was reused by
other vlans or vlan/macval devices.
For handaling events when address was reused/unreused the patch adds
new auxiliary routine - __hw_addr_ref_sync_dev(). It allows to do
nothing when nothing was changed and do updates only for an address
being added/reused/deleted/unreused. Thus, clone address changes for
vlans can be mirrored in the table. The function is exclusive with
__hw_addr_sync_dev(). It's responsibility of the end driver to
identify address vlan device, if it needs so.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
include/linux/netdevice.h | 10 ++++
net/core/dev_addr_lists.c | 97 +++++++++++++++++++++++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 857f8abf7b91..487fa5e0e165 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4068,6 +4068,16 @@ int __hw_addr_sync_dev(struct netdev_hw_addr_list *list,
int (*sync)(struct net_device *, const unsigned char *),
int (*unsync)(struct net_device *,
const unsigned char *));
+int __hw_addr_ref_sync_dev(struct netdev_hw_addr_list *list,
+ struct net_device *dev,
+ int (*sync)(struct net_device *,
+ const unsigned char *, int),
+ int (*unsync)(struct net_device *,
+ const unsigned char *, int));
+void __hw_addr_ref_unsync_dev(struct netdev_hw_addr_list *list,
+ struct net_device *dev,
+ int (*unsync)(struct net_device *,
+ const unsigned char *, int));
void __hw_addr_unsync_dev(struct netdev_hw_addr_list *list,
struct net_device *dev,
int (*unsync)(struct net_device *,
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index d884d8f5f0e5..81a8cd4ea3bd 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -277,6 +277,103 @@ int __hw_addr_sync_dev(struct netdev_hw_addr_list *list,
}
EXPORT_SYMBOL(__hw_addr_sync_dev);
+/**
+ * __hw_addr_ref_sync_dev - Synchronize device's multicast address list taking
+ * into account references
+ * @list: address list to synchronize
+ * @dev: device to sync
+ * @sync: function to call if address or reference on it should be added
+ * @unsync: function to call if address or some reference on it should removed
+ *
+ * This function is intended to be called from the ndo_set_rx_mode
+ * function of devices that require explicit address or references on it
+ * add/remove notifications. The unsync function may be NULL in which case
+ * the addresses or references on it requiring removal will simply be
+ * removed without any notification to the device. That is responsibility of
+ * the driver to identify and distribute address or references on it between
+ * internal address tables.
+ **/
+int __hw_addr_ref_sync_dev(struct netdev_hw_addr_list *list,
+ struct net_device *dev,
+ int (*sync)(struct net_device *,
+ const unsigned char *, int),
+ int (*unsync)(struct net_device *,
+ const unsigned char *, int))
+{
+ struct netdev_hw_addr *ha, *tmp;
+ int err, ref_cnt;
+
+ /* first go through and flush out any unsynced/stale entries */
+ list_for_each_entry_safe(ha, tmp, &list->list, list) {
+ /* sync if address is not used */
+ if ((ha->sync_cnt << 1) <= ha->refcount)
+ continue;
+
+ /* if fails defer unsyncing address */
+ ref_cnt = ha->refcount - ha->sync_cnt;
+ if (unsync && unsync(dev, ha->addr, ref_cnt))
+ continue;
+
+ ha->refcount = (ref_cnt << 1) + 1;
+ ha->sync_cnt = ref_cnt;
+ __hw_addr_del_entry(list, ha, false, false);
+ }
+
+ /* go through and sync updated/new entries to the list */
+ list_for_each_entry_safe(ha, tmp, &list->list, list) {
+ /* sync if address added or reused */
+ if ((ha->sync_cnt << 1) >= ha->refcount)
+ continue;
+
+ ref_cnt = ha->refcount - ha->sync_cnt;
+ err = sync(dev, ha->addr, ref_cnt);
+ if (err)
+ return err;
+
+ ha->refcount = ref_cnt << 1;
+ ha->sync_cnt = ref_cnt;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(__hw_addr_ref_sync_dev);
+
+/**
+ * __hw_addr_ref_unsync_dev - Remove synchronized addresses and references on
+ * it from device
+ * @list: address list to remove synchronized addresses (references on it) from
+ * @dev: device to sync
+ * @unsync: function to call if address and references on it should be removed
+ *
+ * Remove all addresses that were added to the device by
+ * __hw_addr_ref_sync_dev(). This function is intended to be called from the
+ * ndo_stop or ndo_open functions on devices that require explicit address (or
+ * references on it) add/remove notifications. If the unsync function pointer
+ * is NULL then this function can be used to just reset the sync_cnt for the
+ * addresses in the list.
+ **/
+void __hw_addr_ref_unsync_dev(struct netdev_hw_addr_list *list,
+ struct net_device *dev,
+ int (*unsync)(struct net_device *,
+ const unsigned char *, int))
+{
+ struct netdev_hw_addr *ha, *tmp;
+
+ list_for_each_entry_safe(ha, tmp, &list->list, list) {
+ if (!ha->sync_cnt)
+ continue;
+
+ /* if fails defer unsyncing address */
+ if (unsync && unsync(dev, ha->addr, ha->sync_cnt))
+ continue;
+
+ ha->refcount -= ha->sync_cnt - 1;
+ ha->sync_cnt = 0;
+ __hw_addr_del_entry(list, ha, false, false);
+ }
+}
+EXPORT_SYMBOL(__hw_addr_ref_unsync_dev);
+
/**
* __hw_addr_unsync_dev - Remove synchronized addresses from device
* @list: address list to remove synchronized addresses from
--
2.17.1
^ permalink raw reply related
* Re: [Patch net-next] net: move __skb_checksum_complete*() to skbuff.c
From: Cong Wang @ 2018-11-08 20:26 UTC (permalink / raw)
To: Stefano Brivio; +Cc: Linux Kernel Network Developers
In-Reply-To: <20181108211435.1f8d953f@redhat.com>
On Thu, Nov 8, 2018 at 12:14 PM Stefano Brivio <sbrivio@redhat.com> wrote:
>
> On Thu, 8 Nov 2018 12:02:00 -0800
> Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> > On Thu, Nov 8, 2018 at 11:54 AM Stefano Brivio <sbrivio@redhat.com> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 8 Nov 2018 11:49:49 -0800
> > > Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > >
> > > > +EXPORT_SYMBOL(__skb_checksum_complete);
> > > > +
> > > > /* Both of above in one bottle. */
> > >
> > > Maybe you should also update/drop this comment now?
> >
> > I have no idea what that comment means. Do you?
>
> I think it refers to the fact that skb_copy_and_csum_bits() implements
> both skb_checksum() and skb_copy_bits(), that were just above it at
> 1da177e4c3f4.
>
> Then more stuff was moved in between, and the comment was never updated
> or dropped.
I don't want to touch what I don't understand. So, let me just move
these two after skb_copy_and_csum_bits().
^ permalink raw reply
* Re: [PATCH] net: Add trace events for all receive exit points
From: Mathieu Desnoyers @ 2018-11-08 20:25 UTC (permalink / raw)
To: Geneviève Bastien; +Cc: David S. Miller, netdev, rostedt, Ingo Molnar
In-Reply-To: <20181108195648.31846-1-gbastien@versatic.net>
----- On Nov 8, 2018, at 2:56 PM, Geneviève Bastien gbastien@versatic.net wrote:
> Trace events are already present for the receive entry points, to indicate
> how the reception entered the stack.
>
> This patch adds the corresponding exit trace events that will bound the
> reception such that all events occurring between the entry and the exit
> can be considered as part of the reception context. This greatly helps
> for dependency and root cause analyses.
>
> Without this, it is impossible to determine whether a sched_wakeup
> event following a netif_receive_skb event is the result of the packet
> reception or a simple coincidence after further processing by the
> thread.
As a clarification: it is indeed not possible with tracepoint instrumentation,
which I think is your point here. It might be possible to do so by using other
mechanisms like kretprobes, but considering that the "entry" point was deemed
important enough to have a tracepoint, it would be good to add the matching exit
events.
Being able to link the packet reception entry/exit with wakeups is key to
allow tools to perform automated network stack latency analysis, so I think
the use case justifies adding the missing "exit" events.
It might be great if you can provide a glimpse of the wakeup dependency chain
analysis prototypes you have created, which rely on this new event, in order
to justify adding it.
Thanks,
Mathieu
>
> Signed-off-by: Geneviève Bastien <gbastien@versatic.net>
> CC: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@redhat.com>
> CC: David S. Miller <davem@davemloft.net>
> ---
> include/trace/events/net.h | 59 ++++++++++++++++++++++++++++++++++++++
> net/core/dev.c | 30 ++++++++++++++++---
> 2 files changed, 85 insertions(+), 4 deletions(-)
>
> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index 00aa72ce0e7c..318307511018 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -117,6 +117,23 @@ DECLARE_EVENT_CLASS(net_dev_template,
> __get_str(name), __entry->skbaddr, __entry->len)
> )
>
> +DECLARE_EVENT_CLASS(net_dev_template_simple,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb),
> +
> + TP_STRUCT__entry(
> + __field(void *, skbaddr)
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + ),
> +
> + TP_printk("skbaddr=%p", __entry->skbaddr)
> +)
> +
> DEFINE_EVENT(net_dev_template, net_dev_queue,
>
> TP_PROTO(struct sk_buff *skb),
> @@ -244,6 +261,48 @@ DEFINE_EVENT(net_dev_rx_verbose_template,
> netif_rx_ni_entry,
> TP_ARGS(skb)
> );
>
> +DEFINE_EVENT(net_dev_template_simple, napi_gro_frags_exit,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, napi_gro_receive_exit,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_exit,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_list_exit,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, netif_rx_exit,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> +DEFINE_EVENT(net_dev_template_simple, netif_rx_ni_exit,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb)
> +);
> +
> #endif /* _TRACE_NET_H */
>
> /* This part must be outside protection */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 0ffcbdd55fa9..e670ca27e829 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4520,9 +4520,14 @@ static int netif_rx_internal(struct sk_buff *skb)
>
> int netif_rx(struct sk_buff *skb)
> {
> + int ret;
> +
> trace_netif_rx_entry(skb);
>
> - return netif_rx_internal(skb);
> + ret = netif_rx_internal(skb);
> + trace_netif_rx_exit(skb);
> +
> + return ret;
> }
> EXPORT_SYMBOL(netif_rx);
>
> @@ -4537,6 +4542,7 @@ int netif_rx_ni(struct sk_buff *skb)
> if (local_softirq_pending())
> do_softirq();
> preempt_enable();
> + trace_netif_rx_ni_exit(skb);
>
> return err;
> }
> @@ -5222,9 +5228,14 @@ static void netif_receive_skb_list_internal(struct
> list_head *head)
> */
> int netif_receive_skb(struct sk_buff *skb)
> {
> + int ret;
> +
> trace_netif_receive_skb_entry(skb);
>
> - return netif_receive_skb_internal(skb);
> + ret = netif_receive_skb_internal(skb);
> + trace_netif_receive_skb_exit(skb);
> +
> + return ret;
> }
> EXPORT_SYMBOL(netif_receive_skb);
>
> @@ -5247,6 +5258,8 @@ void netif_receive_skb_list(struct list_head *head)
> list_for_each_entry(skb, head, list)
> trace_netif_receive_skb_list_entry(skb);
> netif_receive_skb_list_internal(head);
> + list_for_each_entry(skb, head, list)
> + trace_netif_receive_skb_list_exit(skb);
> }
> EXPORT_SYMBOL(netif_receive_skb_list);
>
> @@ -5634,12 +5647,17 @@ static gro_result_t napi_skb_finish(gro_result_t ret,
> struct sk_buff *skb)
>
> gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
> {
> + gro_result_t ret;
> +
> skb_mark_napi_id(skb, napi);
> trace_napi_gro_receive_entry(skb);
>
> skb_gro_reset_offset(skb);
>
> - return napi_skb_finish(dev_gro_receive(napi, skb), skb);
> + ret = napi_skb_finish(dev_gro_receive(napi, skb), skb);
> + trace_napi_gro_receive_exit(skb);
> +
> + return ret;
> }
> EXPORT_SYMBOL(napi_gro_receive);
>
> @@ -5753,6 +5771,7 @@ static struct sk_buff *napi_frags_skb(struct napi_struct
> *napi)
>
> gro_result_t napi_gro_frags(struct napi_struct *napi)
> {
> + gro_result_t ret;
> struct sk_buff *skb = napi_frags_skb(napi);
>
> if (!skb)
> @@ -5760,7 +5779,10 @@ gro_result_t napi_gro_frags(struct napi_struct *napi)
>
> trace_napi_gro_frags_entry(skb);
>
> - return napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
> + ret = napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
> + trace_napi_gro_frags_exit(skb);
> +
> + return ret;
> }
> EXPORT_SYMBOL(napi_gro_frags);
>
> --
> 2.19.1
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* RE: [PATCH net-next] dpaa2-eth: Introduce TX congestion management
From: Ioana Ciocoi Radulescu @ 2018-11-08 20:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, Ioana Ciornei
In-Reply-To: <20181107.220759.1889877577682317113.davem@davemloft.net>
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Thursday, November 8, 2018 8:08 AM
> To: Ioana Ciocoi Radulescu <ruxandra.radulescu@nxp.com>
> Cc: netdev@vger.kernel.org; Ioana Ciornei <ioana.ciornei@nxp.com>
> Subject: Re: [PATCH net-next] dpaa2-eth: Introduce TX congestion
> management
>
> From: Ioana Ciocoi Radulescu <ruxandra.radulescu@nxp.com>
> Date: Wed, 7 Nov 2018 10:31:16 +0000
>
> > We chose this mechanism over BQL (to which it is conceptually
> > very similar) because a) we can take advantage of the hardware
> > offloading and b) BQL doesn't match well with our driver fastpath
> > (we process ingress (Rx or Tx conf) frames in batches of up to 16,
> > which in certain scenarios confuses the BQL adaptive algorithm,
> > resulting in too low values of the limit and low performance).
>
> First, this kind of explanation belongs in the commit message.
>
> Second, you'll have to describe better what BQL, which is the
> ultimate standard mechanism for every single driver in the
> kernel to deal with this issue.
>
> Are you saying that if 15 TX frames are pending, not TX interrupt
> will arrive at all?
I meant up to 16, not exactly 16.
>
> There absolutely must be some timeout or similar interrupt that gets
> sent in that kind of situation. You cannot leave stale TX packets
> on your ring unprocessed just because a non-multiple of 16 packets
> were queued up and then TX activity stopped.
I'll try to detail a bit how our hardware works, since it's not the standard
ring-based architecture.
In our driver implementation, we have multiple ingress queues (for Rx frames
and Tx confirmation frames) grouped into core-affine channels. Each channel
may contain one or more queues of each type.
When queues inside a channel have available frames, the hardware triggers
a notification for us; we then issue a dequeue command for that channel,
in response to which hardware will write a number of frames (between 1 and
16) at a memory location of our choice. Frames obtained as a result of one
dequeue operation are all from the same queue, but consecutive dequeues
on the same channel may service different queues.
So my initial attempt at implementing BQL called netdev_tx_completed_queue()
for each batch of Tx confirmation frames obtained from a single dequeue
operation. I don't fully understand the BQL algorithm yet, but I think it had
a problem with the fact that we never reported as completed more than
16 frames at a time.
Consider a scenario where a DPAA2 platform does IP forwarding from port1
to port2; to keep things simple, let's say we're single core and have only
one Rx and one Tx confirmation queue per interface.
Without BQL, the egress interface can transmit up to 64 frames at a time
(one for each Rx frame processed by the ingress interface in its NAPI poll)
and then process all 64 confirmations (grouped in 4 dequeues) during its
own NAPI cycle.
With BQL support added, the number of consecutive transmitted frames is
never higher than 33; after that, BQL stops the netdev tx queue until
we mark some of those trasmits as completed. This results in a performance
drop of over 30%.
Today I tried to further coalesce the confirmation frames such that I call
netdev_tx_completed_queue() only at the end of the NAPI poll, once for each
confirmation queue that was serviced during that NAPI. I need to do more
testing, but so far it performs *almost* on par with the non-BQL driver
version. But it does complicate the fastpath code and feels somewhat
unnatural.
So I would still prefer using the hardware mechanism, which I think fits
more smoothly with both our hardware and driver implementation, and adds
less overhead. But if you feel strongly about it I can drop this patch and
polish the BQL support until it's looks decent enough (and hopefully doesn't
mess too much with our performance benchmarks).
Thanks,
Ioana
^ permalink raw reply
* Re: [Patch net-next] net: move __skb_checksum_complete*() to skbuff.c
From: Stefano Brivio @ 2018-11-08 20:14 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpXh03pCAp-t-Zh-etTPRbiBwCSuJLL5LTfr5EK0CT=Qzg@mail.gmail.com>
On Thu, 8 Nov 2018 12:02:00 -0800
Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, Nov 8, 2018 at 11:54 AM Stefano Brivio <sbrivio@redhat.com> wrote:
> >
> > Hi,
> >
> > On Thu, 8 Nov 2018 11:49:49 -0800
> > Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > > +EXPORT_SYMBOL(__skb_checksum_complete);
> > > +
> > > /* Both of above in one bottle. */
> >
> > Maybe you should also update/drop this comment now?
>
> I have no idea what that comment means. Do you?
I think it refers to the fact that skb_copy_and_csum_bits() implements
both skb_checksum() and skb_copy_bits(), that were just above it at
1da177e4c3f4.
Then more stuff was moved in between, and the comment was never updated
or dropped.
--
Stefano
^ permalink raw reply
* Re: [PATCH net-next] sfc: use the new __netdev_tx_sent_queue BQL optimisation
From: Eric Dumazet @ 2018-11-08 20:13 UTC (permalink / raw)
To: Edward Cree, linux-net-drivers, davem; +Cc: netdev
In-Reply-To: <31ee2a24-103c-ac0a-9e60-b3204bd61167@solarflare.com>
On 11/08/2018 11:47 AM, Edward Cree wrote:
> As added in 3e59020abf0f ("net: bql: add __netdev_tx_sent_queue()"), which
> see for performance rationale.
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
> ---
> drivers/net/ethernet/sfc/tx.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
> index c3ad564ac4c0..22eb059086f7 100644
> --- a/drivers/net/ethernet/sfc/tx.c
> +++ b/drivers/net/ethernet/sfc/tx.c
> @@ -553,13 +553,10 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
> if (!data_mapped && (efx_tx_map_data(tx_queue, skb, segments)))
> goto err;
>
> - /* Update BQL */
> - netdev_tx_sent_queue(tx_queue->core_txq, skb_len);
> -
> efx_tx_maybe_stop_queue(tx_queue);
>
> /* Pass off to hardware */
> - if (!xmit_more || netif_xmit_stopped(tx_queue->core_txq)) {
> + if (__netdev_tx_sent_queue(tx_queue->core_txq, skb_len, xmit_more)) {
> struct efx_tx_queue *txq2 = efx_tx_queue_partner(tx_queue);
>
> /* There could be packets left on the partner queue if those
>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: phy: use phy_id_mask value zero for exact match
From: Heiner Kallweit @ 2018-11-08 20:06 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <827c53cd-c9f5-7b1c-84fd-af1a49317fe7@gmail.com>
On 08.11.2018 20:44, Florian Fainelli wrote:
> On 11/7/18 12:53 PM, Heiner Kallweit wrote:
>> A phy_id_mask value zero means every PHYID matches, therefore
>> value zero isn't used. So we can safely redefine the semantics
>> of value zero to mean "exact match". This allows to avoid some
>> boilerplate code in PHY driver configs.
>
> Having run recently into some ethtool quirkyness about how masks are
> supposed to be specified between ntuple/nfc, where the meaning of 0 is
> either don't care or match, I would rather we stick with the current
> behavior where every bit set to 0 is a don't care and bits set t 1 are not.
>
I agree that using a mask value 0 as either exact match or don't care
can be confusing. However I think that the advantages here outweigh
this confusion aspect.
- We get a meaningful default in case a driver author misses to set
the phy_id_mask.
- If a driver author wants to enforce an exact match, he has to do
nothing and can rely on the core. This avoids mistakes like in the
Realtek case where the driver author meant exact match but specified
something completely different.
- Avoid boilerplate code
> Maybe we can find a clever way with a macro to specify only the PHY OUI
> and compute a suitable mask automatically?
>
I don't think so. For Realtek each driver is specific even to a model
revision (therefore mask 0xffffffff). Same applies to intel-xway.
In broadcom.c we have masks 0xfffffff0, so for each model, independent
of revision number. There is no general rule.
Also we can't simply check for the first-bit-set to derive a mask.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/net/phy/phy_device.c | 21 +++++++++++++++------
>> include/linux/phy.h | 2 +-
>> 2 files changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index ab33d1777..d165a2c82 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -483,15 +483,24 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
>> if (!(phydev->c45_ids.devices_in_package & (1 << i)))
>> continue;
>>
>> - if ((phydrv->phy_id & phydrv->phy_id_mask) ==
>> - (phydev->c45_ids.device_ids[i] &
>> - phydrv->phy_id_mask))
>> - return 1;
>> + if (!phydrv->phy_id_mask) {
>> + if (phydrv->phy_id ==
>> + phydev->c45_ids.device_ids[i])
>> + return 1;
>> + } else {
>> + if ((phydrv->phy_id & phydrv->phy_id_mask) ==
>> + (phydev->c45_ids.device_ids[i] &
>> + phydrv->phy_id_mask))
>> + return 1;
>> + }
>> }
>> return 0;
>> } else {
>> - return (phydrv->phy_id & phydrv->phy_id_mask) ==
>> - (phydev->phy_id & phydrv->phy_id_mask);
>> + if (!phydrv->phy_id_mask)
>> + return phydrv->phy_id == phydev->phy_id;
>> + else
>> + return (phydrv->phy_id & phydrv->phy_id_mask) ==
>> + (phydev->phy_id & phydrv->phy_id_mask);
>> }
>> }
>>
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 2090277ea..e30ca2fdd 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -500,7 +500,7 @@ struct phy_driver {
>> struct mdio_driver_common mdiodrv;
>> u32 phy_id;
>> char *name;
>> - u32 phy_id_mask;
>> + u32 phy_id_mask; /* value 0 means exact match */
>> const unsigned long * const features;
>> u32 flags;
>> const void *driver_data;
>>
>
>
^ permalink raw reply
* [PATCH] net: Add trace events for all receive exit points
From: Geneviève Bastien @ 2018-11-08 19:56 UTC (permalink / raw)
To: davem
Cc: netdev, Geneviève Bastien, Mathieu Desnoyers, Steven Rostedt,
Ingo Molnar
Trace events are already present for the receive entry points, to indicate
how the reception entered the stack.
This patch adds the corresponding exit trace events that will bound the
reception such that all events occurring between the entry and the exit
can be considered as part of the reception context. This greatly helps
for dependency and root cause analyses.
Without this, it is impossible to determine whether a sched_wakeup
event following a netif_receive_skb event is the result of the packet
reception or a simple coincidence after further processing by the
thread.
Signed-off-by: Geneviève Bastien <gbastien@versatic.net>
CC: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@redhat.com>
CC: David S. Miller <davem@davemloft.net>
---
include/trace/events/net.h | 59 ++++++++++++++++++++++++++++++++++++++
net/core/dev.c | 30 ++++++++++++++++---
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/include/trace/events/net.h b/include/trace/events/net.h
index 00aa72ce0e7c..318307511018 100644
--- a/include/trace/events/net.h
+++ b/include/trace/events/net.h
@@ -117,6 +117,23 @@ DECLARE_EVENT_CLASS(net_dev_template,
__get_str(name), __entry->skbaddr, __entry->len)
)
+DECLARE_EVENT_CLASS(net_dev_template_simple,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field(void *, skbaddr)
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ ),
+
+ TP_printk("skbaddr=%p", __entry->skbaddr)
+)
+
DEFINE_EVENT(net_dev_template, net_dev_queue,
TP_PROTO(struct sk_buff *skb),
@@ -244,6 +261,48 @@ DEFINE_EVENT(net_dev_rx_verbose_template, netif_rx_ni_entry,
TP_ARGS(skb)
);
+DEFINE_EVENT(net_dev_template_simple, napi_gro_frags_exit,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb)
+);
+
+DEFINE_EVENT(net_dev_template_simple, napi_gro_receive_exit,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb)
+);
+
+DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_exit,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb)
+);
+
+DEFINE_EVENT(net_dev_template_simple, netif_receive_skb_list_exit,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb)
+);
+
+DEFINE_EVENT(net_dev_template_simple, netif_rx_exit,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb)
+);
+
+DEFINE_EVENT(net_dev_template_simple, netif_rx_ni_exit,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb)
+);
+
#endif /* _TRACE_NET_H */
/* This part must be outside protection */
diff --git a/net/core/dev.c b/net/core/dev.c
index 0ffcbdd55fa9..e670ca27e829 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4520,9 +4520,14 @@ static int netif_rx_internal(struct sk_buff *skb)
int netif_rx(struct sk_buff *skb)
{
+ int ret;
+
trace_netif_rx_entry(skb);
- return netif_rx_internal(skb);
+ ret = netif_rx_internal(skb);
+ trace_netif_rx_exit(skb);
+
+ return ret;
}
EXPORT_SYMBOL(netif_rx);
@@ -4537,6 +4542,7 @@ int netif_rx_ni(struct sk_buff *skb)
if (local_softirq_pending())
do_softirq();
preempt_enable();
+ trace_netif_rx_ni_exit(skb);
return err;
}
@@ -5222,9 +5228,14 @@ static void netif_receive_skb_list_internal(struct list_head *head)
*/
int netif_receive_skb(struct sk_buff *skb)
{
+ int ret;
+
trace_netif_receive_skb_entry(skb);
- return netif_receive_skb_internal(skb);
+ ret = netif_receive_skb_internal(skb);
+ trace_netif_receive_skb_exit(skb);
+
+ return ret;
}
EXPORT_SYMBOL(netif_receive_skb);
@@ -5247,6 +5258,8 @@ void netif_receive_skb_list(struct list_head *head)
list_for_each_entry(skb, head, list)
trace_netif_receive_skb_list_entry(skb);
netif_receive_skb_list_internal(head);
+ list_for_each_entry(skb, head, list)
+ trace_netif_receive_skb_list_exit(skb);
}
EXPORT_SYMBOL(netif_receive_skb_list);
@@ -5634,12 +5647,17 @@ static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
{
+ gro_result_t ret;
+
skb_mark_napi_id(skb, napi);
trace_napi_gro_receive_entry(skb);
skb_gro_reset_offset(skb);
- return napi_skb_finish(dev_gro_receive(napi, skb), skb);
+ ret = napi_skb_finish(dev_gro_receive(napi, skb), skb);
+ trace_napi_gro_receive_exit(skb);
+
+ return ret;
}
EXPORT_SYMBOL(napi_gro_receive);
@@ -5753,6 +5771,7 @@ static struct sk_buff *napi_frags_skb(struct napi_struct *napi)
gro_result_t napi_gro_frags(struct napi_struct *napi)
{
+ gro_result_t ret;
struct sk_buff *skb = napi_frags_skb(napi);
if (!skb)
@@ -5760,7 +5779,10 @@ gro_result_t napi_gro_frags(struct napi_struct *napi)
trace_napi_gro_frags_entry(skb);
- return napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
+ ret = napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));
+ trace_napi_gro_frags_exit(skb);
+
+ return ret;
}
EXPORT_SYMBOL(napi_gro_frags);
--
2.19.1
^ permalink raw reply related
* Re: [Patch net-next] net: move __skb_checksum_complete*() to skbuff.c
From: Cong Wang @ 2018-11-08 20:02 UTC (permalink / raw)
To: Stefano Brivio; +Cc: Linux Kernel Network Developers
In-Reply-To: <20181108205413.2008a004@redhat.com>
On Thu, Nov 8, 2018 at 11:54 AM Stefano Brivio <sbrivio@redhat.com> wrote:
>
> Hi,
>
> On Thu, 8 Nov 2018 11:49:49 -0800
> Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> > +EXPORT_SYMBOL(__skb_checksum_complete);
> > +
> > /* Both of above in one bottle. */
>
> Maybe you should also update/drop this comment now?
I have no idea what that comment means. Do you?
It dates back to pre-git history.
Thanks.
^ permalink raw reply
* Re: [Patch net-next] net: move __skb_checksum_complete*() to skbuff.c
From: Stefano Brivio @ 2018-11-08 19:54 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev
In-Reply-To: <20181108194949.11866-1-xiyou.wangcong@gmail.com>
Hi,
On Thu, 8 Nov 2018 11:49:49 -0800
Cong Wang <xiyou.wangcong@gmail.com> wrote:
> +EXPORT_SYMBOL(__skb_checksum_complete);
> +
> /* Both of above in one bottle. */
Maybe you should also update/drop this comment now?
> __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
--
Stefano
^ permalink raw reply
* [PATCH] staging: rtl8723bs: Add missing return for cfg80211_rtw_get_station
From: Larry Finger @ 2018-11-09 5:30 UTC (permalink / raw)
To: gregkh; +Cc: devel, netdev, youling257, linux-wireless, Larry Finger
With Androidx86 8.1, wificond returns "failed to get
nl80211_sta_info_tx_failed" and wificondControl returns "Invalid signal
poll result from wificond". The fix is to OR sinfo->filled with
BIT_ULL(NL80211_STA_INFO_TX_FAILED).
This missing bit is apparently not needed with NetworkManager, but it
does no harm in that case.
Reported-and-Tested-by: youling257 <youling257@gmail.com>
Cc: linux-wireless@vger.kernel.org
Cc: youling257 <youling257@gmail.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
index af2234798fa8..db553f2e4c0b 100644
--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
@@ -1277,7 +1277,7 @@ static int cfg80211_rtw_get_station(struct wiphy *wiphy,
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_PACKETS);
sinfo->tx_packets = psta->sta_stats.tx_pkts;
-
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_FAILED);
}
/* for Ad-Hoc/AP mode */
--
2.19.1
^ permalink raw reply related
* Re: [PATCH mlx5-next 00/10] Collection of ODP fixes
From: Saeed Mahameed @ 2018-11-08 19:50 UTC (permalink / raw)
To: Jason Gunthorpe, leon@kernel.org
Cc: netdev@vger.kernel.org, Majd Dibbiny, Moni Shoua, Leon Romanovsky,
linux-rdma@vger.kernel.org, dledford@redhat.com, Artemy Kovalyov
In-Reply-To: <20181108194542.GE5548@mellanox.com>
On Thu, 2018-11-08 at 19:45 +0000, Jason Gunthorpe wrote:
> On Thu, Nov 08, 2018 at 09:10:07PM +0200, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Hi,
> >
> > This is collection of fixes to mlx5_core and mlx5_ib ODP logic.
> > There are two main reasons why we decided to forward it to mlx5-
> > next
> > and not to rdma-rc or net like our other fixes.
> >
> > 1. We have large number of patches exactly in that area that are
> > ready
> > for submission and we don't want to create extra merge work for
> > maintainers due to that. Among those future series (already ready
> > and
> > tested): internal change of mlx5_core pagefaults handling, moving
> > ODP
> > code to RDMA, change in EQ mechanism, simplification and moving SRQ
> > QP
> > to RDMA, extra fixes to mlx5_ib ODP and ODP SRQ support.
> >
> > 2. Most of those fixes are for old bugs and there is no rush to fix
> > them
> > right now (in -rc1/-rc2).
> >
> > Thanks
> >
> > Moni Shoua (10):
> > net/mlx5: Release resource on error flow
> > IB/mlx5: Avoid hangs due to a missing completion
> > net/mlx5: Add interface to hold and release core resources
> > net/mlx5: Enumerate page fault types
> > IB/mlx5: Lock QP during page fault handling
> > net/mlx5: Return success for PAGE_FAULT_RESUME in internal error
> > state
> > net/mlx5: Use multi threaded workqueue for page fault handling
> > IB/mlx5: Call PAGE_FAULT_RESUME command asynchronously
> > net/mlx5: Remove unused function
> > IB/mlx5: Improve ODP debugging messages
> >
> > drivers/infiniband/core/umem_odp.c | 14 +-
> > drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 +
> > drivers/infiniband/hw/mlx5/mr.c | 15 +-
> > drivers/infiniband/hw/mlx5/odp.c | 158
> > ++++++++++++++----
> > drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 2 +-
> > drivers/net/ethernet/mellanox/mlx5/core/eq.c | 21 +--
> > drivers/net/ethernet/mellanox/mlx5/core/qp.c | 20 ++-
>
> there is alot of ethernet files here, parts of this should probably
> go
> through the shared branch?
>
All of this should go to the shared branch,
after this ODP series we have another series that moves all the ODP
logic out of mlx5_core into mlx5_ib.
> Jason
^ permalink raw reply
* [Patch net-next] net: move __skb_checksum_complete*() to skbuff.c
From: Cong Wang @ 2018-11-08 19:49 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang
__skb_checksum_complete_head() and __skb_checksum_complete()
are both declared in skbuff.h, they fit better in skbuff.c
than datagram.c.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/core/datagram.c | 43 -------------------------------------------
net/core/skbuff.c | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 57f3a6fcfc1e..07983b90d2bd 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -728,49 +728,6 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
return -EFAULT;
}
-__sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
-{
- __sum16 sum;
-
- sum = csum_fold(skb_checksum(skb, 0, len, skb->csum));
- if (likely(!sum)) {
- if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
- !skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
- }
- if (!skb_shared(skb))
- skb->csum_valid = !sum;
- return sum;
-}
-EXPORT_SYMBOL(__skb_checksum_complete_head);
-
-__sum16 __skb_checksum_complete(struct sk_buff *skb)
-{
- __wsum csum;
- __sum16 sum;
-
- csum = skb_checksum(skb, 0, skb->len, 0);
-
- /* skb->csum holds pseudo checksum */
- sum = csum_fold(csum_add(skb->csum, csum));
- if (likely(!sum)) {
- if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
- !skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
- }
-
- if (!skb_shared(skb)) {
- /* Save full packet checksum */
- skb->csum = csum;
- skb->ip_summed = CHECKSUM_COMPLETE;
- skb->csum_complete_sw = 1;
- skb->csum_valid = !sum;
- }
-
- return sum;
-}
-EXPORT_SYMBOL(__skb_checksum_complete);
-
/**
* skb_copy_and_csum_datagram_msg - Copy and checksum skb to user iovec.
* @skb: skbuff
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b4ee5c8b928f..7db6c520ed92 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2559,6 +2559,49 @@ __wsum skb_checksum(const struct sk_buff *skb, int offset,
}
EXPORT_SYMBOL(skb_checksum);
+__sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
+{
+ __sum16 sum;
+
+ sum = csum_fold(skb_checksum(skb, 0, len, skb->csum));
+ if (likely(!sum)) {
+ if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
+ !skb->csum_complete_sw)
+ netdev_rx_csum_fault(skb->dev);
+ }
+ if (!skb_shared(skb))
+ skb->csum_valid = !sum;
+ return sum;
+}
+EXPORT_SYMBOL(__skb_checksum_complete_head);
+
+__sum16 __skb_checksum_complete(struct sk_buff *skb)
+{
+ __wsum csum;
+ __sum16 sum;
+
+ csum = skb_checksum(skb, 0, skb->len, 0);
+
+ /* skb->csum holds pseudo checksum */
+ sum = csum_fold(csum_add(skb->csum, csum));
+ if (likely(!sum)) {
+ if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
+ !skb->csum_complete_sw)
+ netdev_rx_csum_fault(skb->dev);
+ }
+
+ if (!skb_shared(skb)) {
+ /* Save full packet checksum */
+ skb->csum = csum;
+ skb->ip_summed = CHECKSUM_COMPLETE;
+ skb->csum_complete_sw = 1;
+ skb->csum_valid = !sum;
+ }
+
+ return sum;
+}
+EXPORT_SYMBOL(__skb_checksum_complete);
+
/* Both of above in one bottle. */
__wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
--
2.19.1
^ permalink raw reply related
* Re: [PATCH mlx5-next 08/10] IB/mlx5: Call PAGE_FAULT_RESUME command asynchronously
From: Jason Gunthorpe @ 2018-11-08 19:49 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Artemy Kovalyov,
Majd Dibbiny, Moni Shoua, Saeed Mahameed, linux-netdev
In-Reply-To: <20181108191017.21891-9-leon@kernel.org>
On Thu, Nov 08, 2018 at 09:10:15PM +0200, Leon Romanovsky wrote:
> From: Moni Shoua <monis@mellanox.com>
>
> Telling the HCA that page fault handling is done and QP can resume
> its flow is done in the context of the page fault handler. This blocks
> the handling of the next work in queue without a need.
> Call the PAGE_FAULT_RESUME command in an asynchronous manner and free
> the workqueue to pick the next work item for handling. All tasks that
> were executed after PAGE_FAULT_RESUME need to be done now
> in the callback of the asynchronous command mechanism.
>
> Signed-off-by: Moni Shoua <monis@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> drivers/infiniband/hw/mlx5/odp.c | 110 +++++++++++++++++++++++++------
> include/linux/mlx5/driver.h | 3 +
> 2 files changed, 94 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
> index abce55b8b9ba..0c4f469cdd5b 100644
> +++ b/drivers/infiniband/hw/mlx5/odp.c
> @@ -298,20 +298,78 @@ void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev)
> return;
> }
>
> +struct pfault_resume_cb_ctx {
> + struct mlx5_ib_dev *dev;
> + struct mlx5_core_rsc_common *res;
> + struct mlx5_pagefault *pfault;
> +};
> +
> +static void page_fault_resume_callback(int status, void *context)
> +{
> + struct pfault_resume_cb_ctx *ctx = context;
> + struct mlx5_pagefault *pfault = ctx->pfault;
> +
> + if (status)
> + mlx5_ib_err(ctx->dev, "Resolve the page fault failed with status %d\n",
> + status);
> +
> + if (ctx->res)
> + mlx5_core_res_put(ctx->res);
> + kfree(pfault);
> + kfree(ctx);
> +}
> +
> static void mlx5_ib_page_fault_resume(struct mlx5_ib_dev *dev,
> + struct mlx5_core_rsc_common *res,
> struct mlx5_pagefault *pfault,
> - int error)
> + int error,
> + bool async)
> {
> + int ret = 0;
> + u32 *out = pfault->out_pf_resume;
> + u32 *in = pfault->in_pf_resume;
> + u32 token = pfault->token;
> int wq_num = pfault->event_subtype == MLX5_PFAULT_SUBTYPE_WQE ?
> - pfault->wqe.wq_num : pfault->token;
> - int ret = mlx5_core_page_fault_resume(dev->mdev,
> - pfault->token,
> - wq_num,
> - pfault->type,
> - error);
> - if (ret)
> - mlx5_ib_err(dev, "Failed to resolve the page fault on WQ 0x%x\n",
> - wq_num);
> + pfault->wqe.wq_num : pfault->token;
> + u8 type = pfault->type;
> + struct pfault_resume_cb_ctx *ctx = NULL;
> +
> + if (async)
> + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
Why not allocate this ctx ast part of the mlx5_pagefault and avoid
this allocation failure strategy?
Jason
^ permalink raw reply
* [PATCH net-next] sfc: use the new __netdev_tx_sent_queue BQL optimisation
From: Edward Cree @ 2018-11-08 19:47 UTC (permalink / raw)
To: linux-net-drivers, davem; +Cc: netdev
As added in 3e59020abf0f ("net: bql: add __netdev_tx_sent_queue()"), which
see for performance rationale.
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
drivers/net/ethernet/sfc/tx.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index c3ad564ac4c0..22eb059086f7 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -553,13 +553,10 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
if (!data_mapped && (efx_tx_map_data(tx_queue, skb, segments)))
goto err;
- /* Update BQL */
- netdev_tx_sent_queue(tx_queue->core_txq, skb_len);
-
efx_tx_maybe_stop_queue(tx_queue);
/* Pass off to hardware */
- if (!xmit_more || netif_xmit_stopped(tx_queue->core_txq)) {
+ if (__netdev_tx_sent_queue(tx_queue->core_txq, skb_len, xmit_more)) {
struct efx_tx_queue *txq2 = efx_tx_queue_partner(tx_queue);
/* There could be packets left on the partner queue if those
^ permalink raw reply related
* Re: [PATCH v3 bpf-next 4/4] bpftool: support loading flow dissector
From: Jakub Kicinski @ 2018-11-08 19:46 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, linux-kselftest, ast, daniel, shuah, quentin.monnet, guro,
jiong.wang, bhole_prashant_q7, john.fastabend, jbenc,
treeze.taeung, yhs, osk, sandipan
In-Reply-To: <20181108053957.205681-5-sdf@google.com>
On Wed, 7 Nov 2018 21:39:57 -0800, Stanislav Fomichev wrote:
> This commit adds support for loading/attaching/detaching flow
> dissector program. The structure of the flow dissector program is
> assumed to be the same as in the selftests:
>
> * flow_dissector section with the main entry point
> * a bunch of tail call progs
> * a jmp_table map that is populated with the tail call progs
Could you split the loadall changes and the flow_dissector changes into
two separate patches?
^ permalink raw reply
* Re: [PATCH mlx5-next 00/10] Collection of ODP fixes
From: Jason Gunthorpe @ 2018-11-08 19:45 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Artemy Kovalyov,
Majd Dibbiny, Moni Shoua, Saeed Mahameed, linux-netdev
In-Reply-To: <20181108191017.21891-1-leon@kernel.org>
On Thu, Nov 08, 2018 at 09:10:07PM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Hi,
>
> This is collection of fixes to mlx5_core and mlx5_ib ODP logic.
> There are two main reasons why we decided to forward it to mlx5-next
> and not to rdma-rc or net like our other fixes.
>
> 1. We have large number of patches exactly in that area that are ready
> for submission and we don't want to create extra merge work for
> maintainers due to that. Among those future series (already ready and
> tested): internal change of mlx5_core pagefaults handling, moving ODP
> code to RDMA, change in EQ mechanism, simplification and moving SRQ QP
> to RDMA, extra fixes to mlx5_ib ODP and ODP SRQ support.
>
> 2. Most of those fixes are for old bugs and there is no rush to fix them
> right now (in -rc1/-rc2).
>
> Thanks
>
> Moni Shoua (10):
> net/mlx5: Release resource on error flow
> IB/mlx5: Avoid hangs due to a missing completion
> net/mlx5: Add interface to hold and release core resources
> net/mlx5: Enumerate page fault types
> IB/mlx5: Lock QP during page fault handling
> net/mlx5: Return success for PAGE_FAULT_RESUME in internal error state
> net/mlx5: Use multi threaded workqueue for page fault handling
> IB/mlx5: Call PAGE_FAULT_RESUME command asynchronously
> net/mlx5: Remove unused function
> IB/mlx5: Improve ODP debugging messages
>
> drivers/infiniband/core/umem_odp.c | 14 +-
> drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 +
> drivers/infiniband/hw/mlx5/mr.c | 15 +-
> drivers/infiniband/hw/mlx5/odp.c | 158 ++++++++++++++----
> drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 21 +--
> drivers/net/ethernet/mellanox/mlx5/core/qp.c | 20 ++-
there is alot of ethernet files here, parts of this should probably go
through the shared branch?
Jason
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox