* Re: [PATCH net-next] net: wan: cosa: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: David Miller @ 2019-02-25 22:39 UTC (permalink / raw)
To: albin_yang; +Cc: netdev, kas, yang.wei9
In-Reply-To: <1551107141-16702-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Mon, 25 Feb 2019 23:05:41 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in cosa_net_tx_done() when skb
> xmit done. It makes drop profiles(dropwatch, perf) more friendly.
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: wan: z85230: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: David Miller @ 2019-02-25 22:39 UTC (permalink / raw)
To: albin_yang; +Cc: netdev, yang.wei9
In-Reply-To: <1551107184-16755-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Mon, 25 Feb 2019 23:06:24 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in z8530_tx_done() when skb
> xmit done. It makes drop profiles(dropwatch, perf) more friendly.
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: sched: set dedicated tcf_walker flag when tp is empty
From: Cong Wang @ 2019-02-25 22:52 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190225153831.10037-1-vladbu@mellanox.com>
On Mon, Feb 25, 2019 at 7:38 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Using tcf_walker->stop flag to determine when tcf_walker->fn() was called
> at least once is unreliable. Some classifiers set 'stop' flag on error
> before calling walker callback, other classifiers used to call it with NULL
> filter pointer when empty. In order to prevent further regressions, extend
> tcf_walker structure with dedicated 'nonempty' flag. Set this flag in
> tcf_walker->fn() implementation that is used to check if classifier has
> filters configured.
So, after this patch commits like 31a998487641 ("net: sched: fw: don't
set arg->stop in fw_walk() when empty") can be reverted??
>
> Fixes: 8b64678e0af8 ("net: sched: refactor tp insert/delete for concurrent execution")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> include/net/pkt_cls.h | 1 +
> net/sched/cls_api.c | 13 +++++++++----
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index 232f801f2a21..422dd8800478 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -17,6 +17,7 @@ struct tcf_walker {
> int stop;
> int skip;
> int count;
> + bool nonempty;
> unsigned long cookie;
> int (*fn)(struct tcf_proto *, void *node, struct tcf_walker *);
> };
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index e2c888961379..3543be31d400 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -238,18 +238,23 @@ static void tcf_proto_put(struct tcf_proto *tp, bool rtnl_held,
> tcf_proto_destroy(tp, rtnl_held, extack);
> }
>
> -static int walker_noop(struct tcf_proto *tp, void *d, struct tcf_walker *arg)
> +static int walker_check_empty(struct tcf_proto *tp, void *d,
> + struct tcf_walker *arg)
> {
> - return -1;
> + if (tp) {
> + arg->nonempty = true;
> + return -1;
> + }
> + return 0;
How does this even work? If we can simply check tp!=NULL as
non-empty, why do we even need a walker??
For me, it must be pushed down to each implementation to
determine how it is empty.
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Cong Wang @ 2019-02-25 22:58 UTC (permalink / raw)
To: David Miller
Cc: Yafang Shao, Daniel Borkmann, Eric Dumazet, Joe Perches,
Linux Kernel Network Developers, shaoyafang
In-Reply-To: <20190225.142939.803768996918170563.davem@davemloft.net>
On Mon, Feb 25, 2019 at 2:29 PM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Mon, 25 Feb 2019 14:00:09 -0800
>
> > Just to clarify, I have been suggesting to completely remove
> > this unused macro, never suggest to just undefine it in-tree.
> >
> > There is no reason to keep it in-tree, whether defined or undefined,
> > just for downstream users.
>
> And this is where you and I fundamentally disagree.
So you agree that I can add debugging printk's only for my own use?
I can claim that I only use them downstream and you can't force me
to carry local changes?
If not, what is your criteria for accepting debugging printk's? Whose
can be accepted and whose can't?
Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
Thanks!
^ permalink raw reply
* Re: [PATCH bpf-next v6 0/3] libbpf: adding AF_XDP support
From: Daniel Borkmann @ 2019-02-25 22:59 UTC (permalink / raw)
To: Magnus Karlsson, bjorn.topel, ast, netdev, jakub.kicinski,
bjorn.topel, qi.z.zhang
Cc: brouer, xiaolong.ye
In-Reply-To: <1550740888-26439-1-git-send-email-magnus.karlsson@intel.com>
On 02/21/2019 10:21 AM, Magnus Karlsson wrote:
> This patch proposes to add AF_XDP support to libbpf. The main reason
> for this is to facilitate writing applications that use AF_XDP by
> offering higher-level APIs that hide many of the details of the AF_XDP
> uapi. This is in the same vein as libbpf facilitates XDP adoption by
> offering easy-to-use higher level interfaces of XDP
> functionality. Hopefully this will facilitate adoption of AF_XDP, make
> applications using it simpler and smaller, and finally also make it
> possible for applications to benefit from optimizations in the AF_XDP
> user space access code. Previously, people just copied and pasted the
> code from the sample application into their application, which is not
> desirable.
>
> The proposed interface is composed of two parts:
>
> * Low-level access interface to the four rings and the packet
> * High-level control plane interface for creating and setting up umems
> and AF_XDP sockets. This interface also loads a simple XDP program
> that routes all traffic on a queue up to the AF_XDP socket.
>
> The sample program has been updated to use this new interface and in
> that process it lost roughly 300 lines of code. I cannot detect any
> performance degradations due to the use of this library instead of the
> previous functions that were inlined in the sample application. But I
> did measure this on a slower machine and not the Broadwell that we
> normally use.
>
> The rings are now called xsk_ring and when a producer operates on
> it. It is xsk_ring_prod and for a consumer it is xsk_ring_cons. This
> way we can get some compile time error checking that the rings are
> used correctly.
>
> Comments and contenplations:
>
> * The current behaviour is that the library loads an XDP program (if
> requested to do so) but the clean up of this program is left to the
> application. It would be possible to implement this cleanup in the
> library, but it would require state to be kept on netdev level,
> which there is none at the moment, and the synchronization of this
> between processes. All this adding complexity. But when we get an
> XDP program per queue id, then it becomes trivial to also remove the
> XDP program when the application exits. This proposal from Jesper,
> Björn and others will also improve the performance of libbpf, since
> most of the XDP program code can be removed when that feature is
> supported.
>
> * In a future release, I am planning on adding a higher level data
> plane interface too. This will be based around recvmsg and sendmsg
> with the use of struct iovec for batching, without the user having
> to know anything about the underlying four rings of an AF_XDP
> socket. There will be one semantic difference though from the
> standard recvmsg and that is that the kernel will fill in the iovecs
> instead of the application. But the rest should be the same as the
> libc versions so that application writers feel at home.
>
> Patch 1: adds AF_XDP support in libbpf
> Patch 2: updates the xdpsock sample application to use the libbpf functions
> Patch 3: Documentation update to help first time users
>
> Changes v5 to v6:
> * Fixed prog_fd bug found by Xiaolong Ye. Thanks!
> Changes v4 to v5:
> * Added a FAQ to the documentation
> * Removed xsk_umem__get_data and renamed xsk_umem__get_dat_raw to
> xsk_umem__get_data
> * Replaced the netlink code with bpf_get_link_xdp_id()
> * Dynamic allocation of the map sizes. They are now sized after
> the max number of queueus on the netdev in question.
Looks better, I've applied it to bpf-next, thanks!
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Stanislav Fomichev @ 2019-02-25 23:07 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: davem, daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-2-ast@kernel.org>
On 02/25, Alexei Starovoitov wrote:
> JITed BPF programs are indistinguishable from kernel functions, but unlike
> kernel code BPF code can be changed often.
> Typical approach of "perf record" + "perf report" profiling and tuning of
> kernel code works just as well for BPF programs, but kernel code doesn't
> need to be monitored whereas BPF programs do.
> Users load and run large amount of BPF programs.
> These BPF stats allow tools monitor the usage of BPF on the server.
> The monitoring tools will turn sysctl kernel.bpf_stats_enabled
> on and off for few seconds to sample average cost of the programs.
> Aggregated data over hours and days will provide an insight into cost of BPF
> and alarms can trigger in case given program suddenly gets more expensive.
>
> The cost of two sched_clock() per program invocation adds ~20 nsec.
> Fast BPF progs (like selftests/bpf/progs/test_pkt_access.c) will slow down
> from ~10 nsec to ~30 nsec.
> static_key minimizes the cost of the stats collection.
> There is no measurable difference before/after this patch
> with kernel.bpf_stats_enabled=0
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> include/linux/bpf.h | 9 +++++++++
> include/linux/filter.h | 20 +++++++++++++++++++-
> kernel/bpf/core.c | 31 +++++++++++++++++++++++++++++--
> kernel/bpf/syscall.c | 34 ++++++++++++++++++++++++++++++++--
> kernel/bpf/verifier.c | 7 ++++++-
> kernel/sysctl.c | 34 ++++++++++++++++++++++++++++++++++
> 6 files changed, 129 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index de18227b3d95..a2132e09dc1c 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -16,6 +16,7 @@
> #include <linux/rbtree_latch.h>
> #include <linux/numa.h>
> #include <linux/wait.h>
> +#include <linux/u64_stats_sync.h>
>
> struct bpf_verifier_env;
> struct perf_event;
> @@ -340,6 +341,12 @@ enum bpf_cgroup_storage_type {
>
> #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
>
> +struct bpf_prog_stats {
> + u64 cnt;
> + u64 nsecs;
> + struct u64_stats_sync syncp;
> +};
> +
> struct bpf_prog_aux {
> atomic_t refcnt;
> u32 used_map_cnt;
> @@ -389,6 +396,7 @@ struct bpf_prog_aux {
> * main prog always has linfo_idx == 0
> */
> u32 linfo_idx;
> + struct bpf_prog_stats __percpu *stats;
> union {
> struct work_struct work;
> struct rcu_head rcu;
> @@ -559,6 +567,7 @@ void bpf_map_area_free(void *base);
> void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
>
> extern int sysctl_unprivileged_bpf_disabled;
> +extern int sysctl_bpf_stats_enabled;
>
> int bpf_map_new_fd(struct bpf_map *map, int flags);
> int bpf_prog_new_fd(struct bpf_prog *prog);
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index f32b3eca5a04..7e5e3db11106 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -533,7 +533,24 @@ struct sk_filter {
> struct bpf_prog *prog;
> };
>
> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> +DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> +
> +#define BPF_PROG_RUN(prog, ctx) ({ \
> + u32 ret; \
> + cant_sleep(); \
> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
> + struct bpf_prog_stats *stats; \
> + u64 start = sched_clock(); \
QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
in the bpf_test_run()? Or even why not local_clock?
I'm just wondering what king of trade off we are doing here
regarding precision vs run time cost.
> + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> + stats = this_cpu_ptr(prog->aux->stats); \
> + u64_stats_update_begin(&stats->syncp); \
> + stats->cnt++; \
> + stats->nsecs += sched_clock() - start; \
> + u64_stats_update_end(&stats->syncp); \
> + } else { \
> + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> + } \
> + ret; })
>
> #define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
>
> @@ -764,6 +781,7 @@ void bpf_prog_free_jited_linfo(struct bpf_prog *prog);
> void bpf_prog_free_unused_jited_linfo(struct bpf_prog *prog);
>
> struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags);
> +struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flags);
> struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
> gfp_t gfp_extra_flags);
> void __bpf_prog_free(struct bpf_prog *fp);
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ef88b167959d..1c14c347f3cf 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -78,7 +78,7 @@ void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, uns
> return NULL;
> }
>
> -struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
> +struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flags)
> {
> gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO | gfp_extra_flags;
> struct bpf_prog_aux *aux;
> @@ -104,6 +104,26 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
>
> return fp;
> }
> +
> +struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
> +{
> + gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO | gfp_extra_flags;
> + struct bpf_prog *prog;
> +
> + prog = bpf_prog_alloc_no_stats(size, gfp_extra_flags);
> + if (!prog)
> + return NULL;
> +
> + prog->aux->stats = alloc_percpu_gfp(struct bpf_prog_stats, gfp_flags);
> + if (!prog->aux->stats) {
> + kfree(prog->aux);
> + vfree(prog);
> + return NULL;
> + }
> +
> + u64_stats_init(&prog->aux->stats->syncp);
> + return prog;
> +}
> EXPORT_SYMBOL_GPL(bpf_prog_alloc);
>
> int bpf_prog_alloc_jited_linfo(struct bpf_prog *prog)
> @@ -231,7 +251,10 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
>
> void __bpf_prog_free(struct bpf_prog *fp)
> {
> - kfree(fp->aux);
> + if (fp->aux) {
> + free_percpu(fp->aux->stats);
> + kfree(fp->aux);
> + }
> vfree(fp);
> }
>
> @@ -2069,6 +2092,10 @@ int __weak skb_copy_bits(const struct sk_buff *skb, int offset, void *to,
> return -EFAULT;
> }
>
> +DEFINE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> +EXPORT_SYMBOL(bpf_stats_enabled_key);
> +int sysctl_bpf_stats_enabled __read_mostly;
> +
> /* All definitions of tracepoints related to BPF. */
> #define CREATE_TRACE_POINTS
> #include <linux/bpf_trace.h>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ec7c552af76b..31cf66fc3f5c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1283,24 +1283,54 @@ static int bpf_prog_release(struct inode *inode, struct file *filp)
> return 0;
> }
>
> +static void bpf_prog_get_stats(const struct bpf_prog *prog,
> + struct bpf_prog_stats *stats)
> +{
> + u64 nsecs = 0, cnt = 0;
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + const struct bpf_prog_stats *st;
> + unsigned int start;
> + u64 tnsecs, tcnt;
> +
> + st = per_cpu_ptr(prog->aux->stats, cpu);
> + do {
> + start = u64_stats_fetch_begin_irq(&st->syncp);
> + tnsecs = st->nsecs;
> + tcnt = st->cnt;
> + } while (u64_stats_fetch_retry_irq(&st->syncp, start));
> + nsecs += tnsecs;
> + cnt += tcnt;
> + }
> + stats->nsecs = nsecs;
> + stats->cnt = cnt;
> +}
> +
> #ifdef CONFIG_PROC_FS
> static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
> {
> const struct bpf_prog *prog = filp->private_data;
> char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
> + struct bpf_prog_stats stats;
>
> + bpf_prog_get_stats(prog, &stats);
> bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
> seq_printf(m,
> "prog_type:\t%u\n"
> "prog_jited:\t%u\n"
> "prog_tag:\t%s\n"
> "memlock:\t%llu\n"
> - "prog_id:\t%u\n",
> + "prog_id:\t%u\n"
> + "run_time_ns:\t%llu\n"
> + "run_cnt:\t%llu\n",
> prog->type,
> prog->jited,
> prog_tag,
> prog->pages * 1ULL << PAGE_SHIFT,
> - prog->aux->id);
> + prog->aux->id,
> + stats.nsecs,
> + stats.cnt);
> }
> #endif
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b9496c41383..0e4edd7e3c5f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7320,7 +7320,12 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> subprog_end = env->subprog_info[i + 1].start;
>
> len = subprog_end - subprog_start;
> - func[i] = bpf_prog_alloc(bpf_prog_size(len), GFP_USER);
> + /* BPF_PROG_RUN doesn't call subprogs directly,
> + * hence main prog stats include the runtime of subprogs.
> + * subprogs don't have IDs and not reachable via prog_get_next_id
> + * func[i]->aux->stats will never be accessed and stays NULL
> + */
> + func[i] = bpf_prog_alloc_no_stats(bpf_prog_size(len), GFP_USER);
> if (!func[i])
> goto out_free;
> memcpy(func[i]->insnsi, &prog->insnsi[subprog_start],
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index ba4d9e85feb8..86e0771352f2 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -224,6 +224,9 @@ static int proc_dostring_coredump(struct ctl_table *table, int write,
> #endif
> static int proc_dopipe_max_size(struct ctl_table *table, int write,
> void __user *buffer, size_t *lenp, loff_t *ppos);
> +static int proc_dointvec_minmax_bpf_stats(struct ctl_table *table, int write,
> + void __user *buffer, size_t *lenp,
> + loff_t *ppos);
>
> #ifdef CONFIG_MAGIC_SYSRQ
> /* Note: sysrq code uses its own private copy */
> @@ -1230,6 +1233,15 @@ static struct ctl_table kern_table[] = {
> .extra2 = &one,
> },
> #endif
> + {
> + .procname = "bpf_stats_enabled",
> + .data = &sysctl_bpf_stats_enabled,
> + .maxlen = sizeof(sysctl_bpf_stats_enabled),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax_bpf_stats,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> #if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU)
> {
> .procname = "panic_on_rcu_stall",
> @@ -3260,6 +3272,28 @@ int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int write,
>
> #endif /* CONFIG_PROC_SYSCTL */
>
> +static int proc_dointvec_minmax_bpf_stats(struct ctl_table *table, int write,
> + void __user *buffer, size_t *lenp,
> + loff_t *ppos)
> +{
> + int ret, bpf_stats = *(int *)table->data;
> + struct ctl_table tmp = *table;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + tmp.data = &bpf_stats;
> + ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> + if (write && !ret) {
> + *(int *)table->data = bpf_stats;
> + if (bpf_stats)
> + static_branch_enable(&bpf_stats_enabled_key);
> + else
> + static_branch_disable(&bpf_stats_enabled_key);
> + }
> + return ret;
> +}
> +
> /*
> * No sense putting this after each symbol definition, twice,
> * exception granted :-)
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Eric Dumazet @ 2019-02-25 23:09 UTC (permalink / raw)
To: Dmitry Safonov, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <20190225212721.2908-1-dima@arista.com>
On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
> rtnl_unregister() unsets handler from table, which is protected
> by rtnl_lock or RCU. At this moment only dump handlers access the table
> with rcu_lock(). Every other user accesses under rtnl.
>
> Callers may expect that rtnl_unregister() prevents any further handlers
> calls, alike rtnl_unregister_all(). And they do expect it.
>
> I've looked on in-tree caller uses:
> br_mdb: safe, but in err-path br_netlink_init()
> fib_rules: safe - err-path is very early in __init
> ip6mr: safe - following unregister_pernet_subsys() calls internally rcu_barrier()
> qrtr: safe - following sock_unregister() calls internally synchronize_rcu()
If rcu_barrier() was needed, then all callers should use it.
If synchronize_rcu() was needed, then all callers should use it.
But mixing is probably wrong.
>
> While it's possible to document that rtnl_unregister() requires
> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
> the module exit is very much slow-path.
rtnl_unregister_all() needs the sychronize_rcu() at this moment
because of the kfree(tab), not because of the kfree_rcu(link, rcu);
>
> Issue seems to be very theoretical and unlikely, so I'm not Cc'ing
> stable tree.
>
> Fixes: 6853dd488119 ("rtnetlink: protect handler table with rcu")
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: "Hannes Frederic Sowa" <hannes@stressinduktion.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
> net/core/rtnetlink.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 5ea1bed08ede..3db70da4f951 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -308,7 +308,9 @@ int rtnl_unregister(int protocol, int msgtype)
> rcu_assign_pointer(tab[msgindex], NULL);
> rtnl_unlock();
>
> - kfree_rcu(link, rcu);
> + synchronize_net();
> +
> + kfree(link);
I really do not see a difference here (other than this being much slower of course)
If the caller needs rcu_barrier(), then add it in the caller ?
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: David Miller @ 2019-02-25 23:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: laoar.shao, daniel, edumazet, joe, netdev, shaoyafang
In-Reply-To: <CAM_iQpUknnab0bwJgp3-F864-02LsH+J07c8SzMZyYK84yoezw@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 25 Feb 2019 14:58:36 -0800
> So you agree that I can add debugging printk's only for my own use?
Let's put it this way.
If I ignore the fact that some of the data centers with the furtest
reach on the entire planet use something that is in the tree already,
then I am being unreasonable and insensitive.
This means no adding, but keeping things we already have.
Don't try to weasel word what I'm trying to say here Cong.
Have a nice day.
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: David Miller @ 2019-02-25 23:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: laoar.shao, daniel, edumazet, joe, netdev, shaoyafang
In-Reply-To: <CAM_iQpUknnab0bwJgp3-F864-02LsH+J07c8SzMZyYK84yoezw@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 25 Feb 2019 14:58:36 -0800
> Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
And this isn't happening either Cong, I reserve the right to apply my
judgment as networking maintainer on a case by case basis as I so see
fit.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Stanislav Fomichev @ 2019-02-25 23:14 UTC (permalink / raw)
To: brakmo
Cc: netdev, Martin Lau, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Kernel Team
In-Reply-To: <20190223010703.678070-3-brakmo@fb.com>
On 02/22, brakmo wrote:
> From: Martin KaFai Lau <kafai@fb.com>
>
> This patch adds a new bpf helper BPF_FUNC_tcp_enter_cwr
> "int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)".
> It is added to BPF_PROG_TYPE_CGROUP_SKB which can be attached
> to the egress path where the bpf prog is called by
> ip_finish_output() or ip6_finish_output(). The verifier
> ensures that the parameter must be a tcp_sock.
>
> This helper makes a tcp_sock enter CWR state. It can be used
> by a bpf_prog to manage egress network bandwidth limit per
> cgroupv2. A later patch will have a sample program to
> show how it can be used to limit bandwidth usage per cgroupv2.
>
> To ensure it is only called from BPF_CGROUP_INET_EGRESS, the
> attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS
> during load time if the prog uses this new helper.
> The newly added prog->enforce_expected_attach_type bit will also be set
> if this new helper is used. This bit is for backward compatibility reason
> because currently prog->expected_attach_type has been ignored in
> BPF_PROG_TYPE_CGROUP_SKB. During attach time,
> prog->expected_attach_type is only enforced if the
> prog->enforce_expected_attach_type bit is set.
> i.e. prog->expected_attach_type is only enforced if this new helper
> is used by the prog.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> include/linux/bpf.h | 1 +
> include/linux/filter.h | 3 ++-
> include/uapi/linux/bpf.h | 9 ++++++++-
> kernel/bpf/syscall.c | 12 ++++++++++++
> kernel/bpf/verifier.c | 4 ++++
> net/core/filter.c | 25 +++++++++++++++++++++++++
> 6 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d5ba2fc01af3..2d54ba7cf9dd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -195,6 +195,7 @@ enum bpf_arg_type {
> ARG_PTR_TO_SOCKET, /* pointer to bpf_sock */
> ARG_PTR_TO_SPIN_LOCK, /* pointer to bpf_spin_lock */
> ARG_PTR_TO_SOCK_COMMON, /* pointer to sock_common */
> + ARG_PTR_TO_TCP_SOCK, /* pointer to tcp_sock */
> };
>
> /* type of values returned from helper functions */
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index f32b3eca5a04..c6e878bdc5a6 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -510,7 +510,8 @@ struct bpf_prog {
> blinded:1, /* Was blinded */
> is_func:1, /* program is a bpf function */
> kprobe_override:1, /* Do we override a kprobe? */
> - has_callchain_buf:1; /* callchain buffer allocated? */
> + has_callchain_buf:1, /* callchain buffer allocated? */
> + enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */
> enum bpf_prog_type type; /* Type of BPF program */
> enum bpf_attach_type expected_attach_type; /* For some prog types */
> u32 len; /* Number of filter blocks */
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..95b5058fa945 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2359,6 +2359,12 @@ union bpf_attr {
> * Return
> * A **struct bpf_tcp_sock** pointer on success, or NULL in
> * case of failure.
> + *
> + * int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)
> + * Description
> + * Make a tcp_sock enter CWR state.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2457,7 +2463,8 @@ union bpf_attr {
> FN(spin_lock), \
> FN(spin_unlock), \
> FN(sk_fullsock), \
> - FN(tcp_sock),
> + FN(tcp_sock), \
> + FN(tcp_enter_cwr),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ec7c552af76b..9a478f2875cd 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1482,6 +1482,14 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
> default:
> return -EINVAL;
> }
> + case BPF_PROG_TYPE_CGROUP_SKB:
> + switch (expected_attach_type) {
> + case BPF_CGROUP_INET_INGRESS:
> + case BPF_CGROUP_INET_EGRESS:
> + return 0;
> + default:
> + return -EINVAL;
> + }
> default:
> return 0;
> }
> @@ -1725,6 +1733,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> case BPF_PROG_TYPE_CGROUP_SOCK:
> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> + case BPF_PROG_TYPE_CGROUP_SKB:
> + return prog->enforce_expected_attach_type &&
> + prog->expected_attach_type != attach_type ?
> + -EINVAL : 0;
> default:
> return 0;
> }
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b9496c41383..95fb385c6f3c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2424,6 +2424,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> return -EFAULT;
> }
> meta->ptr_id = reg->id;
> + } else if (arg_type == ARG_PTR_TO_TCP_SOCK) {
> + expected_type = PTR_TO_TCP_SOCK;
> + if (type != expected_type)
> + goto err_type;
> } else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
> if (meta->func_id == BPF_FUNC_spin_lock) {
> if (process_spin_lock(env, regno, true))
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 97916eedfe69..ca57ef25279c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5426,6 +5426,24 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
> .arg1_type = ARG_PTR_TO_SOCK_COMMON,
> };
>
> +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
> +{
> + struct sock *sk = (struct sock *)tp;
> +
> + if (sk->sk_state == TCP_ESTABLISHED) {
> + tcp_enter_cwr(sk);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> + .func = bpf_tcp_enter_cwr,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_TCP_SOCK,
> +};
> #endif /* CONFIG_INET */
>
> bool bpf_helper_changes_pkt_data(void *func)
> @@ -5585,6 +5603,13 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> #ifdef CONFIG_INET
> case BPF_FUNC_tcp_sock:
> return &bpf_tcp_sock_proto;
[...]
> + case BPF_FUNC_tcp_enter_cwr:
> + if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
> + prog->enforce_expected_attach_type = 1;
> + return &bpf_tcp_enter_cwr_proto;
Instead of this back and forth with enforce_expected_attach_type, can we
just do here:
if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
return &bpf_tcp_enter_cwr_proto;
else
return null;
Wouldn't it have the same effect?
> + } else {
> + return NULL;
> + }
> #endif
> default:
> return sk_filter_func_proto(func_id, prog);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Cong Wang @ 2019-02-25 23:18 UTC (permalink / raw)
To: David Miller, LKML
Cc: Yafang Shao, Daniel Borkmann, Eric Dumazet, Joe Perches,
Linux Kernel Network Developers, shaoyafang
In-Reply-To: <20190225.151141.792446098735157842.davem@davemloft.net>
(Cc'ing LKML)
On Mon, Feb 25, 2019 at 3:11 PM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Mon, 25 Feb 2019 14:58:36 -0800
>
> > Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
>
> And this isn't happening either Cong, I reserve the right to apply my
> judgment as networking maintainer on a case by case basis as I so see
> fit.
Thanks for rephrasing non-transparency in such a wonderful way.
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Dmitry Safonov @ 2019-02-25 23:21 UTC (permalink / raw)
To: Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <eb7bca34-2b6a-1140-b230-71f333b42807@gmail.com>
Hi Eric,
On 2/25/19 11:09 PM, Eric Dumazet wrote:
> On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
>> While it's possible to document that rtnl_unregister() requires
>> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
>> the module exit is very much slow-path.
>
> rtnl_unregister_all() needs the sychronize_rcu() at this moment
> because of the kfree(tab), not because of the kfree_rcu(link, rcu);
I may be wrong here, but shouldn't we wait for grace period to elapse by
the reason that rtnl_msg_handlers are protected by RCU, not only by rtnl?
Like, without synchronize_net() in rtnl_unregister() - what prevents
module exit race to say, rtnetlink_rcv_msg()=>rtnl_get_link()?
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -308,7 +308,9 @@ int rtnl_unregister(int protocol, int msgtype)
>> rcu_assign_pointer(tab[msgindex], NULL);
>> rtnl_unlock();
>>
>> - kfree_rcu(link, rcu);
>> + synchronize_net();
>> +
>> + kfree(link);
>
>
> I really do not see a difference here (other than this being much slower of course)
>
> If the caller needs rcu_barrier(), then add it in the caller ?
Well, sure - but it seems confusing that rtnl_unregister() will require
synchronize_rcu(), while rtnl_unregister_all() will not.
And I thought no one would care about another synchronize_rcu() in exit
path.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Dmitry Safonov @ 2019-02-25 23:27 UTC (permalink / raw)
To: Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <d20c023b-33d5-4c48-a404-e2ba05f98a99@arista.com>
On 2/25/19 11:21 PM, Dmitry Safonov wrote:
> Hi Eric,
>
> On 2/25/19 11:09 PM, Eric Dumazet wrote:
>> On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
>>> While it's possible to document that rtnl_unregister() requires
>>> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
>>> the module exit is very much slow-path.
>>
>> rtnl_unregister_all() needs the sychronize_rcu() at this moment
>> because of the kfree(tab), not because of the kfree_rcu(link, rcu);
>
> I may be wrong here, but shouldn't we wait for grace period to elapse by
> the reason that rtnl_msg_handlers are protected by RCU, not only by rtnl?
> Like, without synchronize_net() in rtnl_unregister() - what prevents
> module exit race to say, rtnetlink_rcv_msg()=>rtnl_get_link()?
There is synchronize_rcu() in free_module() - but I believe it's a bit
too far in unloading. Maybe, I'm missing another call on the way.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Paul Moore @ 2019-02-25 23:30 UTC (permalink / raw)
To: David Miller; +Cc: s-nazarov, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <20190225.143355.245256286926542053.davem@davemloft.net>
On Mon, Feb 25, 2019 at 5:33 PM David Miller <davem@davemloft.net> wrote:
> From: Nazarov Sergey <s-nazarov@yandex.ru>
> Date: Mon, 25 Feb 2019 19:24:15 +0300
>
> > Add __icmp_send function having ip_options struct parameter
> >
> > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
>
> Applied with Subject line fixes up. This commit doesn't even make
> changes to cipse_v4_error().
>
> Anyone who ACK'd this change or added their Reviewed-by did not read
> this email and are just rubber stamping crap.
I should have checked the subject line closer that's my fault (Gmail
does interesting things to threads sometimes, and obscured the subject
line), however, I did look at the content of patch before giving it my
thumbs up. Claiming the email wasn't read isn't correct (although you
could rightly argue I didn't read the subject line), or I'm "rubber
stamping crap" isn't correct.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Eric Dumazet @ 2019-02-25 23:31 UTC (permalink / raw)
To: Dmitry Safonov, Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <d20c023b-33d5-4c48-a404-e2ba05f98a99@arista.com>
On 02/25/2019 03:21 PM, Dmitry Safonov wrote:
> Hi Eric,
>
> On 2/25/19 11:09 PM, Eric Dumazet wrote:
>> On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
>>> While it's possible to document that rtnl_unregister() requires
>>> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
>>> the module exit is very much slow-path.
>>
>> rtnl_unregister_all() needs the sychronize_rcu() at this moment
>> because of the kfree(tab), not because of the kfree_rcu(link, rcu);
>
> I may be wrong here, but shouldn't we wait for grace period to elapse by
> the reason that rtnl_msg_handlers are protected by RCU, not only by rtnl?
> Like, without synchronize_net() in rtnl_unregister() - what prevents
> module exit race to say, rtnetlink_rcv_msg()=>rtnl_get_link()?
>
>
>>> --- a/net/core/rtnetlink.c
>>> +++ b/net/core/rtnetlink.c
>>> @@ -308,7 +308,9 @@ int rtnl_unregister(int protocol, int msgtype)
>>> rcu_assign_pointer(tab[msgindex], NULL);
>>> rtnl_unlock();
>>>
>>> - kfree_rcu(link, rcu);
>>> + synchronize_net();
>>> +
>>> + kfree(link);
>>
>>
>> I really do not see a difference here (other than this being much slower of course)
>>
>> If the caller needs rcu_barrier(), then add it in the caller ?
>
> Well, sure - but it seems confusing that rtnl_unregister() will require
> synchronize_rcu(), while rtnl_unregister_all() will not.
rtnl_unregister_all() is a different beast, since it removes the whole rtnl_msg_handlers[protocol]
rtnl_unregister() only removes a subset, with different usages.
> And I thought no one would care about another synchronize_rcu() in exit
> path.
We definitely care about things be done properly.
If synchronize_rcu() is needed there, be it, but kfree_rcu() seems to be fine.
In any case, I believe you need to more carefully explain what is the problem here,
because I could not really see it.
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-25 23:52 UTC (permalink / raw)
To: Stanislav Fomichev, Alexei Starovoitov
Cc: davem@davemloft.net, daniel@iogearbox.net, edumazet@google.com,
netdev@vger.kernel.org, bpf@vger.kernel.org, Kernel Team
In-Reply-To: <20190225230703.GB32115@mini-arch>
On 2/25/19 3:07 PM, Stanislav Fomichev wrote:
>> +#define BPF_PROG_RUN(prog, ctx) ({ \
>> + u32 ret; \
>> + cant_sleep(); \
>> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
>> + struct bpf_prog_stats *stats; \
>> + u64 start = sched_clock(); \
> QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
> in the bpf_test_run()? Or even why not local_clock?
> I'm just wondering what king of trade off we are doing here
> regarding precision vs run time cost.
I'm making this decision based on documentation:
Documentation/timers/timekeeping.txt
"Compared to clock sources, sched_clock() has to be very fast: it is
called much more often, especially by the scheduler. If you have to do
trade-offs between accuracy compared to the clock source, you may
sacrifice accuracy for speed in sched_clock()."
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Yafang Shao @ 2019-02-25 23:54 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Daniel Borkmann, Eric Dumazet, Joe Perches,
Linux Kernel Network Developers, shaoyafang
In-Reply-To: <CAM_iQpUknnab0bwJgp3-F864-02LsH+J07c8SzMZyYK84yoezw@mail.gmail.com>
On Tue, Feb 26, 2019 at 6:58 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Feb 25, 2019 at 2:29 PM David Miller <davem@davemloft.net> wrote:
> >
> > From: Cong Wang <xiyou.wangcong@gmail.com>
> > Date: Mon, 25 Feb 2019 14:00:09 -0800
> >
> > > Just to clarify, I have been suggesting to completely remove
> > > this unused macro, never suggest to just undefine it in-tree.
> > >
> > > There is no reason to keep it in-tree, whether defined or undefined,
> > > just for downstream users.
> >
> > And this is where you and I fundamentally disagree.
>
> So you agree that I can add debugging printk's only for my own use?
> I can claim that I only use them downstream and you can't force me
> to carry local changes?
>
> If not, what is your criteria for accepting debugging printk's? Whose
> can be accepted and whose can't?
>
> Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
>
Per my personal view, I agree with you that we should remove it completely.
Clean up such kind of legacy code can make the kernel more clean.
Thanks
Yafang
^ permalink raw reply
* [PATCH] netlabel: fix out-of-bounds memory accesses
From: Paul Moore @ 2019-02-26 0:06 UTC (permalink / raw)
To: netdev; +Cc: linux-security-module, selinux
There are two array out-of-bounds memory accesses, one in
cipso_v4_map_lvl_valid(), the other in netlbl_bitmap_walk(). Both
errors are embarassingly simple, and the fixes are straightforward.
As a FYI for anyone backporting this patch to kernels prior to v4.8,
you'll want to apply the netlbl_bitmap_walk() patch to
cipso_v4_bitmap_walk() as netlbl_bitmap_walk() doesn't exist before
Linux v4.8.
Reported-by: Jann Horn <jannh@google.com>
Fixes: 446fda4f2682 ("[NetLabel]: CIPSOv4 engine")
Fixes: 3faa8f982f95 ("netlabel: Move bitmap manipulation functions to the NetLabel core.")
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
net/ipv4/cipso_ipv4.c | 3 ++-
net/netlabel/netlabel_kapi.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b7fb13..f4b83de2263e 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -667,7 +667,8 @@ static int cipso_v4_map_lvl_valid(const struct cipso_v4_doi *doi_def, u8 level)
case CIPSO_V4_MAP_PASS:
return 0;
case CIPSO_V4_MAP_TRANS:
- if (doi_def->map.std->lvl.cipso[level] < CIPSO_V4_INV_LVL)
+ if ((level < doi_def->map.std->lvl.cipso_size) &&
+ (doi_def->map.std->lvl.cipso[level] < CIPSO_V4_INV_LVL))
return 0;
break;
}
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index ea7c67050792..ee3e5b6471a6 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -903,7 +903,8 @@ int netlbl_bitmap_walk(const unsigned char *bitmap, u32 bitmap_len,
(state == 0 && (byte & bitmask) == 0))
return bit_spot;
- bit_spot++;
+ if (++bit_spot >= bitmap_len)
+ return -1;
bitmask >>= 1;
if (bitmask == 0) {
byte = bitmap[++byte_offset];
^ permalink raw reply related
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Dmitry Safonov @ 2019-02-26 0:09 UTC (permalink / raw)
To: Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <5079b888-a62f-991a-6f0b-1248d23d0045@gmail.com>
On 2/25/19 11:31 PM, Eric Dumazet wrote:
> On 02/25/2019 03:21 PM, Dmitry Safonov wrote:
>> Well, sure - but it seems confusing that rtnl_unregister() will require
>> synchronize_rcu(), while rtnl_unregister_all() will not.
>
> rtnl_unregister_all() is a different beast, since it removes the whole rtnl_msg_handlers[protocol]
>
> rtnl_unregister() only removes a subset, with different usages.
>
>> And I thought no one would care about another synchronize_rcu() in exit
>> path.
>
> We definitely care about things be done properly.
>
> If synchronize_rcu() is needed there, be it, but kfree_rcu() seems to be fine.
>
> In any case, I believe you need to more carefully explain what is the problem here,
> because I could not really see it.
Ugh, sorry - it seems that I haven't had enough coffee today.
I've read again rtnetlink_rcv_msg(), who is the only user of
rtnl_msg_handlers[] under read_lock, and it calls try_module_get(owner).
Sorry for the noise,
Dmitry
^ permalink raw reply
* Re: [PATCH net-next] net: sched: don't release block->lock when dumping chains
From: Cong Wang @ 2019-02-26 0:15 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190225154544.10453-1-vladbu@mellanox.com>
On Mon, Feb 25, 2019 at 7:45 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Function tc_dump_chain() obtains and releases block->lock on each iteration
> of its inner loop that dumps all chains on block. Outputting chain template
> info is fast operation so locking/unlocking mutex multiple times is an
> overhead when lock is highly contested. Modify tc_dump_chain() to only
> obtain block->lock once and dump all chains without releasing it.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks for the followup!
Isn't it similar for __tcf_get_next_proto() in tcf_chain_dump()?
And for tc_dump_tfilter()?
^ permalink raw reply
* [PATCH v4 perf,bpf 00/15] perf annotation of BPF programs
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
Changes v3 to v4:
1. Incorporate feedbacks from Jiri and Namhyung;
2. Fixed compilation error with different feature-disassembler-four-args;
3. Split some patches to smaller patches;
4. Improved error handleing in symbol__disassemble_bpf();
5. Made side band thread more generic;
6. Added comments as suggested.
Changes v2 to v3:
1. Remove unnecessary include in header files;
2. Improved error handling;
3. Better naming of functions, variables, etc.;
4. Enable bpf events by default for perf-top.
Changes v1 to v2:
1. Fix compilation error with different feature-disassembler-four-args;
2. Fix a segfault in perf-record;
3. Split patches 5/9 and 6/9 so that perf_env changes and perf.data changes
are in separate patches.
This series enables annotation of BPF programs in perf.
perf tool gathers information via sys_bpf and (optionally) stores them in
perf.data as headers.
Patch 1/15 fixes a minor issue in kernel;
Patch 2/15 to 4/15 introduce new helper functions and use them in perf and
bpftool;
Patch 5/15 to 9/15 saves information of bpf program in perf_env;
Patch 10/15 adds --bpf-event options to perf-top;
Patch 11/15 to 13/15 enables annotation of bpf progs based on information
gathered in 5/15 to 9/15;
Patch 14/15 introduces side band polling thread that gathers information
for special kernel events during perf-record or perf-top.
Patch 15/15 handles information of short living BPF program using the new
side band polling thread.
Commands tested during developments are perf-top, perf-record, perf-report,
and perf-annotate.
===================== Note on patch dependency ========================
This set has dependency in both bpf-next tree and tip/perf/core. Current
version is developed on bpf-next tree with the following commits
cherry-picked from tip/perf/core:
(from 1/10 to 10/10)
commit 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
commit d764ac646491 ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
commit 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
commit df063c83aa2c ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
commit 9aa0bfa370b2 ("perf tools: Handle PERF_RECORD_KSYMBOL")
commit 45178a928a4b ("perf tools: Handle PERF_RECORD_BPF_EVENT")
commit 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
commit a40b95bcd30c ("perf top: Synthesize BPF events for pre-existing loaded BPF programs")
commit 6934058d9fb6 ("bpf: Add module name [bpf] to ksymbols for bpf programs")
commit 811184fb6977 ("perf bpf: Fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT")
========================================================================
This set is also available at:
https://github.com/liu-song-6/linux/tree/bpf-annotation
Thanks!!
Song Liu (15):
perf, bpf: consider events with attr.bpf_event as side-band events
bpf: libbpf: introduce bpf_program__get_prog_info_linear()
bpf: bpftool: use bpf_program__get_prog_info_linear() in
prog.c:do_dump()
perf, bpf: synthesize bpf events with
bpf_program__get_prog_info_linear()
perf: change prototype of perf_event__synthesize_bpf_events()
perf, bpf: save bpf_prog_info in a rbtree in perf_env
perf, bpf: save bpf_prog_info information as headers to perf.data
perf, bpf: save btf in a rbtree in perf_env
perf, bpf: save btf information as headers to perf.data
perf-top: add option --no-bpf-event
perf: add -lopcodes to feature-libbfd
perf, bpf: enable annotation of bpf program
perf, bpf: process PERF_BPF_EVENT_PROG_LOAD for annotation
perf: introduce side band thread
perf, bpf: save information about short living bpf programs
kernel/events/core.c | 3 +-
tools/bpf/bpftool/prog.c | 266 +++++++------------------------
tools/build/Makefile.feature | 6 +-
tools/lib/bpf/libbpf.c | 251 +++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 63 ++++++++
tools/lib/bpf/libbpf.map | 3 +
tools/perf/Makefile.config | 6 +-
tools/perf/builtin-record.c | 12 +-
tools/perf/builtin-top.c | 15 +-
tools/perf/util/annotate.c | 149 ++++++++++++++++-
tools/perf/util/bpf-event.c | 301 +++++++++++++++++++++++++----------
tools/perf/util/bpf-event.h | 36 ++++-
tools/perf/util/dso.c | 1 +
tools/perf/util/dso.h | 33 ++--
tools/perf/util/env.c | 150 +++++++++++++++++
tools/perf/util/env.h | 21 +++
tools/perf/util/evlist.c | 105 ++++++++++++
tools/perf/util/evlist.h | 13 ++
tools/perf/util/header.c | 256 ++++++++++++++++++++++++++++-
tools/perf/util/header.h | 2 +
tools/perf/util/symbol.c | 1 +
tools/perf/util/top.h | 1 +
tools/perf/util/util.c | 10 ++
tools/perf/util/util.h | 2 +
24 files changed, 1389 insertions(+), 317 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v4 perf,bpf 01/15] perf, bpf: consider events with attr.bpf_event as side-band events
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
Events with bpf_event should be considered as side-band event, as they
carry information about BPF programs.
Fixes: 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
Signed-off-by: Song Liu <songliubraving@fb.com>
---
kernel/events/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 0a8dab322111..9403bdda5f8c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4238,7 +4238,8 @@ static bool is_sb_event(struct perf_event *event)
if (attr->mmap || attr->mmap_data || attr->mmap2 ||
attr->comm || attr->comm_exec ||
attr->task || attr->ksymbol ||
- attr->context_switch)
+ attr->context_switch ||
+ attr->bpf_event)
return true;
return false;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 04/15] perf, bpf: synthesize bpf events with bpf_program__get_prog_info_linear()
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
With bpf_program__get_prog_info_linear, we can simplify the logic that
synthesizes bpf events.
This patch doesn't change the behavior of the code.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/util/bpf-event.c | 118 ++++++++++++------------------------
1 file changed, 40 insertions(+), 78 deletions(-)
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 796ef793f4ce..e6dfb95029e5 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -3,7 +3,9 @@
#include <stdlib.h>
#include <bpf/bpf.h>
#include <bpf/btf.h>
+#include <bpf/libbpf.h>
#include <linux/btf.h>
+#include <linux/err.h>
#include "bpf-event.h"
#include "debug.h"
#include "symbol.h"
@@ -49,99 +51,62 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
{
struct ksymbol_event *ksymbol_event = &event->ksymbol_event;
struct bpf_event *bpf_event = &event->bpf_event;
- u32 sub_prog_cnt, i, func_info_rec_size = 0;
- u8 (*prog_tags)[BPF_TAG_SIZE] = NULL;
- struct bpf_prog_info info = { .type = 0, };
- u32 info_len = sizeof(info);
- void *func_infos = NULL;
- u64 *prog_addrs = NULL;
+ struct bpf_prog_info_linear *info_linear;
+ struct bpf_prog_info *info;
struct btf *btf = NULL;
- u32 *prog_lens = NULL;
bool has_btf = false;
- char errbuf[512];
+ u32 sub_prog_cnt, i;
int err = 0;
+ u64 arrays;
- /* Call bpf_obj_get_info_by_fd() to get sizes of arrays */
- err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+ arrays = 1UL << BPF_PROG_INFO_JITED_KSYMS;
+ arrays |= 1UL << BPF_PROG_INFO_JITED_FUNC_LENS;
+ arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
+ arrays |= 1UL << BPF_PROG_INFO_PROG_TAGS;
- if (err) {
- pr_debug("%s: failed to get BPF program info: %s, aborting\n",
- __func__, str_error_r(errno, errbuf, sizeof(errbuf)));
+ info_linear = bpf_program__get_prog_info_linear(fd, arrays);
+ if (IS_ERR_OR_NULL(info_linear)) {
+ info_linear = NULL;
+ pr_debug("%s: failed to get BPF program info. aborting\n", __func__);
return -1;
}
- if (info_len < offsetof(struct bpf_prog_info, prog_tags)) {
+
+ if (info_linear->info_len < offsetof(struct bpf_prog_info, prog_tags)) {
pr_debug("%s: the kernel is too old, aborting\n", __func__);
return -2;
}
+ info = &info_linear->info;
+
/* number of ksyms, func_lengths, and tags should match */
- sub_prog_cnt = info.nr_jited_ksyms;
- if (sub_prog_cnt != info.nr_prog_tags ||
- sub_prog_cnt != info.nr_jited_func_lens)
+ sub_prog_cnt = info->nr_jited_ksyms;
+ if (sub_prog_cnt != info->nr_prog_tags ||
+ sub_prog_cnt != info->nr_jited_func_lens)
return -1;
/* check BTF func info support */
- if (info.btf_id && info.nr_func_info && info.func_info_rec_size) {
+ if (info->btf_id && info->nr_func_info && info->func_info_rec_size) {
/* btf func info number should be same as sub_prog_cnt */
- if (sub_prog_cnt != info.nr_func_info) {
+ if (sub_prog_cnt != info->nr_func_info) {
pr_debug("%s: mismatch in BPF sub program count and BTF function info count, aborting\n", __func__);
- return -1;
- }
- if (btf__get_from_id(info.btf_id, &btf)) {
- pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, info.btf_id);
- return -1;
+ err = -1;
+ goto out;
}
- func_info_rec_size = info.func_info_rec_size;
- func_infos = calloc(sub_prog_cnt, func_info_rec_size);
- if (!func_infos) {
- pr_debug("%s: failed to allocate memory for func_infos, aborting\n", __func__);
- return -1;
+ if (btf__get_from_id(info->btf_id, &btf)) {
+ pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, info->btf_id);
+ err = -1;
+ btf = NULL;
+ goto out;
}
has_btf = true;
}
- /*
- * We need address, length, and tag for each sub program.
- * Allocate memory and call bpf_obj_get_info_by_fd() again
- */
- prog_addrs = calloc(sub_prog_cnt, sizeof(u64));
- if (!prog_addrs) {
- pr_debug("%s: failed to allocate memory for prog_addrs, aborting\n", __func__);
- goto out;
- }
- prog_lens = calloc(sub_prog_cnt, sizeof(u32));
- if (!prog_lens) {
- pr_debug("%s: failed to allocate memory for prog_lens, aborting\n", __func__);
- goto out;
- }
- prog_tags = calloc(sub_prog_cnt, BPF_TAG_SIZE);
- if (!prog_tags) {
- pr_debug("%s: failed to allocate memory for prog_tags, aborting\n", __func__);
- goto out;
- }
-
- memset(&info, 0, sizeof(info));
- info.nr_jited_ksyms = sub_prog_cnt;
- info.nr_jited_func_lens = sub_prog_cnt;
- info.nr_prog_tags = sub_prog_cnt;
- info.jited_ksyms = ptr_to_u64(prog_addrs);
- info.jited_func_lens = ptr_to_u64(prog_lens);
- info.prog_tags = ptr_to_u64(prog_tags);
- info_len = sizeof(info);
- if (has_btf) {
- info.nr_func_info = sub_prog_cnt;
- info.func_info_rec_size = func_info_rec_size;
- info.func_info = ptr_to_u64(func_infos);
- }
-
- err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
- if (err) {
- pr_debug("%s: failed to get BPF program info, aborting\n", __func__);
- goto out;
- }
-
/* Synthesize PERF_RECORD_KSYMBOL */
for (i = 0; i < sub_prog_cnt; i++) {
+ u8 (*prog_tags)[BPF_TAG_SIZE] = (void *)(info->prog_tags);
+ __u32 *prog_lens = (__u32 *)(info->jited_func_lens);
+ __u64 *prog_addrs = (__u64 *)(info->jited_ksyms);
+ void *func_infos = (void *)(info->func_info);
const struct bpf_func_info *finfo;
const char *short_name = NULL;
const struct btf_type *t;
@@ -163,13 +128,13 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
KSYM_NAME_LEN - name_len,
prog_tags[i], BPF_TAG_SIZE);
if (has_btf) {
- finfo = func_infos + i * info.func_info_rec_size;
+ finfo = func_infos + i * info->func_info_rec_size;
t = btf__type_by_id(btf, finfo->type_id);
short_name = btf__name_by_offset(btf, t->name_off);
} else if (i == 0 && sub_prog_cnt == 1) {
/* no subprog */
- if (info.name[0])
- short_name = info.name;
+ if (info->name[0])
+ short_name = info->name;
} else
short_name = "F";
if (short_name)
@@ -195,9 +160,9 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
},
.type = PERF_BPF_EVENT_PROG_LOAD,
.flags = 0,
- .id = info.id,
+ .id = info->id,
};
- memcpy(bpf_event->tag, prog_tags[i], BPF_TAG_SIZE);
+ memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
memset((void *)event + event->header.size, 0, machine->id_hdr_size);
event->header.size += machine->id_hdr_size;
err = perf_tool__process_synth_event(tool, event,
@@ -205,10 +170,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
}
out:
- free(prog_tags);
- free(prog_lens);
- free(prog_addrs);
- free(func_infos);
+ free(info_linear);
free(btf);
return err ? -1 : 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 05/15] perf: change prototype of perf_event__synthesize_bpf_events()
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
This patch changes the arguments of perf_event__synthesize_bpf_events()
to include perf_session* instead of perf_tool*. perf_session will be used
in the next patch.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/util/bpf-event.c | 8 +++++---
tools/perf/util/bpf-event.h | 4 ++--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 88ea11d57c6f..2355e0a9eda0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1083,7 +1083,7 @@ static int record__synthesize(struct record *rec, bool tail)
return err;
}
- err = perf_event__synthesize_bpf_events(tool, process_synthesized_event,
+ err = perf_event__synthesize_bpf_events(session, process_synthesized_event,
machine, opts);
if (err < 0)
pr_warning("Couldn't synthesize bpf events.\n");
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5a486d4de56e..27d8d42e0a4d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1216,7 +1216,7 @@ static int __cmd_top(struct perf_top *top)
init_process_thread(top);
- ret = perf_event__synthesize_bpf_events(&top->tool, perf_event__process,
+ ret = perf_event__synthesize_bpf_events(top->session, perf_event__process,
&top->session->machines.host,
&top->record_opts);
if (ret < 0)
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index e6dfb95029e5..ff7ee149ec46 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -10,6 +10,7 @@
#include "debug.h"
#include "symbol.h"
#include "machine.h"
+#include "session.h"
#define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
@@ -42,7 +43,7 @@ int machine__process_bpf_event(struct machine *machine __maybe_unused,
* -1 for failures;
* -2 for lack of kernel support.
*/
-static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
+static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
perf_event__handler_t process,
struct machine *machine,
int fd,
@@ -52,6 +53,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
struct ksymbol_event *ksymbol_event = &event->ksymbol_event;
struct bpf_event *bpf_event = &event->bpf_event;
struct bpf_prog_info_linear *info_linear;
+ struct perf_tool *tool = session->tool;
struct bpf_prog_info *info;
struct btf *btf = NULL;
bool has_btf = false;
@@ -175,7 +177,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
return err ? -1 : 0;
}
-int perf_event__synthesize_bpf_events(struct perf_tool *tool,
+int perf_event__synthesize_bpf_events(struct perf_session *session,
perf_event__handler_t process,
struct machine *machine,
struct record_opts *opts)
@@ -209,7 +211,7 @@ int perf_event__synthesize_bpf_events(struct perf_tool *tool,
continue;
}
- err = perf_event__synthesize_one_bpf_prog(tool, process,
+ err = perf_event__synthesize_one_bpf_prog(session, process,
machine, fd,
event, opts);
close(fd);
diff --git a/tools/perf/util/bpf-event.h b/tools/perf/util/bpf-event.h
index 7890067e1a37..6698683612a7 100644
--- a/tools/perf/util/bpf-event.h
+++ b/tools/perf/util/bpf-event.h
@@ -15,7 +15,7 @@ struct record_opts;
int machine__process_bpf_event(struct machine *machine, union perf_event *event,
struct perf_sample *sample);
-int perf_event__synthesize_bpf_events(struct perf_tool *tool,
+int perf_event__synthesize_bpf_events(struct perf_session *session,
perf_event__handler_t process,
struct machine *machine,
struct record_opts *opts);
@@ -27,7 +27,7 @@ static inline int machine__process_bpf_event(struct machine *machine __maybe_unu
return 0;
}
-static inline int perf_event__synthesize_bpf_events(struct perf_tool *tool __maybe_unused,
+static inline int perf_event__synthesize_bpf_events(struct perf_session *session __maybe_unused,
perf_event__handler_t process __maybe_unused,
struct machine *machine __maybe_unused,
struct record_opts *opts __maybe_unused)
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 09/15] perf, bpf: save btf information as headers to perf.data
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
This patch enables perf-record to save btf information as headers to
perf.data A new header type HEADER_BPF_BTF is introduced for this data.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
| 108 ++++++++++++++++++++++++++++++++++++++-
| 1 +
2 files changed, 108 insertions(+), 1 deletion(-)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c89dce7942d1..ce5402101468 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1127,6 +1127,45 @@ static int write_bpf_prog_info(struct feat_fd *ff,
return ret;
}
+static int write_bpf_btf(struct feat_fd *ff,
+ struct perf_evlist *evlist __maybe_unused)
+{
+ struct perf_env *env = &ff->ph->env;
+ struct rb_root *root;
+ struct rb_node *next;
+ u32 count = 0;
+ int ret;
+
+ down_read(&env->bpf_progs.lock);
+
+ root = &env->bpf_progs.btfs;
+ next = rb_first(root);
+ while (next) {
+ ++count;
+ next = rb_next(next);
+ }
+
+ ret = do_write(ff, &count, sizeof(count));
+
+ if (ret < 0)
+ goto out;
+
+ next = rb_first(root);
+ while (next) {
+ struct btf_node *node;
+
+ node = rb_entry(next, struct btf_node, rb_node);
+ next = rb_next(&node->rb_node);
+ ret = do_write(ff, &node->id,
+ sizeof(u32) * 2 + node->data_size);
+ if (ret < 0)
+ goto out;
+ }
+out:
+ up_read(&env->bpf_progs.lock);
+ return ret;
+}
+
static int cpu_cache_level__sort(const void *a, const void *b)
{
struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a;
@@ -1630,6 +1669,28 @@ static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp)
up_read(&env->bpf_progs.lock);
}
+static void print_bpf_btf(struct feat_fd *ff, FILE *fp)
+{
+ struct perf_env *env = &ff->ph->env;
+ struct rb_root *root;
+ struct rb_node *next;
+
+ down_read(&env->bpf_progs.lock);
+
+ root = &env->bpf_progs.btfs;
+ next = rb_first(root);
+
+ while (next) {
+ struct btf_node *node;
+
+ node = rb_entry(next, struct btf_node, rb_node);
+ next = rb_next(&node->rb_node);
+ fprintf(fp, "# btf info of id %u\n", node->id);
+ }
+
+ up_read(&env->bpf_progs.lock);
+}
+
static void free_event_desc(struct perf_evsel *events)
{
struct perf_evsel *evsel;
@@ -2732,6 +2793,50 @@ static int process_bpf_prog_info(struct feat_fd *ff,
return err;
}
+static int process_bpf_btf(struct feat_fd *ff, void *data __maybe_unused)
+{
+ struct perf_env *env = &ff->ph->env;
+ u32 count, i;
+
+ if (do_read_u32(ff, &count))
+ return -1;
+
+ down_write(&env->bpf_progs.lock);
+
+ for (i = 0; i < count; ++i) {
+ struct btf_node *node;
+ u32 id, data_size;
+
+ if (do_read_u32(ff, &id))
+ return -1;
+ if (do_read_u32(ff, &data_size))
+ return -1;
+
+ node = malloc(sizeof(struct btf_node) + data_size);
+ if (!node)
+ return -1;
+
+ node->id = id;
+ node->data_size = data_size;
+
+ if (__do_read(ff, node->data, data_size)) {
+ free(node);
+ return -1;
+ }
+
+ /* endian mismatch, drop the btf, continue */
+ if (ff->ph->needs_swap) {
+ free(node);
+ continue;
+ }
+
+ perf_env__insert_btf(env, node);
+ }
+
+ up_write(&env->bpf_progs.lock);
+ return 0;
+}
+
struct feature_ops {
int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
void (*print)(struct feat_fd *ff, FILE *fp);
@@ -2792,7 +2897,8 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
FEAT_OPR(SAMPLE_TIME, sample_time, false),
FEAT_OPR(MEM_TOPOLOGY, mem_topology, true),
FEAT_OPR(CLOCKID, clockid, false),
- FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false)
+ FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false),
+ FEAT_OPR(BPF_BTF, bpf_btf, false)
};
struct header_print_data {
--git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 0785c91b4c3a..9e7d931f7c0d 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -40,6 +40,7 @@ enum {
HEADER_MEM_TOPOLOGY,
HEADER_CLOCKID,
HEADER_BPF_PROG_INFO,
+ HEADER_BPF_BTF,
HEADER_LAST_FEATURE,
HEADER_FEAT_BITS = 256,
};
--
2.17.1
^ permalink raw reply related
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