* 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 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 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
* [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
* [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 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 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 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
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Vinicius Costa Gomes @ 2017-09-18 23:06 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, jhs, xiyou.wangcong, jiri, intel-wired-lan, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong
In-Reply-To: <20170918080214.yrejz67wwnp2pjzf@localhost>
Hi Richard,
Richard Cochran <richardcochran@gmail.com> writes:
> On Thu, Aug 31, 2017 at 06:26:20PM -0700, Vinicius Costa Gomes wrote:
>> * Time-aware shaper (802.1Qbv):
>
> I just posted a working alternative showing how to handle 802.1Qbv and
> many other Ethernet field buses.
>
>> The idea we are currently exploring is to add a "time-aware", priority based
>> qdisc, that also exposes the Tx queues available and provides a mechanism for
>> mapping priority <-> traffic class <-> Tx queues in a similar fashion as
>> mqprio. We are calling this qdisc 'taprio', and its 'tc' cmd line would be:
>>
>> $ $ tc qdisc add dev ens4 parent root handle 100 taprio num_tc 4 \
>> map 2 2 1 0 3 3 3 3 3 3 3 3 3 3 3 3 \
>> queues 0 1 2 3 \
>> sched-file gates.sched [base-time <interval>] \
>> [cycle-time <interval>] [extension-time <interval>]
>>
>> <file> is multi-line, with each line being of the following format:
>> <cmd> <gate mask> <interval in nanoseconds>
>>
>> Qbv only defines one <cmd>: "S" for 'SetGates'
>>
>> For example:
>>
>> S 0x01 300
>> S 0x03 500
>>
>> This means that there are two intervals, the first will have the gate
>> for traffic class 0 open for 300 nanoseconds, the second will have
>> both traffic classes open for 500 nanoseconds.
>
> The idea of the schedule file will not work in practice. Consider the
> fact that the application wants to deliver time critical data in a
> particular slot. How can it find out a) what the time slots are and
> b) when the next slot is scheduled? With this Qdisc, it cannot do
> this, AFAICT. The admin might delete the file after configuring the
> Qdisc!
That's the point, the application does not need to know that, and asking
that would be stupid. From the point of view of the Qbv specification,
applications only need to care about its basic bandwidth requirements:
its interval, frame size, frames per interval (using the terms of the
SRP section of 802.1Q). The traffic schedule is provided (off band) by a
"god box" which knows all the requirements of all applications in all
the nodes and how they are connected.
(And that's another nice point of how 802.1Qbv works, applications do
not need to be changed to use it, and I think we should work to achieve
this on the Linux side)
That being said, that only works for kinds of traffic that maps well to
this configuration in advance model, which is the model that the IEEE
(see 802.1Qcc) and the AVNU Alliance[1] are pushing for.
In the real world, I can see multiple types of applications, some using
something like TXTIME, and some configured in advance.
>
> Using the SO_TXTIME option, the application has total control over the
> scheduling. The great advantages of this approach is that we can
> support any possible combination of periodic or aperiodic scheduling
> and we can support any priority scheme user space dreams up.
It has the disavantage of that the scheduling information has to be
in-band with the data. I *really* think that for scheduled traffic,
there should be a clear separation, we should not mix the dataflow with
scheduling. In short, an application in the network don't need to have
all the information necessary to schedule its own traffic well.
I have two points here: 1. I see both "solutions" (taprio and SO_TXTIME)
as being ortoghonal and useful, both; 2. trying to make one do the job
of the other, however, looks like "If all I have is a hammer, everything
looks like a nail".
In short, I see a per-packet transmission time and a per-queue schedule
as solutions to different problems.
>
> For example, one can imaging running two or more loops that only
> occasionally collide. When they do collide, which packet should be
> sent first? Just let user space decide.
>
> Thanks,
> Richard
Cheers,
^ permalink raw reply
* Re: [PATCH 16/16] thunderbolt: Add support for networking over Thunderbolt cable
From: Andrew Lunn @ 2017-09-18 23:21 UTC (permalink / raw)
To: Mika Westerberg
Cc: Greg Kroah-Hartman, David S . Miller, Andreas Noever,
Michael Jamet, Yehezkel Bernat, Amir Levy, Mario.Limonciello,
Lukas Wunner, Andy Shevchenko, linux-kernel, netdev
In-Reply-To: <20170918153049.44185-17-mika.westerberg@linux.intel.com>
On Mon, Sep 18, 2017 at 06:30:49PM +0300, Mika Westerberg wrote:
> From: Amir Levy <amir.jer.levy@intel.com>
>
> ThunderboltIP is a protocol created by Apple to tunnel IP/ethernet
> traffic over a Thunderbolt cable. The protocol consists of configuration
> phase where each side sends ThunderboltIP login packets (the protocol is
> determined by UUID in the XDomain packet header) over the configuration
> channel. Once both sides get positive acknowledgment to their login
> packet, they configure high-speed DMA path accordingly. This DMA path is
> then used to transmit and receive networking traffic.
>
> This patch creates a virtual ethernet interface the host software can
> use in the same way as any other networking interface. Once the
> interface is brought up successfully network packets get tunneled over
> the Thunderbolt cable to the remote host and back.
>
> The connection is terminated by sending a ThunderboltIP logout packet
> over the configuration channel. We do this when the network interface is
> brought down by user or the driver is unloaded.
>
> Signed-off-by: Amir Levy <amir.jer.levy@intel.com>
> Signed-off-by: Michael Jamet <michael.jamet@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
> ---
> Documentation/admin-guide/thunderbolt.rst | 24 +
> drivers/thunderbolt/Kconfig | 12 +
> drivers/thunderbolt/Makefile | 3 +
> drivers/thunderbolt/net.c | 1392 +++++++++++++++++++++++++++++
> 4 files changed, 1431 insertions(+)
> create mode 100644 drivers/thunderbolt/net.c
Hi Mika
Could this be renamed to driver/net/thunderbolt.c?
At minimum, it needs a MAINTAINER entry pointing to netdev, so patches
get reviewed by netdev people. However, since the driver seems to be a
lot more netdev than thunderbolt, placing it in driver/net could be
better.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net-next 00/12] net: dsa: b53/bcm_sf2 cleanups
From: David Miller @ 2017-09-18 23:24 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
In-Reply-To: <24876029-5ebb-075e-3f02-50cf0856474f@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 18 Sep 2017 14:46:48 -0700
> On 09/18/2017 02:41 PM, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch series is a first pass set of clean-ups to reduce the number of LOCs
>> between b53 and bcm_sf2 and sharing as many functions as possible.
>>
>> There is a number of additional cleanups queued up locally that require more
>> thorough testing.
>
> David, I just spotted a missing EXPORT_SYMBOL() in patch 8 that was not
> flagged since I had temporarily disabled modular build, I will resubmit
> this shortly after checking the other patches too. Thanks!
Ok.
^ permalink raw reply
* [PATCH net] net: systemport: Fix 64-bit statistics dependency
From: Florian Fainelli @ 2017-09-18 23:31 UTC (permalink / raw)
To: netdev; +Cc: edumazet, davem, jqiaoulk, kiki-good, Florian Fainelli
There are several problems with commit 10377ba7673d ("net: systemport:
Support 64bit statistics", first one got fixed in 7095c973453e ("net:
systemport: Fix 64-bit stats deadlock").
The second problem is that this specific code updates the
stats64.tx_{packets,bytes} from ndo_get_stats64() and that is what we
are returning to ethtool -S. If we are not running a tool that involves
calling ndo_get_stats64(), then we won't get updated ethtool stats.
The solution to this is to update the stats from both call sites,
factoring that into a specific function, While at it, don't just check
the sizeof() but also the type of the statistics in order to use the
64-bit stats seqlock.
Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics"
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/broadcom/bcmsysport.c | 52 ++++++++++++++++++------------
1 file changed, 32 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index c3c53f6cd9e6..83eec9a8c275 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -432,6 +432,27 @@ static void bcm_sysport_update_mib_counters(struct bcm_sysport_priv *priv)
netif_dbg(priv, hw, priv->netdev, "updated MIB counters\n");
}
+static void bcm_sysport_update_tx_stats(struct bcm_sysport_priv *priv,
+ u64 *tx_bytes, u64 *tx_packets)
+{
+ struct bcm_sysport_tx_ring *ring;
+ u64 bytes = 0, packets = 0;
+ unsigned int start;
+ unsigned int q;
+
+ for (q = 0; q < priv->netdev->num_tx_queues; q++) {
+ ring = &priv->tx_rings[q];
+ do {
+ start = u64_stats_fetch_begin_irq(&priv->syncp);
+ bytes = ring->bytes;
+ packets = ring->packets;
+ } while (u64_stats_fetch_retry_irq(&priv->syncp, start));
+
+ *tx_bytes += bytes;
+ *tx_packets += packets;
+ }
+}
+
static void bcm_sysport_get_stats(struct net_device *dev,
struct ethtool_stats *stats, u64 *data)
{
@@ -439,11 +460,16 @@ static void bcm_sysport_get_stats(struct net_device *dev,
struct bcm_sysport_stats64 *stats64 = &priv->stats64;
struct u64_stats_sync *syncp = &priv->syncp;
struct bcm_sysport_tx_ring *ring;
+ u64 tx_bytes = 0, tx_packets = 0;
unsigned int start;
int i, j;
- if (netif_running(dev))
+ if (netif_running(dev)) {
bcm_sysport_update_mib_counters(priv);
+ bcm_sysport_update_tx_stats(priv, &tx_bytes, &tx_packets);
+ stats64->tx_bytes = tx_bytes;
+ stats64->tx_packets = tx_packets;
+ }
for (i = 0, j = 0; i < BCM_SYSPORT_STATS_LEN; i++) {
const struct bcm_sysport_stats *s;
@@ -461,12 +487,13 @@ static void bcm_sysport_get_stats(struct net_device *dev,
continue;
p += s->stat_offset;
- if (s->stat_sizeof == sizeof(u64))
+ if (s->stat_sizeof == sizeof(u64) &&
+ s->type == BCM_SYSPORT_STAT_NETDEV64) {
do {
start = u64_stats_fetch_begin_irq(syncp);
data[i] = *(u64 *)p;
} while (u64_stats_fetch_retry_irq(syncp, start));
- else
+ } else
data[i] = *(u32 *)p;
j++;
}
@@ -1716,27 +1743,12 @@ static void bcm_sysport_get_stats64(struct net_device *dev,
{
struct bcm_sysport_priv *priv = netdev_priv(dev);
struct bcm_sysport_stats64 *stats64 = &priv->stats64;
- struct bcm_sysport_tx_ring *ring;
- u64 tx_packets = 0, tx_bytes = 0;
unsigned int start;
- unsigned int q;
netdev_stats_to_stats64(stats, &dev->stats);
- for (q = 0; q < dev->num_tx_queues; q++) {
- ring = &priv->tx_rings[q];
- do {
- start = u64_stats_fetch_begin_irq(&priv->syncp);
- tx_bytes = ring->bytes;
- tx_packets = ring->packets;
- } while (u64_stats_fetch_retry_irq(&priv->syncp, start));
-
- stats->tx_bytes += tx_bytes;
- stats->tx_packets += tx_packets;
- }
-
- stats64->tx_bytes = stats->tx_bytes;
- stats64->tx_packets = stats->tx_packets;
+ bcm_sysport_update_tx_stats(priv, &stats->tx_bytes,
+ &stats->tx_packets);
do {
start = u64_stats_fetch_begin_irq(&priv->syncp);
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net] net: phy: Fix mask value write on gmii2rgmii converter speed register
From: David Miller @ 2017-09-18 23:34 UTC (permalink / raw)
To: fahad.kunnathadi
Cc: f.fainelli, michal.simek, andrew, appanad, soren.brinkmann,
netdev, linux-kernel
In-Reply-To: <1505457118-3933-1-git-send-email-fahad.kunnathadi@dexceldesigns.com>
From: Fahad Kunnathadi <fahad.kunnathadi@dexceldesigns.com>
Date: Fri, 15 Sep 2017 12:01:58 +0530
> To clear Speed Selection in MDIO control register(0x10),
> ie, clear bits 6 and 13 to zero while keeping other bits same.
> Before AND operation,The Mask value has to be perform with bitwise NOT
> operation (ie, ~ operator)
>
> This patch clears current speed selection before writing the
> new speed settings to gmii2rgmii converter
>
> Fixes: f411a6160bd4 ("net: phy: Add gmiitorgmii converter support")
>
> Signed-off-by: Fahad Kunnathadi <fahad.kunnathadi@dexceldesigns.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel: do not allow loading ip6_tunnel if ipv6 is disabled in cmdline
From: David Miller @ 2017-09-18 23:35 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, xeb, jbenc
In-Reply-To: <107ac334855fea1e6492b2e523b788209e54ec2f.1505462312.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 15 Sep 2017 15:58:33 +0800
> If ipv6 has been disabled from cmdline since kernel started, it makes
> no sense to allow users to create any ip6 tunnel. Otherwise, it could
> some potential problem.
>
> Jianlin found a kernel crash caused by this in ip6_gre when he set
> ipv6.disable=1 in grub:
>
> [ 209.588865] Unable to handle kernel paging request for data at address 0x00000080
> [ 209.588872] Faulting instruction address: 0xc000000000a3aa6c
> [ 209.588879] Oops: Kernel access of bad area, sig: 11 [#1]
> [ 209.589062] NIP [c000000000a3aa6c] fib_rules_lookup+0x4c/0x260
> [ 209.589071] LR [c000000000b9ad90] fib6_rule_lookup+0x50/0xb0
> [ 209.589076] Call Trace:
> [ 209.589097] fib6_rule_lookup+0x50/0xb0
> [ 209.589106] rt6_lookup+0xc4/0x110
> [ 209.589116] ip6gre_tnl_link_config+0x214/0x2f0 [ip6_gre]
> [ 209.589125] ip6gre_newlink+0x138/0x3a0 [ip6_gre]
> [ 209.589134] rtnl_newlink+0x798/0xb80
> [ 209.589142] rtnetlink_rcv_msg+0xec/0x390
> [ 209.589151] netlink_rcv_skb+0x138/0x150
> [ 209.589159] rtnetlink_rcv+0x48/0x70
> [ 209.589169] netlink_unicast+0x538/0x640
> [ 209.589175] netlink_sendmsg+0x40c/0x480
> [ 209.589184] ___sys_sendmsg+0x384/0x4e0
> [ 209.589194] SyS_sendmsg+0xd4/0x140
> [ 209.589201] SyS_socketcall+0x3e0/0x4f0
> [ 209.589209] system_call+0x38/0xe0
>
> This patch is to return -EOPNOTSUPP in ip6_tunnel_init if ipv6 has been
> disabled from cmdline.
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] net/sched: cls_matchall: fix crash when used with classful qdisc
From: David Miller @ 2017-09-18 23:38 UTC (permalink / raw)
To: dcaratti; +Cc: jiri, jhs, jbenc, netdev
In-Reply-To: <b930159de5531a4d216a1cd2c2ef03aa41f421f9.1505562794.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Sat, 16 Sep 2017 14:02:21 +0200
> this script, edited from Linux Advanced Routing and Traffic Control guide
>
> tc q a dev en0 root handle 1: htb default a
> tc c a dev en0 parent 1: classid 1:1 htb rate 6mbit burst 15k
> tc c a dev en0 parent 1:1 classid 1:a htb rate 5mbit ceil 6mbit burst 15k
> tc c a dev en0 parent 1:1 classid 1:b htb rate 1mbit ceil 6mbit burst 15k
> tc f a dev en0 parent 1:0 prio 1 $clsname $clsargs classid 1:b
> ping $address -c1
> tc -s c s dev en0
>
> classifies traffic to 1:b or 1:a, depending on whether the packet matches
> or not the pattern $clsargs of filter $clsname. However, when $clsname is
> 'matchall', a systematic crash can be observed in htb_classify(). HTB and
> classful qdiscs don't assign initial value to struct tcf_result, but then
> they expect it to contain valid values after filters have been run. Thus,
> current 'matchall' ignores the TCA_MATCHALL_CLASSID attribute, configured
> by user, and makes HTB (and classful qdiscs) dereference random pointers.
>
> By assigning head->res to *res in mall_classify(), before the actions are
> invoked, we fix this crash and enable TCA_MATCHALL_CLASSID functionality,
> that had no effect on 'matchall' classifier since its first introduction.
>
> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1460213
> Reported-by: Jiri Benc <jbenc@redhat.com>
> Fixes: b87f7936a932 ("net/sched: introduce Match-all classifier")
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* [PATCH net] bpf: one perf event close won't free bpf program attached by another perf event
From: Yonghong Song @ 2017-09-18 23:38 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
This patch fixes a bug exhibited by the following scenario:
1. fd1 = perf_event_open with attr.config = ID1
2. attach bpf program prog1 to fd1
3. fd2 = perf_event_open with attr.config = ID1
<this will be successful>
4. user program closes fd2 and prog1 is detached from the tracepoint.
5. user program with fd1 does not work properly as tracepoint
no output any more.
The issue happens at step 4. Multiple perf_event_open can be called
successfully, but only one bpf prog pointer in the tp_event. In the
current logic, any fd release for the same tp_event will free
the tp_event->prog.
The fix is to free tp_event->prog only when the closing fd
corresponds to the one which registered the program.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
Additional context: discussed with Alexei internally but did not find
a solution which can avoid introducing the additional field in
trace_event_call structure.
Peter, could you take a look as well and maybe you could have better
alternative? Thanks!
include/linux/trace_events.h | 1 +
kernel/events/core.c | 3 ++-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 7f11050..2e0f222 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -272,6 +272,7 @@ struct trace_event_call {
int perf_refcount;
struct hlist_head __percpu *perf_events;
struct bpf_prog *prog;
+ struct perf_event *bpf_prog_owner;
int (*perf_perm)(struct trace_event_call *,
struct perf_event *);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3e691b7..6bc21e2 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8171,6 +8171,7 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
}
}
event->tp_event->prog = prog;
+ event->tp_event->bpf_prog_owner = event;
return 0;
}
@@ -8185,7 +8186,7 @@ static void perf_event_free_bpf_prog(struct perf_event *event)
return;
prog = event->tp_event->prog;
- if (prog) {
+ if (prog && event->tp_event->bpf_prog_owner == event) {
event->tp_event->prog = NULL;
bpf_prog_put(prog);
}
--
2.9.5
^ permalink raw reply related
* Re: [PATCH] Documentation: networking: fix ASCII art in switchdev.txt
From: David Miller @ 2017-09-18 23:39 UTC (permalink / raw)
To: rdunlap; +Cc: netdev, pavel, andrew, schwab, jiri, sfeldma
In-Reply-To: <984d9d24-b558-6308-3252-1c8ddd5eb7c8@infradead.org>
From: Randy Dunlap <rdunlap@infradead.org>
Date: Sat, 16 Sep 2017 13:10:06 -0700
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix ASCII art in Documentation/networking/switchdev.txt:
>
> Change non-ASCII "spaces" to ASCII spaces.
>
> Change 2 erroneous '+' characters in ASCII art to '-' (at the '*'
> characters below):
>
> line 32:
> +--+----+----+----+-*--+----+---+ +-----+-----+
> line 41:
> +--------------+---*------------+
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Applied, thanks Randy.
^ permalink raw reply
* Re: [PATCH] hamradio: baycom: use new parport device model
From: David Miller @ 2017-09-18 23:40 UTC (permalink / raw)
To: sudipm.mukherjee; +Cc: t.sailer, linux-kernel, linux-hams, netdev
In-Reply-To: <1505648780-4385-1-git-send-email-sudipm.mukherjee@gmail.com>
From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Date: Sun, 17 Sep 2017 12:46:20 +0100
> Modify baycom driver to use the new parallel port device model.
>
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Applied to net-next, thanks.
^ permalink raw reply
* RE: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Tristram.Ha @ 2017-09-18 23:44 UTC (permalink / raw)
To: andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, f.fainelli,
netdev, linux-kernel, Woojung.Huh
In-Reply-To: <20170918225142.GC29615@lunn.ch>
> > 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 logic of the switch driver is if the membership of the port receiving
the frame contains other ports--not counting cpu port--the bit
offload_fwd_mark is set. In RSTP closing the blocked port is generally good
enough, but there are exceptions, so the port is removed from the
membership of other forwarding ports. A disabled port will have its
membership completely reset so it cannot receive anything. It does not
matter much in RSTP as the software bridge should know whether to forward
the frame or not.
We are back to square one. Is there any plan to add this offload_fwd_mark
support to DSA driver so that it can be reported properly? It can be set all the
time, except during port initialization or before bridge creation the forwarding
state does not reflect reality.
If not the port membership can be fixed and there is no internal switch
forwarding, leaving everything handled by the software bridge.
^ permalink raw reply
* Re: [PATCH RFC 6/6] Modify tag_ksz.c to support other KSZ switch drivers
From: Florian Fainelli @ 2017-09-18 23:48 UTC (permalink / raw)
To: Tristram.Ha, andrew
Cc: muvarov, pavel, nathan.leigh.conrad, vivien.didelot, netdev,
linux-kernel, Woojung.Huh
In-Reply-To: <93AF473E2DA327428DE3D46B72B1E9FD41124E0C@CHN-SV-EXMX02.mchp-main.com>
On 09/18/2017 04:44 PM, Tristram.Ha@microchip.com wrote:
>>> 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 logic of the switch driver is if the membership of the port receiving
> the frame contains other ports--not counting cpu port--the bit
> offload_fwd_mark is set. In RSTP closing the blocked port is generally good
> enough, but there are exceptions, so the port is removed from the
> membership of other forwarding ports. A disabled port will have its
> membership completely reset so it cannot receive anything. It does not
> matter much in RSTP as the software bridge should know whether to forward
> the frame or not.
>
> We are back to square one. Is there any plan to add this offload_fwd_mark
> support to DSA driver so that it can be reported properly? It can be set all the
> time, except during port initialization or before bridge creation the forwarding
> state does not reflect reality.
>
> If not the port membership can be fixed and there is no internal switch
> forwarding, leaving everything handled by the software bridge.
I am not really sure why this is such a concern for you so soon when
your driver is not even included yet. You should really aim for baby
steps here: get the basic driver(s) included, with a limited set of
features, and gradually add more features to the driver. When
fwd_offload_mark and RSTP become a real problem, we can most
definitively find a way to fix those in DSA and depending drivers.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v2 0/7] korina: performance fixes and cleanup
From: David Miller @ 2017-09-18 23:50 UTC (permalink / raw)
To: leroi.lists; +Cc: netdev
In-Reply-To: <20170917172353.32225-1-roman@advem.lv>
From: Roman Yeryomin <leroi.lists@gmail.com>
Date: Sun, 17 Sep 2017 20:23:53 +0300
> Changes from v1:
> - use GRO instead of increasing ring size
> - use NAPI_POLL_WEIGHT instead of defining own NAPI_WEIGHT
> - optimize rx descriptor flags processing
Series applied, thank you.
^ permalink raw reply
* Re: [net PATCH] bnxt_en: check for ingress qdisc in flower offload
From: David Miller @ 2017-09-18 23:52 UTC (permalink / raw)
To: sathya.perla; +Cc: netdev, michael.chan
In-Reply-To: <1505734537-6695-1-git-send-email-sathya.perla@broadcom.com>
From: Sathya Perla <sathya.perla@broadcom.com>
Date: Mon, 18 Sep 2017 17:05:37 +0530
> Check for ingress-only qdisc for flower offload, as other qdiscs
> are not supported for flower offload.
>
> Suggested-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Sathya Perla <sathya.perla@broadcom.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net_sched: use explicit size of struct tcmsg, remove need to declare tcm
From: David Miller @ 2017-09-18 23:52 UTC (permalink / raw)
To: colin.king
Cc: jhs, xiyou.wangcong, jiri, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20170918114038.29741-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Mon, 18 Sep 2017 12:40:38 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Pointer tcm is being initialized and is never read, it is only being used
> to determine the size of struct tcmsg. Clean this up by removing
> variable tcm and explicitly using the sizeof struct tcmsg rather than *tcm.
> Cleans up clang warning:
>
> warning: Value stored to 'tcm' during its initialization is never read
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH] bpf: devmap: pass on return value of bpf_map_precharge_memlock
From: David Miller @ 2017-09-18 23:53 UTC (permalink / raw)
To: tklauser; +Cc: ast, daniel, john.fastabend, netdev
In-Reply-To: <20170918130346.10833-1-tklauser@distanz.ch>
From: Tobias Klauser <tklauser@distanz.ch>
Date: Mon, 18 Sep 2017 15:03:46 +0200
> If bpf_map_precharge_memlock in dev_map_alloc, -ENOMEM is returned
> regardless of the actual error produced by bpf_map_precharge_memlock.
> Fix it by passing on the error returned by bpf_map_precharge_memlock.
>
> Also return -EINVAL instead of -ENOMEM if the page count overflow check
> fails.
>
> This makes dev_map_alloc match the behavior of other bpf maps' alloc
> functions wrt. return values.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Applied, thank you.
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH 2/5] e1000e: Fix wrong comment related to link detection
From: Brown, Aaron F @ 2017-09-19 0:13 UTC (permalink / raw)
To: Benjamin Poirier, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org, Lennart Sorensen
In-Reply-To: <20170721183627.13373-2-bpoirier@suse.com>
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> Of Benjamin Poirier
> Sent: Friday, July 21, 2017 11:36 AM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>
> Cc: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org; Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Subject: [Intel-wired-lan] [PATCH 2/5] e1000e: Fix wrong comment related to
> link detection
>
> Reading e1000e_check_for_copper_link() shows that get_link_status is set to
> false after link has been detected. Therefore, it stays TRUE until then.
>
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Tested-by: Aaron Brown <aaron.f.brown@intel.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