* [PATCH net-next v3 2/4] bpf: add a test case for helper bpf_perf_event_read_value
From: Yonghong Song @ 2017-09-18 23:05 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170918230553.1624357-1-yhs@fb.com>
The bpf sample program tracex6 is enhanced to use the new
helper to read enabled/running time as well.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/tracex6_kern.c | 26 ++++++++++++++++++++++++++
samples/bpf/tracex6_user.c | 13 ++++++++++++-
tools/include/uapi/linux/bpf.h | 3 ++-
tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
4 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/samples/bpf/tracex6_kern.c b/samples/bpf/tracex6_kern.c
index e7d1803..46c557a 100644
--- a/samples/bpf/tracex6_kern.c
+++ b/samples/bpf/tracex6_kern.c
@@ -15,6 +15,12 @@ struct bpf_map_def SEC("maps") values = {
.value_size = sizeof(u64),
.max_entries = 64,
};
+struct bpf_map_def SEC("maps") values2 = {
+ .type = BPF_MAP_TYPE_HASH,
+ .key_size = sizeof(int),
+ .value_size = sizeof(struct bpf_perf_event_value),
+ .max_entries = 64,
+};
SEC("kprobe/htab_map_get_next_key")
int bpf_prog1(struct pt_regs *ctx)
@@ -37,5 +43,25 @@ int bpf_prog1(struct pt_regs *ctx)
return 0;
}
+SEC("kprobe/htab_map_lookup_elem")
+int bpf_prog2(struct pt_regs *ctx)
+{
+ u32 key = bpf_get_smp_processor_id();
+ struct bpf_perf_event_value *val, buf;
+ int error;
+
+ error = bpf_perf_event_read_value(&counters, key, &buf, sizeof(buf));
+ if (error)
+ return 0;
+
+ val = bpf_map_lookup_elem(&values2, &key);
+ if (val)
+ *val = buf;
+ else
+ bpf_map_update_elem(&values2, &key, &buf, BPF_NOEXIST);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/tracex6_user.c b/samples/bpf/tracex6_user.c
index a05a99a..3341a96 100644
--- a/samples/bpf/tracex6_user.c
+++ b/samples/bpf/tracex6_user.c
@@ -22,6 +22,7 @@
static void check_on_cpu(int cpu, struct perf_event_attr *attr)
{
+ struct bpf_perf_event_value value2;
int pmu_fd, error = 0;
cpu_set_t set;
__u64 value;
@@ -46,8 +47,18 @@ static void check_on_cpu(int cpu, struct perf_event_attr *attr)
fprintf(stderr, "Value missing for CPU %d\n", cpu);
error = 1;
goto on_exit;
+ } else {
+ fprintf(stderr, "CPU %d: %llu\n", cpu, value);
+ }
+ /* The above bpf_map_lookup_elem should trigger the second kprobe */
+ if (bpf_map_lookup_elem(map_fd[2], &cpu, &value2)) {
+ fprintf(stderr, "Value2 missing for CPU %d\n", cpu);
+ error = 1;
+ goto on_exit;
+ } else {
+ fprintf(stderr, "CPU %d: counter: %llu, enabled: %llu, running: %llu\n", cpu,
+ value2.counter, value2.enabled, value2.running);
}
- fprintf(stderr, "CPU %d: %llu\n", cpu, value);
on_exit:
assert(bpf_map_delete_elem(map_fd[0], &cpu) == 0 || error);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 461811e..79eb529 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -632,7 +632,8 @@ union bpf_attr {
FN(skb_adjust_room), \
FN(redirect_map), \
FN(sk_redirect_map), \
- FN(sock_map_update),
+ FN(sock_map_update), \
+ FN(perf_event_read_value),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 36fb916..c866682 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -70,6 +70,9 @@ static int (*bpf_sk_redirect_map)(void *map, int key, int flags) =
static int (*bpf_sock_map_update)(void *map, void *key, void *value,
unsigned long long flags) =
(void *) BPF_FUNC_sock_map_update;
+static int (*bpf_perf_event_read_value)(void *map, unsigned long long flags,
+ void *buf, unsigned int buf_size) =
+ (void *) BPF_FUNC_perf_event_read_value;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v3 3/4] bpf: add helper bpf_perf_prog_read_value
From: Yonghong Song @ 2017-09-18 23:05 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170918230553.1624357-1-yhs@fb.com>
This patch adds helper bpf_perf_prog_read_cvalue for perf event based bpf
programs, to read event counter and enabled/running time.
The enabled/running time is accumulated since the perf event open.
The typical use case for perf event based bpf program is to attach itself
to a single event. In such cases, if it is desirable to get scaling factor
between two bpf invocations, users can can save the time values in a map,
and use the value from the map and the current value to calculate
the scaling factor.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 1 +
include/uapi/linux/bpf.h | 8 ++++++++
kernel/events/core.c | 1 +
kernel/trace/bpf_trace.c | 23 +++++++++++++++++++++++
4 files changed, 33 insertions(+)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 13f08ee..5ff3055 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -806,6 +806,7 @@ struct perf_output_handle {
struct bpf_perf_event_data_kern {
struct pt_regs *regs;
struct perf_sample_data *data;
+ struct perf_event *event;
};
#ifdef CONFIG_CGROUP_PERF
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 2c68b9e..ba77022 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -590,6 +590,13 @@ union bpf_attr {
* @buf: buf to fill
* @buf_size: size of the buf
* Return: 0 on success or negative error code
+ *
+ * int bpf_perf_prog_read_value(ctx, buf, buf_size)
+ * read perf prog attached perf event counter and enabled/running time
+ * @ctx: pointer to ctx
+ * @buf: buf to fill
+ * @buf_size: size of the buf
+ * Return : 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -647,6 +654,7 @@ union bpf_attr {
FN(sk_redirect_map), \
FN(sock_map_update), \
FN(perf_event_read_value), \
+ FN(perf_prog_read_value), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2d5bbe5..d039086 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8081,6 +8081,7 @@ static void bpf_overflow_handler(struct perf_event *event,
struct bpf_perf_event_data_kern ctx = {
.data = data,
.regs = regs,
+ .event = event,
};
int ret = 0;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 39ce5d9..596b5c9 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -603,6 +603,18 @@ BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
flags, 0, 0);
}
+BPF_CALL_3(bpf_perf_prog_read_value_tp, void *, ctx, struct bpf_perf_event_value *,
+ buf, u32, size)
+{
+ struct bpf_perf_event_data_kern *kctx = (struct bpf_perf_event_data_kern *)ctx;
+
+ if (size != sizeof(struct bpf_perf_event_value))
+ return -EINVAL;
+
+ return perf_event_read_local(kctx->event, &buf->counter, &buf->enabled,
+ &buf->running);
+}
+
static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
.func = bpf_get_stackid_tp,
.gpl_only = true,
@@ -612,6 +624,15 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
.arg3_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = {
+ .func = bpf_perf_prog_read_value_tp,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg3_type = ARG_CONST_SIZE,
+};
+
static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
{
switch (func_id) {
@@ -619,6 +640,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
return &bpf_perf_event_output_proto_tp;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto_tp;
+ case BPF_FUNC_perf_prog_read_value:
+ return &bpf_perf_prog_read_value_proto_tp;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v3 4/4] bpf: add a test case for helper bpf_perf_prog_read_value
From: Yonghong Song @ 2017-09-18 23:05 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170918230553.1624357-1-yhs@fb.com>
The bpf sample program trace_event is enhanced to use the new
helper to print out enabled/running time.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/trace_event_kern.c | 10 ++++++++++
samples/bpf/trace_event_user.c | 13 ++++++++-----
tools/include/uapi/linux/bpf.h | 3 ++-
tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 41b6115..a77a583d 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -37,10 +37,14 @@ struct bpf_map_def SEC("maps") stackmap = {
SEC("perf_event")
int bpf_prog1(struct bpf_perf_event_data *ctx)
{
+ char time_fmt1[] = "Time Enabled: %llu, Time Running: %llu";
+ char time_fmt2[] = "Get Time Failed, ErrCode: %d";
char fmt[] = "CPU-%d period %lld ip %llx";
u32 cpu = bpf_get_smp_processor_id();
+ struct bpf_perf_event_value value_buf;
struct key_t key;
u64 *val, one = 1;
+ int ret;
if (ctx->sample_period < 10000)
/* ignore warmup */
@@ -54,6 +58,12 @@ int bpf_prog1(struct bpf_perf_event_data *ctx)
return 0;
}
+ ret = bpf_perf_prog_read_value(ctx, (void *)&value_buf, sizeof(struct bpf_perf_event_value));
+ if (!ret)
+ bpf_trace_printk(time_fmt1, sizeof(time_fmt1), value_buf.enabled, value_buf.running);
+ else
+ bpf_trace_printk(time_fmt2, sizeof(time_fmt2), ret);
+
val = bpf_map_lookup_elem(&counts, &key);
if (val)
(*val)++;
diff --git a/samples/bpf/trace_event_user.c b/samples/bpf/trace_event_user.c
index 7bd827b..bf4f1b6 100644
--- a/samples/bpf/trace_event_user.c
+++ b/samples/bpf/trace_event_user.c
@@ -127,6 +127,9 @@ static void test_perf_event_all_cpu(struct perf_event_attr *attr)
int *pmu_fd = malloc(nr_cpus * sizeof(int));
int i, error = 0;
+ /* system wide perf event, no need to inherit */
+ attr->inherit = 0;
+
/* open perf_event on all cpus */
for (i = 0; i < nr_cpus; i++) {
pmu_fd[i] = sys_perf_event_open(attr, -1, i, -1, 0);
@@ -154,6 +157,11 @@ static void test_perf_event_task(struct perf_event_attr *attr)
{
int pmu_fd;
+ /* per task perf event, enable inherit so the "dd ..." command can be traced properly.
+ * Enabling inherit will cause bpf_perf_prog_read_time helper failure.
+ */
+ attr->inherit = 1;
+
/* open task bound event */
pmu_fd = sys_perf_event_open(attr, 0, -1, -1, 0);
if (pmu_fd < 0) {
@@ -175,14 +183,12 @@ static void test_bpf_perf_event(void)
.freq = 1,
.type = PERF_TYPE_HARDWARE,
.config = PERF_COUNT_HW_CPU_CYCLES,
- .inherit = 1,
};
struct perf_event_attr attr_type_sw = {
.sample_freq = SAMPLE_FREQ,
.freq = 1,
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_CPU_CLOCK,
- .inherit = 1,
};
struct perf_event_attr attr_hw_cache_l1d = {
.sample_freq = SAMPLE_FREQ,
@@ -192,7 +198,6 @@ static void test_bpf_perf_event(void)
PERF_COUNT_HW_CACHE_L1D |
(PERF_COUNT_HW_CACHE_OP_READ << 8) |
(PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
- .inherit = 1,
};
struct perf_event_attr attr_hw_cache_branch_miss = {
.sample_freq = SAMPLE_FREQ,
@@ -202,7 +207,6 @@ static void test_bpf_perf_event(void)
PERF_COUNT_HW_CACHE_BPU |
(PERF_COUNT_HW_CACHE_OP_READ << 8) |
(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
- .inherit = 1,
};
struct perf_event_attr attr_type_raw = {
.sample_freq = SAMPLE_FREQ,
@@ -210,7 +214,6 @@ static void test_bpf_perf_event(void)
.type = PERF_TYPE_RAW,
/* Intel Instruction Retired */
.config = 0xc0,
- .inherit = 1,
};
printf("Test HW_CPU_CYCLES\n");
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 79eb529..fa1be2c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -633,7 +633,8 @@ union bpf_attr {
FN(redirect_map), \
FN(sk_redirect_map), \
FN(sock_map_update), \
- FN(perf_event_read_value),
+ FN(perf_event_read_value), \
+ FN(perf_prog_read_value),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index c866682..892d785 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -73,6 +73,9 @@ static int (*bpf_sock_map_update)(void *map, void *key, void *value,
static int (*bpf_perf_event_read_value)(void *map, unsigned long long flags,
void *buf, unsigned int buf_size) =
(void *) BPF_FUNC_perf_event_read_value;
+static int (*bpf_perf_prog_read_value)(void *ctx, void *buf,
+ unsigned int buf_size) =
+ (void *) BPF_FUNC_perf_prog_read_value;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v3 1/4] bpf: add helper bpf_perf_event_read_value for perf event array map
From: Yonghong Song @ 2017-09-18 23:05 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170918230553.1624357-1-yhs@fb.com>
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch adds helper bpf_perf_event_read_value for kprobed based perf
event array map, to read perf counter and enabled/running time.
The enabled/running time is accumulated since the perf event open.
To achieve scaling factor between two bpf invocations, users
can can use cpu_id as the key (which is typical for perf array usage model)
to remember the previous value and do the calculation inside the
bpf program.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 3 ++-
include/uapi/linux/bpf.h | 18 +++++++++++++++++-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/verifier.c | 4 +++-
kernel/events/core.c | 15 ++++++++++++---
kernel/trace/bpf_trace.c | 44 ++++++++++++++++++++++++++++++++++++++++----
6 files changed, 75 insertions(+), 11 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 8e22f24..13f08ee 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -884,7 +884,8 @@ perf_event_create_kernel_counter(struct perf_event_attr *attr,
void *context);
extern void perf_pmu_migrate_context(struct pmu *pmu,
int src_cpu, int dst_cpu);
-int perf_event_read_local(struct perf_event *event, u64 *value);
+int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running);
extern u64 perf_event_read_value(struct perf_event *event,
u64 *enabled, u64 *running);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 43ab5c4..2c68b9e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -582,6 +582,14 @@ union bpf_attr {
* @map: pointer to sockmap to update
* @key: key to insert/update sock in map
* @flags: same flags as map update elem
+ *
+ * int bpf_perf_event_read_value(map, flags, buf, buf_size)
+ * read perf event counter value and perf event enabled/running time
+ * @map: pointer to perf_event_array map
+ * @flags: index of event in the map or bitmask flags
+ * @buf: buf to fill
+ * @buf_size: size of the buf
+ * Return: 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -638,6 +646,7 @@ union bpf_attr {
FN(redirect_map), \
FN(sk_redirect_map), \
FN(sock_map_update), \
+ FN(perf_event_read_value), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -681,7 +690,8 @@ enum bpf_func_id {
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
#define BPF_F_DONT_FRAGMENT (1ULL << 2)
-/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
+/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
+ * BPF_FUNC_perf_event_read_value flags. */
#define BPF_F_INDEX_MASK 0xffffffffULL
#define BPF_F_CURRENT_CPU BPF_F_INDEX_MASK
/* BPF_FUNC_perf_event_output for sk_buff input context. */
@@ -864,4 +874,10 @@ enum {
#define TCP_BPF_IW 1001 /* Set TCP initial congestion window */
#define TCP_BPF_SNDCWND_CLAMP 1002 /* Set sndcwnd_clamp */
+struct bpf_perf_event_value {
+ __u64 counter;
+ __u64 enabled;
+ __u64 running;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 98c0f00..68d8666 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -492,7 +492,7 @@ static void *perf_event_fd_array_get_ptr(struct bpf_map *map,
ee = ERR_PTR(-EOPNOTSUPP);
event = perf_file->private_data;
- if (perf_event_read_local(event, &value) == -EOPNOTSUPP)
+ if (perf_event_read_local(event, &value, NULL, NULL) == -EOPNOTSUPP)
goto err_out;
ee = bpf_event_entry_gen(perf_file, map_file);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 799b245..1bf9d7b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1494,7 +1494,8 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
if (func_id != BPF_FUNC_perf_event_read &&
- func_id != BPF_FUNC_perf_event_output)
+ func_id != BPF_FUNC_perf_event_output &&
+ func_id != BPF_FUNC_perf_event_read_value)
goto error;
break;
case BPF_MAP_TYPE_STACK_TRACE:
@@ -1537,6 +1538,7 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_FUNC_perf_event_read:
case BPF_FUNC_perf_event_output:
+ case BPF_FUNC_perf_event_read_value:
if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
goto error;
break;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3e691b7..2d5bbe5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3684,10 +3684,12 @@ static inline u64 perf_event_count(struct perf_event *event)
* will not be local and we cannot read them atomically
* - must not have a pmu::count method
*/
-int perf_event_read_local(struct perf_event *event, u64 *value)
+int perf_event_read_local(struct perf_event *event, u64 *value,
+ u64 *enabled, u64 *running)
{
unsigned long flags;
int ret = 0;
+ u64 now;
/*
* Disabling interrupts avoids all counter scheduling (context
@@ -3718,14 +3720,21 @@ int perf_event_read_local(struct perf_event *event, u64 *value)
goto out;
}
+ now = event->shadow_ctx_time + perf_clock();
+ if (enabled)
+ *enabled = now - event->tstamp_enabled;
/*
* If the event is currently on this CPU, its either a per-task event,
* or local to this CPU. Furthermore it means its ACTIVE (otherwise
* oncpu == -1).
*/
- if (event->oncpu == smp_processor_id())
+ if (event->oncpu == smp_processor_id()) {
event->pmu->read(event);
-
+ if (running)
+ *running = now - event->tstamp_running;
+ } else if (running) {
+ *running = event->total_time_running;
+ }
*value = local64_read(&event->count);
out:
local_irq_restore(flags);
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index dc498b6..39ce5d9 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
return &bpf_trace_printk_proto;
}
-BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
-{
+static __always_inline int
+get_map_perf_counter(struct bpf_map *map, u64 flags,
+ u64 *value, u64 *enabled, u64 *running) {
struct bpf_array *array = container_of(map, struct bpf_array, map);
unsigned int cpu = smp_processor_id();
u64 index = flags & BPF_F_INDEX_MASK;
struct bpf_event_entry *ee;
- u64 value = 0;
int err;
if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
@@ -275,7 +275,17 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
if (!ee)
return -ENOENT;
- err = perf_event_read_local(ee->event, &value);
+ err = perf_event_read_local(ee->event, value, enabled, running);
+ return err;
+}
+
+
+BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
+{
+ u64 value = 0;
+ int err;
+
+ err = get_map_perf_counter(map, flags, &value, NULL, NULL);
/*
* this api is ugly since we miss [-22..-2] range of valid
* counter values, but that's uapi
@@ -285,6 +295,20 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
return value;
}
+BPF_CALL_4(bpf_perf_event_read_value, struct bpf_map *, map, u64, flags,
+ struct bpf_perf_event_value *, buf, u32, size)
+{
+ int err;
+
+ if (unlikely(size != sizeof(struct bpf_perf_event_value)))
+ return -EINVAL;
+ err = get_map_perf_counter(map, flags, &buf->counter, &buf->enabled,
+ &buf->running);
+ if (err)
+ return err;
+ return 0;
+}
+
static const struct bpf_func_proto bpf_perf_event_read_proto = {
.func = bpf_perf_event_read,
.gpl_only = true,
@@ -293,6 +317,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
.arg2_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_event_read_value_proto = {
+ .func = bpf_perf_event_read_value,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg4_type = ARG_CONST_SIZE,
+};
+
static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
static __always_inline u64
@@ -499,6 +533,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_perf_event_output_proto;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto;
+ case BPF_FUNC_perf_event_read_value:
+ return &bpf_perf_event_read_value_proto;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v3 0/4] bpf: add two helpers to read perf event enabled/running time
From: Yonghong Song @ 2017-09-18 23:05 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch set implements two helper functions.
The helper bpf_perf_event_read_value reads counter/time_enabled/time_running
for perf event array map. The helper bpf_perf_prog_read_value read
counter/time_enabled/time_running for bpf prog with type BPF_PROG_TYPE_PERF_EVENT.
Changelogs:
v2->v3:
. counters should be read in order to read enabled/running time. This is to
prevent that counters and enabled/running time may be read separately.
v1->v2:
. reading enabled/running time should be together with reading counters
which contains the logic to ensure the result is valid.
Yonghong Song (4):
bpf: add helper bpf_perf_event_read_value for perf event array map
bpf: add a test case for helper bpf_perf_event_read_value
bpf: add helper bpf_perf_prog_read_value
bpf: add a test case for helper bpf_perf_prog_read_value
include/linux/perf_event.h | 4 +-
include/uapi/linux/bpf.h | 26 +++++++++++-
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/verifier.c | 4 +-
kernel/events/core.c | 16 ++++++--
kernel/trace/bpf_trace.c | 67 +++++++++++++++++++++++++++++--
samples/bpf/trace_event_kern.c | 10 +++++
samples/bpf/trace_event_user.c | 13 +++---
samples/bpf/tracex6_kern.c | 26 ++++++++++++
samples/bpf/tracex6_user.c | 13 +++++-
tools/include/uapi/linux/bpf.h | 4 +-
tools/testing/selftests/bpf/bpf_helpers.h | 6 +++
12 files changed, 173 insertions(+), 18 deletions(-)
--
2.9.5
^ permalink raw reply
* Re: [PATCH net-next 3/3] bpf: Test deletion in BPF_MAP_TYPE_LPM_TRIE
From: Alexei Starovoitov @ 2017-09-18 22:54 UTC (permalink / raw)
To: Craig Gallek, Daniel Mack, Daniel Borkmann, David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-4-kraigatgoog@gmail.com>
On 9/18/17 12:30 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> Extend the 'random' operation tests to include a delete operation
> (delete half of the nodes from both lpm implementions and ensure
> that lookups are still equivalent).
>
> Also, add a simple IPv4 test which verifies lookup behavior as nodes
> are deleted from the tree.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
Thanks for the tests!
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next 2/3] bpf: Add uniqueness invariant to trivial lpm test implementation
From: Alexei Starovoitov @ 2017-09-18 22:54 UTC (permalink / raw)
To: Craig Gallek, Daniel Mack, Daniel Borkmann, David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-3-kraigatgoog@gmail.com>
On 9/18/17 12:30 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> The 'trivial' lpm implementation in this test allows equivalent nodes
> to be added (that is, nodes consisting of the same prefix and prefix
> length). For lookup operations, this is fine because insertion happens
> at the head of the (singly linked) list and the first, best match is
> returned. In order to support deletion, the tlpm data structue must
> first enforce uniqueness. This change modifies the insertion algorithm
> to search for equivalent nodes and remove them. Note: the
> BPF_MAP_TYPE_LPM_TRIE already has a uniqueness invariant that is
> implemented as node replacement.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Alexei Starovoitov @ 2017-09-18 22:53 UTC (permalink / raw)
To: Craig Gallek, Daniel Mack, Daniel Borkmann, David S . Miller; +Cc: netdev
In-Reply-To: <20170918193057.37644-2-kraigatgoog@gmail.com>
On 9/18/17 12:30 PM, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
>
> This is a simple non-recursive delete operation. It prunes paths
> of empty nodes in the tree, but it does not try to further compress
> the tree as nodes are removed.
>
> Signed-off-by: Craig Gallek <kraig@google.com>
> ---
> kernel/bpf/lpm_trie.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 77 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
> index 1b767844a76f..9d58a576b2ae 100644
> --- a/kernel/bpf/lpm_trie.c
> +++ b/kernel/bpf/lpm_trie.c
> @@ -389,10 +389,84 @@ static int trie_update_elem(struct bpf_map *map,
> return ret;
> }
>
> -static int trie_delete_elem(struct bpf_map *map, void *key)
> +/* Called from syscall or from eBPF program */
> +static int trie_delete_elem(struct bpf_map *map, void *_key)
> {
> - /* TODO */
> - return -ENOSYS;
> + struct lpm_trie *trie = container_of(map, struct lpm_trie, map);
> + struct bpf_lpm_trie_key *key = _key;
> + struct lpm_trie_node __rcu **trim;
> + struct lpm_trie_node *node;
> + unsigned long irq_flags;
> + unsigned int next_bit;
> + size_t matchlen = 0;
> + int ret = 0;
> +
> + if (key->prefixlen > trie->max_prefixlen)
> + return -EINVAL;
> +
> + raw_spin_lock_irqsave(&trie->lock, irq_flags);
> +
> + /* Walk the tree looking for an exact key/length match and keeping
> + * track of where we could begin trimming the tree. The trim-point
> + * is the sub-tree along the walk consisting of only single-child
> + * intermediate nodes and ending at a leaf node that we want to
> + * remove.
> + */
> + trim = &trie->root;
> + node = rcu_dereference_protected(
> + trie->root, lockdep_is_held(&trie->lock));
> + while (node) {
> + matchlen = longest_prefix_match(trie, node, key);
> +
> + if (node->prefixlen != matchlen ||
> + node->prefixlen == key->prefixlen)
> + break;
curious why there is no need to do
'node->prefixlen == trie->max_prefixlen' in the above
like update/lookup do?
> +
> + next_bit = extract_bit(key->data, node->prefixlen);
> + /* If we hit a node that has more than one child or is a valid
> + * prefix itself, do not remove it. Reset the root of the trim
> + * path to its descendant on our path.
> + */
> + if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
> + (node->child[0] && node->child[1]))
> + trim = &node->child[next_bit];
> + node = rcu_dereference_protected(
> + node->child[next_bit], lockdep_is_held(&trie->lock));
> + }
> +
> + if (!node || node->prefixlen != key->prefixlen ||
> + (node->flags & LPM_TREE_NODE_FLAG_IM)) {
> + ret = -ENOENT;
> + goto out;
> + }
> +
> + trie->n_entries--;
> +
> + /* If the node we are removing is not a leaf node, simply mark it
> + * as intermediate and we are done.
> + */
> + if (rcu_access_pointer(node->child[0]) ||
> + rcu_access_pointer(node->child[1])) {
> + node->flags |= LPM_TREE_NODE_FLAG_IM;
> + goto out;
> + }
> +
> + /* trim should now point to the slot holding the start of a path from
> + * zero or more intermediate nodes to our leaf node for deletion.
> + */
> + while ((node = rcu_dereference_protected(
> + *trim, lockdep_is_held(&trie->lock)))) {
> + RCU_INIT_POINTER(*trim, NULL);
> + trim = rcu_access_pointer(node->child[0]) ?
> + &node->child[0] :
> + &node->child[1];
> + kfree_rcu(node, rcu);
can it be that some of the nodes this loop walks have
both child[0] and [1] ?
^ permalink raw reply
* Re: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Andrew Lunn @ 2017-09-18 22:51 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41124D7C@CHN-SV-EXMX02.mchp-main.com>
> In the old DSA implementation all the ports are partitioned into its own device
> and the bridge joining them will do all the forwarding. This is useful for quick
> testing with some protocols like RSTP but it is probably useless for real
> operation.
It is a good minimal driver, to get something into the kernel. You can
then add features to it.
> The new switchdev model tries to use the switch hardware as much as
> possible. This offload_fwd_mark bit means the frame is forwarded by the
> hardware switch, so the software bridge does not need to do it again. Without
> this bit there will be duplicated multicast frames coming out the ports if internal
> forwarding is enabled.
Correct. Once you switch driver is clever enough, you can enable
offload_fwd_mark.
> When RSTP is used the port can be put in blocked state and so the forwarding
> will stop for that port. Currently the switch driver will check that membership
> to decide whether to set that bit.
This i don't get. RSTP or STP just break loops. How does RSTP vs STP
mean you need to set offload_fwd_mark differently?
> The KSZ switches never have a built-in MAC controller, so it is always a support
> issue for us.
Not quite right. Once your drivers are in mainline, it becomes a
support issue for the community, with you being part of the community.
I've helped others fix this issue, one of the less well used Marvell
Ethernet drivers had this problem, and i gave somebody pointers how to
fix it.
Andrew
^ permalink raw reply
* Re: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Andrew Lunn @ 2017-09-18 22:42 UTC (permalink / raw)
To: Tristram.Ha
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41124D7C@CHN-SV-EXMX02.mchp-main.com>
On Mon, Sep 18, 2017 at 08:55:17PM +0000, Tristram.Ha@microchip.com wrote:
> > > > This is ugly. We have a clean separation between a switch driver and a
> > > > tag driver. Here you are mixing them together. Don't. Look at the
> > > > mailing lists for what Florian and I suggested to Pavel. It will also
> > > > solve your include file issue above.
> > >
> > > It seems to me all tag_*.c are hard-coded. They do not have access to
> > > the switch device and just use the bit definitions defined in the top to
> > > do the job.
> > >
> > > This creates a problem for all KSZ switch devices as they have different
> > > tail tag formats and lengths. There will be potentially 4 additional
> > > DSA_TAG_PROT_KSZ* just to handle them. Extra files will be added
> > > with about the same code.
> >
> > Hi Tristram
> >
> > Think about factoring out the common code into a shared functions.
> >
>
> I am a little unsure what you have in mind. Can you elaborate?
You say you need 4 DSA_TAG_PROT_KSZ. I guess the code is nearly
identical in them all? If i remember correctly, the two tag bytes are
the other way around?
static int ksz8k_xmit( *skb, struct net_device *dev)
{
uint8* tag;
tag = ksz_xmit(skb, dev)
if (!tag)
return NULL;
tag[0] = 1 << p->dp->index;
tag[1] = 0;
return skb;
}
static int ksz9k_xmit( *skb, struct net_device *dev)
{
uint8* tag;
tag = ksz_xmit(skb, dev)
if (!tag)
return NULL;
tag[0] = 0;
tag[1] = 1 << p->dp->index;
return skb;
}
const struct dsa_device_ops ksz8k_netdev_ops = {
.xmit = ksz8k_xmit,
.rcv = ksz8k_rcv,
};
const struct dsa_device_ops ksz9k_netdev_ops = {
.xmit = ksz9k_xmit,
.rcv = ksz9k_rcv,
};
Andrew
^ permalink raw reply
* Re: [PATCH net-next 09/12] net: dsa: b53: Wire-up EEE
From: Florian Fainelli @ 2017-09-18 22:34 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: davem, andrew
In-Reply-To: <87mv5r1vzx.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 09/18/2017 03:29 PM, Vivien Didelot wrote:
> Hi Florian,
>
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> @@ -1000,6 +1005,9 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
>> b53_write8(dev, B53_CTRL_PAGE, po_reg, gmii_po);
>> }
>> }
>> +
>> + /* Re-negotiate EEE if it was enabled already */
>> + p->eee_enabled = b53_eee_init(ds, port, phydev);
>> }
>
> Same here, I think we can move this up to DSA core, maybe with a
> eee_enabled mask in dsa_switch or a bool in dsa_port.
This can be done as a subsequent patch, sure.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 08/12] net: dsa: b53: Move EEE functions to b53
From: Florian Fainelli @ 2017-09-18 22:33 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: davem, andrew
In-Reply-To: <87poan1w2q.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 09/18/2017 03:27 PM, Vivien Didelot wrote:
> Hi Florian,
>
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> @@ -649,7 +595,7 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
>> core_writel(priv, reg, offset);
>>
>> if (!phydev->is_pseudo_fixed_link)
>> - p->eee_enabled = bcm_sf2_eee_init(ds, port, phydev);
>> + p->eee_enabled = b53_eee_init(ds, port, phydev);
>> }
>
> I know this is a bit out-of-scope of this patch, but I have to say I am
> not confortable with having still phy device stuffs in switch drivers...
Yes, this is out of scope :)
>
> Can this is_pseudo_fixed_link check + phy_eee_init + eee_enable be moved
> up to dsa_slave_adjust_link in a future patch maybe?
Not 100% positive this applies to all switches, which is why this is
still largely a switch driver decision.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 09/12] net: dsa: b53: Wire-up EEE
From: Vivien Didelot @ 2017-09-18 22:29 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-10-f.fainelli@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> @@ -1000,6 +1005,9 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
> b53_write8(dev, B53_CTRL_PAGE, po_reg, gmii_po);
> }
> }
> +
> + /* Re-negotiate EEE if it was enabled already */
> + p->eee_enabled = b53_eee_init(ds, port, phydev);
> }
Same here, I think we can move this up to DSA core, maybe with a
eee_enabled mask in dsa_switch or a bool in dsa_port.
^ permalink raw reply
* Re: [PATCH net-next 08/12] net: dsa: b53: Move EEE functions to b53
From: Vivien Didelot @ 2017-09-18 22:27 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-9-f.fainelli@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> @@ -649,7 +595,7 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
> core_writel(priv, reg, offset);
>
> if (!phydev->is_pseudo_fixed_link)
> - p->eee_enabled = bcm_sf2_eee_init(ds, port, phydev);
> + p->eee_enabled = b53_eee_init(ds, port, phydev);
> }
I know this is a bit out-of-scope of this patch, but I have to say I am
not confortable with having still phy device stuffs in switch drivers...
Can this is_pseudo_fixed_link check + phy_eee_init + eee_enable be moved
up to dsa_slave_adjust_link in a future patch maybe?
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 06/12] net: dsa: b53: Move Broadcom header setup to b53
From: Vivien Didelot @ 2017-09-18 22:14 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-7-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> The code to enable Broadcom tags/headers is largely switch independent,
> and in preparation for enabling it for multiple devices with b53, move
> the code we have in bcm_sf2.c to b53_common.c
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH net-next 05/12] net: dsa: b53: Use a macro to define I/O operations
From: Vivien Didelot @ 2017-09-18 22:12 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-6-f.fainelli@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> Instead of repeating the same pattern: acquire mutex, read/write, release
> mutex, define a macro: b53_build_op() which takes the type (read|write), I/O
> size, and value (scalar or pointer). This helps with fixing bugs that could
> exit (e.g: missing barrier, lock etc.).
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH 2/3] selftests: actually run the various net selftests
From: Shuah Khan @ 2017-09-18 22:14 UTC (permalink / raw)
To: josef
Cc: Josef Bacik, David S. Miller, linux-kernel, linux-kselftest,
netdev, Shuah Khan, Shuah Khan
In-Reply-To: <1505755982-7855-2-git-send-email-jbacik@fb.com>
On 09/18/2017 11:32 AM, josef@toxicpanda.com wrote:
> From: Josef Bacik <jbacik@fb.com>
>
> These self tests are just self contained binaries, they are not run by
> any of the scripts in the directory. This means they need to be marked
> with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> tools/testing/selftests/net/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index 3df542c84610..45a4e77a47c4 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -6,8 +6,8 @@ CFLAGS += -I../../../../usr/include/
> TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
> TEST_GEN_FILES = socket
> TEST_GEN_FILES += psock_fanout psock_tpacket
> -TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> -TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
> +TEST_GEN_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> +TEST_GEN_PROGS += reuseport_dualstack msg_zerocopy reuseaddr_conflict
Hmm. I see msg_zerocopy.sh for running msg_zerocopy. msg_zerocopy should
still stay in TEST_GEN_FILES and msg_zerocopy.sh needs to be added to
TEST_PROGS so it runs.
thanks,
-- Shuah
^ permalink raw reply
* Re: RACK not getting disabled
From: Yuchung Cheng @ 2017-09-18 22:05 UTC (permalink / raw)
To: hiren panchasara; +Cc: Eric Dumazet, netdev
In-Reply-To: <20170918215522.GE28186@strugglingcoder.info>
On Mon, Sep 18, 2017 at 2:55 PM, hiren panchasara
<hiren@strugglingcoder.info> wrote:
> On 09/18/17 at 02:46P, Yuchung Cheng wrote:
>> On Mon, Sep 18, 2017 at 2:29 PM, hiren panchasara
>> <hiren@strugglingcoder.info> wrote:
>> > On 09/18/17 at 02:18P, Eric Dumazet wrote:
>> >> On Mon, 2017-09-18 at 13:14 -0700, hiren panchasara wrote:
>> >> > Hi all, I am trying to disable rack to see 3dupacks in action during
>> >> > loss-detection but based on the pcap, I see that it's still trigger
>> >> > loss-recovery on the first SACK (as if RACK is still enabled/active).
>> just to be clear: 3-dupack (aka RFC3517) is still enabled with RACK
>> enabled. I am experimenting a patch set to disable 3-dupack approach
>> completely.
>
> So any incoming packet undergoes both checks right now to decide whether
> to mark it lost based on 3-dupacks (and eventually rfc6675) and also
> rack? Any insights into how they are working together would be great.
>
> Also whichever scheme detects loss first can kick connection into
> loss-recovery, right?
Yes. essentially we run both algorithms. the recovery starts when any
packet is deemed lost
>
> Thanks for the clarification, Yuchung.
>
> Cheers,
> Hiren
^ permalink raw reply
* Re: [PATCH net-next 04/12] net: dsa: bcm_sf2: Defer port enabling to calling port_enable
From: Vivien Didelot @ 2017-09-18 21:54 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-5-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> There is no need to configure the enabled ports once in bcm_sf2_sw_setup() and
> then a second time around when dsa_switch_ops::port_enable is called, just do
> it when port_enable is called which is better in terms of power consumption and
> correctness.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH net-next 03/12] net: dsa: b53: Defer port enabling to calling port_enable
From: Vivien Didelot @ 2017-09-18 21:53 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-4-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> There is no need to configure the enabled ports once in b53_setup() and then a
> second time around when dsa_switch_ops::port_enable is called, just do it when
> port_enable is called which is better in terms of power consumption and
> correctness.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Great, my next step is to move up the ports enabling/disabling withing
DSA core. This patch helps going in that direction, thanks.
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ permalink raw reply
* Re: [PATCH v2] net/ethernet/freescale: fix warning for ucc_geth
From: David Miller @ 2017-09-18 21:56 UTC (permalink / raw)
To: valentin.longchamp; +Cc: leoli, netdev, christophe.leroy, linuxppc-dev
In-Reply-To: <20170915055847.29716-1-valentin.longchamp@keymile.com>
From: Valentin Longchamp <valentin.longchamp@keymile.com>
Date: Fri, 15 Sep 2017 07:58:47 +0200
> uf_info.regs is resource_size_t i.e. phys_addr_t that can be either u32
> or u64 according to CONFIG_PHYS_ADDR_T_64BIT.
>
> The printk format is thus adaptet to u64 and the regs value cast to u64
> to take both u32 and u64 into account.
>
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
Applied to net-next, thanks.
^ permalink raw reply
* Re: RACK not getting disabled
From: hiren panchasara @ 2017-09-18 21:55 UTC (permalink / raw)
To: Yuchung Cheng; +Cc: Eric Dumazet, netdev
In-Reply-To: <CAK6E8=eu+1ywBfpAM34seZDKVc-ep5HBuz9fQRuzoaX9BWqeJA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 985 bytes --]
On 09/18/17 at 02:46P, Yuchung Cheng wrote:
> On Mon, Sep 18, 2017 at 2:29 PM, hiren panchasara
> <hiren@strugglingcoder.info> wrote:
> > On 09/18/17 at 02:18P, Eric Dumazet wrote:
> >> On Mon, 2017-09-18 at 13:14 -0700, hiren panchasara wrote:
> >> > Hi all, I am trying to disable rack to see 3dupacks in action during
> >> > loss-detection but based on the pcap, I see that it's still trigger
> >> > loss-recovery on the first SACK (as if RACK is still enabled/active).
> just to be clear: 3-dupack (aka RFC3517) is still enabled with RACK
> enabled. I am experimenting a patch set to disable 3-dupack approach
> completely.
So any incoming packet undergoes both checks right now to decide whether
to mark it lost based on 3-dupacks (and eventually rfc6675) and also
rack? Any insights into how they are working together would be great.
Also whichever scheme detects loss first can kick connection into
loss-recovery, right?
Thanks for the clarification, Yuchung.
Cheers,
Hiren
[-- Attachment #2: Type: application/pgp-signature, Size: 603 bytes --]
^ permalink raw reply
* Re: [PATCH 1/1] ipv6_skip_exthdr: use ipv6_authlen for AH header length computation
From: David Miller @ 2017-09-18 21:53 UTC (permalink / raw)
To: qasdfgtyuiop; +Cc: trivial, netdev, kuznet, yoshfuji
In-Reply-To: <CAMtaSwRk0ne+kG3ejH6685N=ByEAVOkvtyCbmPu9jTng3h2YNQ@mail.gmail.com>
From: Xiang Gao <qasdfgtyuiop@gmail.com>
Date: Fri, 15 Sep 2017 01:04:27 -0400
> From 09cf2e3cf09cf591283785aaa8159baf39ac2e08 Mon Sep 17 00:00:00 2001
> From: Xiang Gao <qasdfgtyuiop@gmail.com>
> Date: Fri, 15 Sep 2017 00:44:12 -0400
> Subject: [PATCH] ipv6_skip_exthdr: use ipv6_authlen for AH hdrlen
>
> In ipv6_skip_exthdr, the lengh of AH header is computed manually
> as (hp->hdrlen+2)<<2. However, in include/linux/ipv6.h, a macro
> named ipv6_authlen is already defined for exactly the same job. This
> commit replaces the manual computation code with the macro.
Your patch was whitespace corrupted by your email client.
^ permalink raw reply
* Re: [PATCH net] ip6_gre: skb_push ipv6hdr before packing the header in ip6gre_header
From: David Miller @ 2017-09-18 21:52 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, xeb
In-Reply-To: <4337e23831098961437503efa2c9d7adfce96b81.1505448007.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 15 Sep 2017 12:00:07 +0800
> Now in ip6gre_header before packing the ipv6 header, it skb_push t->hlen
> which only includes encap_hlen + tun_hlen. It means greh and inner header
> would be over written by ipv6 stuff and ipv6h might have no chance to set
> up.
>
> Jianlin found this issue when using remote any on ip6_gre, the packets he
> captured on gre dev are truncated:
>
> 22:50:26.210866 Out ethertype IPv6 (0x86dd), length 120: truncated-ip6 -\
> 8128 bytes missing!(flowlabel 0x92f40, hlim 0, next-header Options (0) \
> payload length: 8192) ::1:2000:0 > ::1:0:86dd: HBH [trunc] ip-proto-128 \
> 8184
>
> It should also skb_push ipv6hdr so that ipv6h points to the right position
> to set ipv6 stuff up.
>
> This patch is to skb_push hlen + sizeof(*ipv6h) and also fix some indents
> in ip6gre_header.
>
> Fixes: c12b395a4664 ("gre: Support GRE over IPv6")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Yes, this is consistent with how t->hlen is used in the rest of this
tunnel driver.
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: [PATCH net-next 02/12] net: dsa: b53: Make b53_enable_cpu_port() take a port argument
From: Vivien Didelot @ 2017-09-18 21:46 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, andrew, Florian Fainelli
In-Reply-To: <20170918214128.27896-3-f.fainelli@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> In preparation for future changes allowing the configuring of multiple
> CPU ports, make b53_enable_cpu_port() take a port argument.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks,
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
^ 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