* Re: [net-next 00/16][pull request] 100GbE Intel Wired LAN Driver Updates 2019-02-22
From: David Miller @ 2019-02-25 22:14 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann
In-Reply-To: <20190225184306.13505-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 25 Feb 2019 10:42:50 -0800
> This series contains updates to the ice driver only.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] net: phy: aquantia: add hwmon support
From: David Miller @ 2019-02-25 22:16 UTC (permalink / raw)
To: hkallweit1; +Cc: andrew, f.fainelli, netdev
In-Reply-To: <71142110-f853-86e8-e597-f205d97d3491@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 25 Feb 2019 19:52:14 +0100
> This series adds HWMON support for the temperature sensor and the
> related alarms on the 107/108/109 chips.
>
> v2:
> - remove struct aqr_priv
> - rename header file to aquantia.h
> v3:
> - add conditional compiling of aquantia_hwmon.c
> - improve converting sensor register values to/from long
> - add helper aqr_hwmon_test_bit
Series applied, thanks Heiner.
^ permalink raw reply
* Re: [PATCH net-next] mlxsw: spectrum: acl: Use struct_size() in kzalloc()
From: David Miller @ 2019-02-25 22:18 UTC (permalink / raw)
To: gustavo; +Cc: jiri, idosch, netdev, linux-kernel
In-Reply-To: <20190225190132.GA23478@embeddedor>
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
Date: Mon, 25 Feb 2019 13:01:32 -0600
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> struct boo entry[];
> };
>
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = kzalloc(size, GFP_KERNEL)
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
>
> Notice that, in this case, variable alloc_size is not necessary, hence
> it is removed.
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] MIPS: lantiq: Remove separate GPHY Firmware loader
From: Paul Burton @ 2019-02-25 22:19 UTC (permalink / raw)
To: Hauke Mehrtens
Cc: Paul Burton, linux-mips@vger.kernel.org,
devicetree@vger.kernel.org, john@phrozen.org,
martin.blumenstingl@googlemail.com, netdev@vger.kernel.org,
Hauke Mehrtens, linux-mips@vger.kernel.org
In-Reply-To: <20190222201347.29242-1-hauke@hauke-m.de>
Hello,
Hauke Mehrtens wrote:
> The separate GPHY Firmware loader driver is not used any more, the GPHY
> firmware is now loaded by the GSWIP switch driver which also makes use
> of the GPHY.
> Remove the old unused GPHY firmware loader driver.
>
> The GPHY firmware is useless without an Ethernet and switch driver, it
> should not harm if loading this does not work for system using an old
> device tree.
> I am not aware of any vendor separating the device tree from the kernel
> binary, it should be ok to remove this.
>
> The code and the functionality form this separate GPHY firmware loader
> was added to the gswip driver in commit 14fceff4771e ("net: dsa: Add
> Lantiq / Intel DSA driver for vrx200")
>
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
Applied to mips-next.
Thanks,
Paul
[ This message was auto-generated; if you believe anything is incorrect
then please email paul.burton@mips.com to report it. ]
^ permalink raw reply
* Re: [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: David Miller @ 2019-02-25 22:21 UTC (permalink / raw)
To: lesliemonis; +Cc: netdev, tahiliani, dave.taht, jhs
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>
From: Leslie Monis <lesliemonis@gmail.com>
Date: Tue, 26 Feb 2019 00:39:54 +0530
> The current implementation of the PIE queuing discipline is according to the
> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> However, a lot of necessary modifications and enhancements have been proposed
> in RFC 8033, which have not yet been incorporated in the source code of Linux.
> This patch series helps in achieving the same.
>
> Performance tests carried out using Flent [https://flent.org/]
>
> Changes from v2 to v3:
> - Used div_u64() instead of direct division after explicit type casting as
> recommended by David
>
> Changes from v1 to v2:
> - Excluded the patch setting PIE dynamically active/inactive as the test
> results were unsatisfactory
> - Fixed a scaling issue when adding more auto-tuning cases which caused
> local variables to underflow
> - Changed the long if/else chain to a loop as suggested by Stephen
> - Changed the position of the accu_prob variable in the pie_vars
> structure as recommended by Stephen
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH] tun: remove unnecessary memory barrier
From: David Miller @ 2019-02-25 22:28 UTC (permalink / raw)
To: mail; +Cc: eric.dumazet, netdev, linux-kernel
In-Reply-To: <20190225201313.GA30005@timur-laptop.fritz.box>
From: Timur Celik <mail@timurcelik.de>
Date: Mon, 25 Feb 2019 21:13:13 +0100
> Replace set_current_state with __set_current_state since no memory
> barrier is needed at this point.
>
> Signed-off-by: Timur Celik <mail@timurcelik.de>
Applied, thanks.
^ permalink raw reply
* [PATCH v3 bpf-next 2/4] bpf: expose program stats via bpf_prog_info
From: Alexei Starovoitov @ 2019-02-25 22:28 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-1-ast@kernel.org>
Return bpf program run_time_ns and run_cnt via bpf_prog_info
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Andrii Nakryiko <andriin@fb.com>
---
include/uapi/linux/bpf.h | 2 ++
kernel/bpf/syscall.c | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..2e308e90ffea 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2813,6 +2813,8 @@ struct bpf_prog_info {
__u32 jited_line_info_rec_size;
__u32 nr_prog_tags;
__aligned_u64 prog_tags;
+ __u64 run_time_ns;
+ __u64 run_cnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 31cf66fc3f5c..174581dfe225 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2152,6 +2152,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
struct bpf_prog_info __user *uinfo = u64_to_user_ptr(attr->info.info);
struct bpf_prog_info info = {};
u32 info_len = attr->info.info_len;
+ struct bpf_prog_stats stats;
char __user *uinsns;
u32 ulen;
int err;
@@ -2191,6 +2192,10 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
if (err)
return err;
+ bpf_prog_get_stats(prog, &stats);
+ info.run_time_ns = stats.nsecs;
+ info.run_cnt = stats.cnt;
+
if (!capable(CAP_SYS_ADMIN)) {
info.jited_prog_len = 0;
info.xlated_prog_len = 0;
--
2.20.0
^ permalink raw reply related
* [PATCH v3 bpf-next 3/4] tools/bpf: sync bpf.h into tools
From: Alexei Starovoitov @ 2019-02-25 22:28 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-1-ast@kernel.org>
sync bpf.h into tools directory
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/include/uapi/linux/bpf.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..2e308e90ffea 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2813,6 +2813,8 @@ struct bpf_prog_info {
__u32 jited_line_info_rec_size;
__u32 nr_prog_tags;
__aligned_u64 prog_tags;
+ __u64 run_time_ns;
+ __u64 run_cnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
--
2.20.0
^ permalink raw reply related
* [PATCH v3 bpf-next 4/4] tools/bpftool: recognize bpf_prog_info run_time_ns and run_cnt
From: Alexei Starovoitov @ 2019-02-25 22:28 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-1-ast@kernel.org>
$ bpftool p s
1: kprobe tag a56587d488d216c9 gpl run_time_ns 79786 run_cnt 8
loaded_at 2019-02-22T12:22:51-0800 uid 0
xlated 352B not jited memlock 4096B
$ bpftool --json --pretty p s
[{
"id": 1,
"type": "kprobe",
"tag": "a56587d488d216c9",
"gpl_compatible": true,
"run_time_ns": 79786,
"run_cnt": 8,
"loaded_at": 1550866971,
"uid": 0,
"bytes_xlated": 352,
"jited": false,
"bytes_memlock": 4096
}
]
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 +++-
tools/bpf/bpftool/prog.c | 7 +++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 12bc1e2d4b46..9386bd6e0396 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -171,7 +171,7 @@ EXAMPLES
::
- 10: xdp name some_prog tag 005a3d2123620c8b gpl
+ 10: xdp name some_prog tag 005a3d2123620c8b gpl run_time_ns 81632 run_cnt 10
loaded_at 2017-09-29T20:11:00+0000 uid 0
xlated 528B jited 370B memlock 4096B map_ids 10
@@ -184,6 +184,8 @@ EXAMPLES
"type": "xdp",
"tag": "005a3d2123620c8b",
"gpl_compatible": true,
+ "run_time_ns": 81632,
+ "run_cnt": 10,
"loaded_at": 1506715860,
"uid": 0,
"bytes_xlated": 528,
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index db978c8d76a8..0c35dd543d49 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -214,6 +214,10 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
+ if (info->run_time_ns) {
+ jsonw_uint_field(json_wtr, "run_time_ns", info->run_time_ns);
+ jsonw_uint_field(json_wtr, "run_cnt", info->run_cnt);
+ }
print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
@@ -277,6 +281,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
printf("%s", info->gpl_compatible ? " gpl" : "");
+ if (info->run_time_ns)
+ printf(" run_time_ns %lld run_cnt %lld",
+ info->run_time_ns, info->run_cnt);
printf("\n");
if (info->load_time) {
--
2.20.0
^ permalink raw reply related
* [PATCH v3 bpf-next 0/4] bpf: per program stats
From: Alexei Starovoitov @ 2019-02-25 22:28 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
Introduce per program stats to monitor the usage BPF
v2->v3:
- rename to run_time_ns/run_cnt everywhere
v1->v2:
- fixed u64 stats on 32-bit archs. Thanks Eric
- use more verbose run_time_ns in json output as suggested by Andrii
- refactored prog_alloc and clarified behavior of stats in subprogs
Alexei Starovoitov (4):
bpf: enable program stats
bpf: expose program stats via bpf_prog_info
tools/bpf: sync bpf.h into tools
tools/bpftool: recognize bpf_prog_info run_time_ns and run_cnt
include/linux/bpf.h | 9 +++++
include/linux/filter.h | 20 +++++++++-
include/uapi/linux/bpf.h | 2 +
kernel/bpf/core.c | 31 ++++++++++++++-
kernel/bpf/syscall.c | 39 ++++++++++++++++++-
kernel/bpf/verifier.c | 7 +++-
kernel/sysctl.c | 34 ++++++++++++++++
.../bpftool/Documentation/bpftool-prog.rst | 4 +-
tools/bpf/bpftool/prog.c | 7 ++++
tools/include/uapi/linux/bpf.h | 2 +
10 files changed, 148 insertions(+), 7 deletions(-)
--
2.20.0
^ permalink raw reply
* [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-25 22:28 UTC (permalink / raw)
To: davem; +Cc: daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-1-ast@kernel.org>
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(); \
+ 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 related
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: David Miller @ 2019-02-25 22:29 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: laoar.shao, daniel, edumazet, joe, netdev, shaoyafang
In-Reply-To: <CAM_iQpUePM2L5qStoq7XmQe-4sDrM9n6BRwa=FShfzZda=Lccw@mail.gmail.com>
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.
^ permalink raw reply
* Re: [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: David Miller @ 2019-02-25 22:33 UTC (permalink / raw)
To: s-nazarov; +Cc: paul, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <52295911551111855@myt5-f1576e7b5bad.qloud-c.yandex.net>
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.
^ permalink raw reply
* Re: [PATCH v2 2/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: David Miller @ 2019-02-25 22:34 UTC (permalink / raw)
To: s-nazarov; +Cc: paul, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <3666661551112035@myt2-dc4bba9bb23c.qloud-c.yandex.net>
From: Nazarov Sergey <s-nazarov@yandex.ru>
Date: Mon, 25 Feb 2019 19:27:15 +0300
> Extract IP options in cipso_v4_error and use __icmp_send.
>
> Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: lmc: 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, colin.king, yang.wei9
In-Reply-To: <1551106660-16428-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Mon, 25 Feb 2019 22:57:40 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in lmc_interrupt() when skb
> xmit done. It makes drop profiles(dropwatch, perf) more friendly.
>
> Delete a redundant comment line in lmc_interrupt().
>
> Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: wan: wanxl: 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, khc, yang.wei9
In-Reply-To: <1551106910-16515-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Mon, 25 Feb 2019 23:01:50 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in wanxl_tx_intr() 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: ixp4xx_hss: 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, khalasa, yang.wei9
In-Reply-To: <1551106977-16579-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Mon, 25 Feb 2019 23:02:57 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in hss_hdlc_txdone_irq() 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 01/12] net: sched: flower: don't check for rtnl on head dereference
From: Cong Wang @ 2019-02-25 22:39 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <vbf36obu9y9.fsf@mellanox.com>
On Mon, Feb 25, 2019 at 8:11 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
>
> On Fri 22 Feb 2019 at 19:32, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > So if it is no longer RCU any more, why do you still use
> > rcu_dereference_protected()? That is, why not just deref it as a raw
> > pointer?
Any answer for this question?
> >
> > And, I don't think I can buy your argument here. The RCU infrastructure
> > should not be changed even after your patches, the fast path is still
> > protocted by RCU read lock, while the slow path now is protected by
> > some smaller-scope locks. What makes cls_flower so unique that
> > it doesn't even need RCU here? tp->root is not reassigned but it is still
> > freed via RCU infra, that is in fl_destroy_sleepable().
> >
> > Thanks.
>
> My cls API patch set introduced reference counting for tcf_proto
> structure. With that change tp->ops->destroy() (which calls fl_destroy()
> and fl_destroy_sleepable(), in case of flower classifier) is only called
> after last reference to tp is released. All slow path users of tp->ops
> must obtain reference to tp, so concurrent call to fl_destroy() is not
> possible. Before this change tcf_proto structure didn't have reference
> counting support and required users to obtain rtnl mutex before calling
> its ops callbacks. This was verified in flower by using rtnl_dereference
> to obtain tp->root.
Yes, but fast path doesn't hold a refnct of tp, does it? If not, you still
rely on RCU for sync with readers. If yes, then probably RCU can be
gone.
Now you are in a middle of the two, that is taking RCU read lock on
fast path without a refcnt, meanwhile still uses rcu_dereference on
slow paths without any lock.
For me, you at least don't use the RCU API correctly here.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: wan: sbni: 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: <1551107020-16629-1-git-send-email-albin_yang@163.com>
From: Yang Wei <albin_yang@163.com>
Date: Mon, 25 Feb 2019 23:03:40 +0800
> From: Yang Wei <yang.wei9@zte.com.cn>
>
> dev_consume_skb_irq() should be called in send_complete() 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: 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
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