Netdev List
 help / color / mirror / Atom feed
* [PATCH v14 0/5] Replace PCI pool by DMA pool API
From: Romain Perier @ 2017-10-23 17:59 UTC (permalink / raw)
  To: Jens Axboe, Andrew Morton, Dan Williams, Vinod Koul, Jeff Kirsher,
	Aviad Krawczyk, Bjorn Helgaas, linux-pci
  Cc: dmaengine, netdev, linux-kernel, Romain Perier

The current PCI pool API are simple macro functions direct expanded to
the appropriate dma pool functions. The prototypes are almost the same
and semantically, they are very similar. I propose to use the DMA pool
API directly and get rid of the old API.

This set of patches, replaces the old API by the dma pool API
and remove the defines.

Changes in v14:
- Rebased series onto next-20171018
- Rebased patch 03/05 on latest driver

Changes in v13:
- Rebased series onto next-20170906
- Added a new commit for the hinic ethernet driver
- Remove previously merged patches

Changes in v12:
- Rebased series onto next-20170822

Changes in v11:
- Rebased series onto next-20170809
- Removed patches 08-14, these have been merged.

Changes in v10:
- Rebased series onto next-20170706
- I have fixed and improved patch "scsi: megaraid: Replace PCI pool old API"

Changes in v9:
- Rebased series onto next-20170522
- I have fixed and improved the patch for lpfc driver

Changes in v8:
- Rebased series onto next-20170428

Changes in v7:
- Rebased series onto next-20170416
- Added Acked-by, Tested-by and Reviwed-by tags

Changes in v6:
- Fixed an issue reported by kbuild test robot about changes in DAC960
- Removed patches 15/19,16/19,17/19,18/19. They have been merged by Greg
- Added Acked-by Tags

Changes in v5:
- Re-worded the cover letter (remove sentence about checkpatch.pl)
- Rebased series onto next-20170308
- Fix typos in commit message
- Added Acked-by Tags

Changes in v4:
- Rebased series onto next-20170301
- Removed patch 20/20: checks done by checkpath.pl, no longer required.
  Thanks to Peter and Joe for their feedbacks.
- Added Reviewed-by tags

Changes in v3:
- Rebased series onto next-20170224
- Fix checkpath.pl reports for patch 11/20 and patch 12/20
- Remove prefix RFC
Changes in v2:
- Introduced patch 18/20
- Fixed cosmetic changes: spaces before brace, live over 80 characters
- Removed some of the check for NULL pointers before calling dma_pool_destroy
- Improved the regexp in checkpatch for pci_pool, thanks to Joe Perches
- Added Tested-by and Acked-by tags

Romain Perier (5):
  block: DAC960: Replace PCI pool old API
  dmaengine: pch_dma: Replace PCI pool old API
  net: e100: Replace PCI pool old API
  hinic: Replace PCI pool old API
  PCI: Remove PCI pool macro functions

 drivers/block/DAC960.c                            | 38 +++++++++++------------
 drivers/block/DAC960.h                            |  4 +--
 drivers/dma/pch_dma.c                             | 12 +++----
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 10 +++---
 drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h |  2 +-
 drivers/net/ethernet/intel/e100.c                 | 12 +++----
 include/linux/pci.h                               |  9 ------
 7 files changed, 38 insertions(+), 49 deletions(-)

-- 
2.14.1

^ permalink raw reply

* [PATCH net-next 2/3] bpf: permit multiple bpf attachments for a single perf event
From: Yonghong Song @ 2017-10-23 17:58 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171023175805.2898366-1-yhs@fb.com>

This patch enables multiple bpf attachments for a
kprobe/uprobe/tracepoint single trace event.
Each trace_event keeps a list of attached perf events.
When an event happens, all attached bpf programs will
be executed based on the order of attachment.

A global bpf_event_mutex lock is introduced to protect
prog_array attaching and detaching. An alternative will
be introduce a mutex lock in every trace_event_call
structure, but it takes a lot of extra memory.
So a global bpf_event_mutex lock is a good compromise.

The bpf prog detachment involves allocation of memory.
If the allocation fails, a dummy do-nothing program
will replace to-be-detached program in-place.

Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 include/linux/bpf.h           | 30 +++++++++++++---
 include/linux/trace_events.h  | 33 ++++++++++++++---
 include/trace/perf.h          |  6 ++--
 kernel/bpf/core.c             | 80 +++++++++++++++++++++++++++++++++++++++++
 kernel/events/core.c          | 26 +++++---------
 kernel/trace/bpf_trace.c      | 83 ++++++++++++++++++++++++++++++++++++++++---
 kernel/trace/trace_kprobe.c   |  6 ++--
 kernel/trace/trace_syscalls.c | 32 +++++++++--------
 kernel/trace/trace_uprobe.c   |  3 +-
 9 files changed, 244 insertions(+), 55 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1e334b2..0d7e4e8 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -273,18 +273,38 @@ int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
 int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
 				__u32 __user *prog_ids, u32 cnt);
 
-#define BPF_PROG_RUN_ARRAY(array, ctx, func)		\
+void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
+				struct bpf_prog *old_prog);
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+			struct bpf_prog *exclude_prog,
+			struct bpf_prog *include_prog,
+			struct bpf_prog_array **new_array);
+
+#define __BPF_PROG_RUN_ARRAY(array, ctx, func, check_non_null)	\
 	({						\
-		struct bpf_prog **_prog;		\
+		struct bpf_prog **_prog, *__prog;	\
+		struct bpf_prog_array *_array;		\
 		u32 _ret = 1;				\
 		rcu_read_lock();			\
-		_prog = rcu_dereference(array)->progs;	\
-		for (; *_prog; _prog++)			\
-			_ret &= func(*_prog, ctx);	\
+		_array = rcu_dereference(array);	\
+		if (unlikely(check_non_null && !_array))\
+			goto _out;			\
+		_prog = _array->progs;			\
+		while ((__prog = READ_ONCE(*_prog))) {	\
+			_ret &= func(__prog, ctx);	\
+			_prog++;			\
+		}					\
+_out:							\
 		rcu_read_unlock();			\
 		_ret;					\
 	 })
 
+#define BPF_PROG_RUN_ARRAY(array, ctx, func)		\
+	__BPF_PROG_RUN_ARRAY(array, ctx, func, false)
+
+#define BPF_PROG_RUN_ARRAY_SAFE(array, ctx, func)	\
+	__BPF_PROG_RUN_ARRAY(array, ctx, func, true)
+
 #ifdef CONFIG_BPF_SYSCALL
 DECLARE_PER_CPU(int, bpf_prog_active);
 
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 2e0f222..c811464 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -271,14 +271,37 @@ struct trace_event_call {
 #ifdef CONFIG_PERF_EVENTS
 	int				perf_refcount;
 	struct hlist_head __percpu	*perf_events;
-	struct bpf_prog			*prog;
-	struct perf_event		*bpf_prog_owner;
+	struct bpf_prog_array __rcu	*prog_array;
 
 	int	(*perf_perm)(struct trace_event_call *,
 			     struct perf_event *);
 #endif
 };
 
+#ifdef CONFIG_PERF_EVENTS
+static inline bool bpf_prog_array_valid(struct trace_event_call *call)
+{
+	/*
+	 * This inline function checks whether call->prog_array
+	 * is valid or not. The function is called in various places,
+	 * outside rcu_read_lock/unlock, as a heuristic to speed up execution.
+	 *
+	 * If this function returns true, and later call->prog_array
+	 * becomes false inside rcu_read_lock/unlock region,
+	 * we bail out then. If this function return false,
+	 * there is a risk that we might miss a few events if the checking
+	 * were delayed until inside rcu_read_lock/unlock region and
+	 * call->prog_array happened to become non-NULL then.
+	 *
+	 * Here, READ_ONCE() is used instead of rcu_access_pointer().
+	 * rcu_access_pointer() requires the actual definition of
+	 * "struct bpf_prog_array" while READ_ONCE() only needs
+	 * a declaration of the same type.
+	 */
+	return !!READ_ONCE(call->prog_array);
+}
+#endif
+
 static inline const char *
 trace_event_name(struct trace_event_call *call)
 {
@@ -435,9 +458,9 @@ trace_trigger_soft_disabled(struct trace_event_file *file)
 }
 
 #ifdef CONFIG_BPF_EVENTS
-unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx);
+unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
 #else
-static inline unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
+static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
 {
 	return 1;
 }
@@ -499,6 +522,8 @@ extern void ftrace_profile_free_filter(struct perf_event *event);
 void perf_trace_buf_update(void *record, u16 type);
 void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp);
 
+int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog);
+void perf_event_detach_bpf_prog(struct perf_event *event);
 void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
 			       struct trace_event_call *call, u64 count,
 			       struct pt_regs *regs, struct hlist_head *head,
diff --git a/include/trace/perf.h b/include/trace/perf.h
index 04fe68bb..4ffe57d 100644
--- a/include/trace/perf.h
+++ b/include/trace/perf.h
@@ -34,7 +34,6 @@ perf_trace_##call(void *__data, proto)					\
 	struct trace_event_call *event_call = __data;			\
 	struct trace_event_data_offsets_##call __maybe_unused __data_offsets;\
 	struct trace_event_raw_##call *entry;				\
-	struct bpf_prog *prog = event_call->prog;			\
 	struct pt_regs *__regs;						\
 	u64 __count = 1;						\
 	struct task_struct *__task = NULL;				\
@@ -46,8 +45,9 @@ perf_trace_##call(void *__data, proto)					\
 	__data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
 									\
 	head = this_cpu_ptr(event_call->perf_events);			\
-	if (!prog && __builtin_constant_p(!__task) && !__task &&	\
-				hlist_empty(head))			\
+	if (!bpf_prog_array_valid(event_call) &&			\
+			__builtin_constant_p(!__task) && !__task &&	\
+			hlist_empty(head))				\
 		return;							\
 									\
 	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 8e7c8bf..bac5307 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1394,6 +1394,19 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
 }
 EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
 
+static unsigned int __bpf_prog_ret1(const void *ctx,
+				    const struct bpf_insn *insn)
+{
+	return 1;
+}
+static struct bpf_prog_dummy {
+	struct bpf_prog prog;
+} dummy_bpf_prog = {
+	.prog = {
+		.bpf_func = __bpf_prog_ret1,
+	},
+};
+
 /* to avoid allocating empty bpf_prog_array for cgroups that
  * don't have bpf program attached use one global 'empty_prog_array'
  * It will not be modified the caller of bpf_prog_array_alloc()
@@ -1463,6 +1476,73 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
 	return 0;
 }
 
+void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
+				struct bpf_prog *old_prog)
+{
+	struct bpf_prog **prog = progs->progs;
+
+	for (; *prog; prog++)
+		if (*prog == old_prog) {
+			WRITE_ONCE(*prog, &dummy_bpf_prog.prog);
+			break;
+		}
+}
+
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+			struct bpf_prog *exclude_prog,
+			struct bpf_prog *include_prog,
+			struct bpf_prog_array **new_array)
+{
+	int new_prog_cnt, carry_prog_cnt = 0;
+	struct bpf_prog **existing_prog;
+	struct bpf_prog_array *tarray;
+	int new_prog_idx = 0;
+
+	/* Figure out how many existing progs we need to carry over to
+	 * the new array.
+	 */
+	if (old_array) {
+		existing_prog = old_array->progs;
+		for (; *existing_prog; existing_prog++) {
+			if (*existing_prog != exclude_prog &&
+			    *existing_prog != &dummy_bpf_prog.prog)
+				carry_prog_cnt++;
+			if (*existing_prog == include_prog)
+				return -EEXIST;
+		}
+	}
+
+	/* How many progs (not NULL) will be in the new array? */
+	new_prog_cnt = carry_prog_cnt;
+	if (include_prog)
+		new_prog_cnt += 1;
+
+	/* Do we have any prog (not NULL) in the new array? */
+	if (!new_prog_cnt) {
+		*new_array = NULL;
+		return 0;
+	}
+
+	/* +1 as the end of prog_array is marked with NULL */
+	tarray = bpf_prog_array_alloc(new_prog_cnt + 1, GFP_KERNEL);
+	if (tarray == NULL)
+		return -ENOMEM;
+
+	/* Fill in the new prog array */
+	if (carry_prog_cnt) {
+		existing_prog = old_array->progs;
+		for (; *existing_prog; existing_prog++)
+			if (*existing_prog != exclude_prog &&
+			    *existing_prog != &dummy_bpf_prog.prog)
+				tarray->progs[new_prog_idx++] = *existing_prog;
+	}
+	if (include_prog)
+		tarray->progs[new_prog_idx++] = include_prog;
+	tarray->progs[new_prog_idx] = NULL;
+	*new_array = tarray;
+	return 0;
+}
+
 static void bpf_prog_free_deferred(struct work_struct *work)
 {
 	struct bpf_prog_aux *aux;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9f78a682..9660ee6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7954,11 +7954,9 @@ void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
 			       struct pt_regs *regs, struct hlist_head *head,
 			       struct task_struct *task)
 {
-	struct bpf_prog *prog = call->prog;
-
-	if (prog) {
+	if (bpf_prog_array_valid(call)) {
 		*(struct pt_regs **)raw_data = regs;
-		if (!trace_call_bpf(prog, raw_data) || hlist_empty(head)) {
+		if (!trace_call_bpf(call, raw_data) || hlist_empty(head)) {
 			perf_swevent_put_recursion_context(rctx);
 			return;
 		}
@@ -8147,13 +8145,11 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
 {
 	bool is_kprobe, is_tracepoint, is_syscall_tp;
 	struct bpf_prog *prog;
+	int ret;
 
 	if (event->attr.type != PERF_TYPE_TRACEPOINT)
 		return perf_event_set_bpf_handler(event, prog_fd);
 
-	if (event->tp_event->prog)
-		return -EEXIST;
-
 	is_kprobe = event->tp_event->flags & TRACE_EVENT_FL_UKPROBE;
 	is_tracepoint = event->tp_event->flags & TRACE_EVENT_FL_TRACEPOINT;
 	is_syscall_tp = is_syscall_trace_event(event->tp_event);
@@ -8181,26 +8177,20 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
 			return -EACCES;
 		}
 	}
-	event->tp_event->prog = prog;
-	event->tp_event->bpf_prog_owner = event;
 
-	return 0;
+	ret = perf_event_attach_bpf_prog(event, prog);
+	if (ret)
+		bpf_prog_put(prog);
+	return ret;
 }
 
 static void perf_event_free_bpf_prog(struct perf_event *event)
 {
-	struct bpf_prog *prog;
-
 	if (event->attr.type != PERF_TYPE_TRACEPOINT) {
 		perf_event_free_bpf_handler(event);
 		return;
 	}
-
-	prog = event->tp_event->prog;
-	if (prog && event->tp_event->bpf_prog_owner == event) {
-		event->tp_event->prog = NULL;
-		bpf_prog_put(prog);
-	}
+	perf_event_detach_bpf_prog(event);
 }
 
 #else
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3126da2..2e1c4e0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -17,7 +17,7 @@
 
 /**
  * trace_call_bpf - invoke BPF program
- * @prog: BPF program
+ * @call: tracepoint event
  * @ctx: opaque context pointer
  *
  * kprobe handlers execute BPF programs via this helper.
@@ -29,7 +29,7 @@
  * 1 - store kprobe event into ring buffer
  * Other values are reserved and currently alias to 1
  */
-unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
+unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
 {
 	unsigned int ret;
 
@@ -49,9 +49,22 @@ unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
 		goto out;
 	}
 
-	rcu_read_lock();
-	ret = BPF_PROG_RUN(prog, ctx);
-	rcu_read_unlock();
+	/*
+	 * Instead of moving rcu_read_lock/rcu_dereference/rcu_read_unlock
+	 * to all call sites, we did a bpf_prog_array_valid() there to check
+	 * whether call->prog_array is empty or not, which is
+	 * a heurisitc to speed up execution.
+	 *
+	 * If bpf_prog_array_valid() fetched prog_array was
+	 * non-NULL, we go into trace_call_bpf() and do the actual
+	 * proper rcu_dereference() under RCU lock.
+	 * If it turns out that prog_array is NULL then, we bail out.
+	 * For the opposite, if the bpf_prog_array_valid() fetched pointer
+	 * was NULL, you'll skip the prog_array with the risk of missing
+	 * out of events when it was updated in between this and the
+	 * rcu_dereference() which is accepted risk.
+	 */
+	ret = BPF_PROG_RUN_ARRAY_SAFE(call->prog_array, ctx, BPF_PROG_RUN);
 
  out:
 	__this_cpu_dec(bpf_prog_active);
@@ -741,3 +754,63 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
 
 const struct bpf_prog_ops perf_event_prog_ops = {
 };
+
+static DEFINE_MUTEX(bpf_event_mutex);
+
+int perf_event_attach_bpf_prog(struct perf_event *event,
+			   struct bpf_prog *prog)
+{
+	struct bpf_prog_array __rcu *old_array;
+	struct bpf_prog_array *new_array;
+	int ret;
+
+	mutex_lock(&bpf_event_mutex);
+
+	if (event->prog)
+		return -EEXIST;
+
+	old_array = rcu_dereference_protected(event->tp_event->prog_array,
+					      lockdep_is_held(&bpf_event_mutex));
+	ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
+	if (ret < 0)
+		goto out;
+
+	/* set the new array to event->tp_event and set event->prog */
+	event->prog = prog;
+	rcu_assign_pointer(event->tp_event->prog_array, new_array);
+
+	if (old_array)
+		bpf_prog_array_free(old_array);
+
+out:
+	mutex_unlock(&bpf_event_mutex);
+	return ret;
+}
+
+void perf_event_detach_bpf_prog(struct perf_event *event)
+{
+	struct bpf_prog_array __rcu *old_array;
+	struct bpf_prog_array *new_array;
+	int ret;
+
+	mutex_lock(&bpf_event_mutex);
+
+	if (!event->prog)
+		return;
+
+	old_array = rcu_dereference_protected(event->tp_event->prog_array,
+					      lockdep_is_held(&bpf_event_mutex));
+
+	ret = bpf_prog_array_copy(old_array, event->prog, NULL, &new_array);
+	if (ret < 0) {
+		bpf_prog_array_delete_safe(old_array, event->prog);
+	} else {
+		rcu_assign_pointer(event->tp_event->prog_array, new_array);
+		bpf_prog_array_free(old_array);
+	}
+
+	bpf_prog_put(event->prog);
+	event->prog = NULL;
+
+	mutex_unlock(&bpf_event_mutex);
+}
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8a907e1..abf92e4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1174,13 +1174,12 @@ static void
 kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
 {
 	struct trace_event_call *call = &tk->tp.call;
-	struct bpf_prog *prog = call->prog;
 	struct kprobe_trace_entry_head *entry;
 	struct hlist_head *head;
 	int size, __size, dsize;
 	int rctx;
 
-	if (prog && !trace_call_bpf(prog, regs))
+	if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
 		return;
 
 	head = this_cpu_ptr(call->perf_events);
@@ -1210,13 +1209,12 @@ kretprobe_perf_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
 		    struct pt_regs *regs)
 {
 	struct trace_event_call *call = &tk->tp.call;
-	struct bpf_prog *prog = call->prog;
 	struct kretprobe_trace_entry_head *entry;
 	struct hlist_head *head;
 	int size, __size, dsize;
 	int rctx;
 
-	if (prog && !trace_call_bpf(prog, regs))
+	if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
 		return;
 
 	head = this_cpu_ptr(call->perf_events);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 696afe7..5527030 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -559,9 +559,10 @@ static DECLARE_BITMAP(enabled_perf_exit_syscalls, NR_syscalls);
 static int sys_perf_refcount_enter;
 static int sys_perf_refcount_exit;
 
-static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
+static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *regs,
 			      struct syscall_metadata *sys_data,
-			      struct syscall_trace_enter *rec) {
+			      struct syscall_trace_enter *rec)
+{
 	struct syscall_tp_t {
 		unsigned long long regs;
 		unsigned long syscall_nr;
@@ -573,7 +574,7 @@ static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
 	param.syscall_nr = rec->nr;
 	for (i = 0; i < sys_data->nb_args; i++)
 		param.args[i] = rec->args[i];
-	return trace_call_bpf(prog, &param);
+	return trace_call_bpf(call, &param);
 }
 
 static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
@@ -581,7 +582,7 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	struct syscall_metadata *sys_data;
 	struct syscall_trace_enter *rec;
 	struct hlist_head *head;
-	struct bpf_prog *prog;
+	bool valid_prog_array;
 	int syscall_nr;
 	int rctx;
 	int size;
@@ -596,9 +597,9 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	if (!sys_data)
 		return;
 
-	prog = READ_ONCE(sys_data->enter_event->prog);
 	head = this_cpu_ptr(sys_data->enter_event->perf_events);
-	if (!prog && hlist_empty(head))
+	valid_prog_array = bpf_prog_array_valid(sys_data->enter_event);
+	if (!valid_prog_array && hlist_empty(head))
 		return;
 
 	/* get the size after alignment with the u32 buffer size field */
@@ -614,7 +615,8 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 	syscall_get_arguments(current, regs, 0, sys_data->nb_args,
 			       (unsigned long *)&rec->args);
 
-	if ((prog && !perf_call_bpf_enter(prog, regs, sys_data, rec)) ||
+	if ((valid_prog_array &&
+	     !perf_call_bpf_enter(sys_data->enter_event, regs, sys_data, rec)) ||
 	    hlist_empty(head)) {
 		perf_swevent_put_recursion_context(rctx);
 		return;
@@ -659,8 +661,9 @@ static void perf_sysenter_disable(struct trace_event_call *call)
 	mutex_unlock(&syscall_trace_lock);
 }
 
-static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
-			      struct syscall_trace_exit *rec) {
+static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *regs,
+			      struct syscall_trace_exit *rec)
+{
 	struct syscall_tp_t {
 		unsigned long long regs;
 		unsigned long syscall_nr;
@@ -670,7 +673,7 @@ static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
 	*(struct pt_regs **)&param = regs;
 	param.syscall_nr = rec->nr;
 	param.ret = rec->ret;
-	return trace_call_bpf(prog, &param);
+	return trace_call_bpf(call, &param);
 }
 
 static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
@@ -678,7 +681,7 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
 	struct syscall_metadata *sys_data;
 	struct syscall_trace_exit *rec;
 	struct hlist_head *head;
-	struct bpf_prog *prog;
+	bool valid_prog_array;
 	int syscall_nr;
 	int rctx;
 	int size;
@@ -693,9 +696,9 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
 	if (!sys_data)
 		return;
 
-	prog = READ_ONCE(sys_data->exit_event->prog);
 	head = this_cpu_ptr(sys_data->exit_event->perf_events);
-	if (!prog && hlist_empty(head))
+	valid_prog_array = bpf_prog_array_valid(sys_data->exit_event);
+	if (!valid_prog_array && hlist_empty(head))
 		return;
 
 	/* We can probably do that at build time */
@@ -709,7 +712,8 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
 	rec->nr = syscall_nr;
 	rec->ret = syscall_get_return_value(current, regs);
 
-	if ((prog && !perf_call_bpf_exit(prog, regs, rec)) ||
+	if ((valid_prog_array &&
+	     !perf_call_bpf_exit(sys_data->exit_event, regs, rec)) ||
 	    hlist_empty(head)) {
 		perf_swevent_put_recursion_context(rctx);
 		return;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 4525e02..153c0e4 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1113,13 +1113,12 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
 {
 	struct trace_event_call *call = &tu->tp.call;
 	struct uprobe_trace_entry_head *entry;
-	struct bpf_prog *prog = call->prog;
 	struct hlist_head *head;
 	void *data;
 	int size, esize;
 	int rctx;
 
-	if (prog && !trace_call_bpf(prog, regs))
+	if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
 		return;
 
 	esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next 3/3] bpf: add a test case to test single tp multiple bpf attachment
From: Yonghong Song @ 2017-10-23 17:58 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171023175805.2898366-1-yhs@fb.com>

The bpf sample program syscall_tp is modified to
show attachment of more than bpf programs
for a particular kernel tracepoint.

Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 samples/bpf/syscall_tp_user.c | 66 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 53 insertions(+), 13 deletions(-)

diff --git a/samples/bpf/syscall_tp_user.c b/samples/bpf/syscall_tp_user.c
index a3cb91e..9169d32 100644
--- a/samples/bpf/syscall_tp_user.c
+++ b/samples/bpf/syscall_tp_user.c
@@ -23,6 +23,13 @@
  * This requires kernel CONFIG_FTRACE_SYSCALLS to be set.
  */
 
+static void usage(const char *cmd)
+{
+	printf("USAGE: %s [-i num_progs] [-h]\n", cmd);
+	printf("       -i num_progs      # number of progs of the test\n");
+	printf("       -h                # help\n");
+}
+
 static void verify_map(int map_id)
 {
 	__u32 key = 0;
@@ -32,22 +39,29 @@ static void verify_map(int map_id)
 		fprintf(stderr, "map_lookup failed: %s\n", strerror(errno));
 		return;
 	}
-	if (val == 0)
+	if (val == 0) {
 		fprintf(stderr, "failed: map #%d returns value 0\n", map_id);
+		return;
+	}
+	val = 0;
+	if (bpf_map_update_elem(map_id, &key, &val, BPF_ANY) != 0) {
+		fprintf(stderr, "map_update failed: %s\n", strerror(errno));
+		return;
+	}
 }
 
-int main(int argc, char **argv)
+static int test(char *filename, int num_progs)
 {
-	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
-	char filename[256];
-	int fd;
+	int i, fd, map0_fds[num_progs], map1_fds[num_progs];
 
-	setrlimit(RLIMIT_MEMLOCK, &r);
-	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
-
-	if (load_bpf_file(filename)) {
-		fprintf(stderr, "%s", bpf_log_buf);
-		return 1;
+	for (i = 0; i < num_progs; i++) {
+		if (load_bpf_file(filename)) {
+			fprintf(stderr, "%s", bpf_log_buf);
+			return 1;
+		}
+		printf("prog #%d: map ids %d %d\n", i, map_fd[0], map_fd[1]);
+		map0_fds[i] = map_fd[0];
+		map1_fds[i] = map_fd[1];
 	}
 
 	/* current load_bpf_file has perf_event_open default pid = -1
@@ -64,8 +78,34 @@ int main(int argc, char **argv)
 	close(fd);
 
 	/* verify the map */
-	verify_map(map_fd[0]);
-	verify_map(map_fd[1]);
+	for (i = 0; i < num_progs; i++) {
+		verify_map(map0_fds[i]);
+		verify_map(map1_fds[i]);
+	}
 
 	return 0;
 }
+
+int main(int argc, char **argv)
+{
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	int opt, num_progs = 1;
+	char filename[256];
+
+	while ((opt = getopt(argc, argv, "i:h")) != -1) {
+		switch (opt) {
+		case 'i':
+			num_progs = atoi(optarg);
+			break;
+		case 'h':
+		default:
+			usage(argv[0]);
+			return 0;
+		}
+	}
+
+	setrlimit(RLIMIT_MEMLOCK, &r);
+	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+	return test(filename, num_progs);
+}
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next 0/3] bpf: permit multiple bpf attachments for a single perf tracepoint event
From: Yonghong Song @ 2017-10-23 17:58 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team

This patch set adds support to permit multiple bpf prog attachments
for a single perf tracepoint event. Patch 1 does some cleanup such
that perf_event_{set|free}_bpf_handler is called under the
same condition. Patch 2 has the core implementation, and
Patch 3 adds a test case.

Yonghong Song (3):
  bpf: use the same condition in perf event set/free bpf handler
  bpf: permit multiple bpf attachments for a single perf event
  bpf: add a test case to test single tp multiple bpf attachment

 include/linux/bpf.h           | 30 +++++++++++++---
 include/linux/trace_events.h  | 33 ++++++++++++++---
 include/trace/perf.h          |  6 ++--
 kernel/bpf/core.c             | 80 +++++++++++++++++++++++++++++++++++++++++
 kernel/events/core.c          | 30 ++++++----------
 kernel/trace/bpf_trace.c      | 83 ++++++++++++++++++++++++++++++++++++++++---
 kernel/trace/trace_kprobe.c   |  6 ++--
 kernel/trace/trace_syscalls.c | 32 +++++++++--------
 kernel/trace/trace_uprobe.c   |  3 +-
 samples/bpf/syscall_tp_user.c | 66 +++++++++++++++++++++++++++-------
 10 files changed, 299 insertions(+), 70 deletions(-)

-- 
2.9.5

^ permalink raw reply

* [PATCH net-next 1/3] bpf: use the same condition in perf event set/free bpf handler
From: Yonghong Song @ 2017-10-23 17:58 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171023175805.2898366-1-yhs@fb.com>

This is a cleanup such that doing the same check in
perf_event_free_bpf_prog as we already do in
perf_event_set_bpf_prog step.

Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 kernel/events/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 31ee304..9f78a682 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8191,10 +8191,10 @@ static void perf_event_free_bpf_prog(struct perf_event *event)
 {
 	struct bpf_prog *prog;
 
-	perf_event_free_bpf_handler(event);
-
-	if (!event->tp_event)
+	if (event->attr.type != PERF_TYPE_TRACEPOINT) {
+		perf_event_free_bpf_handler(event);
 		return;
+	}
 
 	prog = event->tp_event->prog;
 	if (prog && event->tp_event->bpf_prog_owner == event) {
-- 
2.9.5

^ permalink raw reply related

* Re: [PATCH net-next 00/12] tools: bpftool: Add JSON output to bpftool
From: Alexei Starovoitov @ 2017-10-23 17:44 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

On Mon, Oct 23, 2017 at 09:24:04AM -0700, Jakub Kicinski wrote:
> Quentin says:
> 
> This series introduces support for JSON output to all bpftool commands. It
> adds option parsing, and several options are created:
> 
>   * -j, --json     Switch to JSON output.
>   * -p, --pretty   Switch to JSON and print it in a human-friendly fashion.
>   * -h, --help     Print generic help message.
>   * -V, --version  Print version number.
> 
> This code uses a "json_writer", which is a copy of the one written by
> Stephen Hemminger in iproute2.
> ---
> I don't know if there is an easy way to share the code for json_write
> without copying the file, so I am very open to suggestions on this matter.

I think copy-paste is fine in this case.
If we ever decide to remove it from here and move it into iproute2
this json.[hc] can be unified, but I think kernel/tools/bpf/ is
a better place since introspection interface is still evolving a lot
and bpftool is not networking specific. Like would be great if it
can show what programs attached to which cgroups, tracepoints, kprobes.

Overall looks like a nice set of improvements.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* [net-next PATCH] bpf: cpumap fix potential lost wake-up problem
From: Jesper Dangaard Brouer @ 2017-10-23 17:39 UTC (permalink / raw)
  To: netdev, Michael S. Tsirkin
  Cc: Rik van Riel, Daniel Borkmann, Alexei Starovoitov,
	Jesper Dangaard Brouer

As pointed out by Michael, commit 1c601d829ab0 ("bpf: cpumap xdp_buff
to skb conversion and allocation") contains a classical example of the
potential lost wake-up problem.

We need to recheck the condition __ptr_ring_empty() after changing
current->state to TASK_INTERRUPTIBLE, this avoids a race between
wake_up_process() and schedule(). After this, a race with
wake_up_process() will simply change the state to TASK_RUNNING, and
the schedule() call not really put us to sleep.

Fixes: 1c601d829ab0 ("bpf: cpumap xdp_buff to skb conversion and allocation")
Reported-by: "Michael S. Tsirkin" <mst@redhat.com>
---
 kernel/bpf/cpumap.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index b4358d84ddf1..86e29cbf7827 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -288,13 +288,17 @@ static int cpu_map_kthread_run(void *data)
 
 		/* Release CPU reschedule checks */
 		if (__ptr_ring_empty(rcpu->queue)) {
-			__set_current_state(TASK_INTERRUPTIBLE);
-			schedule();
-			sched = 1;
+			set_current_state(TASK_INTERRUPTIBLE);
+			/* Recheck to avoid lost wake-up */
+			if (__ptr_ring_empty(rcpu->queue)) {
+				schedule();
+				sched = 1;
+			} else {
+				__set_current_state(TASK_RUNNING);
+			}
 		} else {
 			sched = cond_resched();
 		}
-		__set_current_state(TASK_RUNNING);
 
 		/* Process packets in rcpu->queue */
 		local_bh_disable();

^ permalink raw reply related

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Jesus Sanchez-Palencia @ 2017-10-23 17:18 UTC (permalink / raw)
  To: Richard Cochran
  Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
	intel-wired-lan, andre.guedes, ivan.briano, boon.leong.ong,
	Levi Pearson, Henrik Austad
In-Reply-To: <20171019203949.pyv44i5sy6lnuavh@localhost>

Hi,

On 10/19/2017 01:39 PM, Richard Cochran wrote:
> On Wed, Oct 18, 2017 at 03:37:35PM -0700, Jesus Sanchez-Palencia wrote:
>> I also did some tests with when you don't set valid launch times, but here using
>> your idea from above, so with the driver calculating a valid launch time (i.e.
>> current NIC time + X ns, varying X across tests) for packets that didn't have it
>> set by the user, and I wasn't too happy with its reliability. It could
>> definitely be improved, but it has left me wondering: instead, what about
>> documenting that if you enable TXTIME, then you *must* provide a valid Launch
>> time for all packets on traffic classes that are affected?
> 
> If txtime is enabled, then CBS is pointless because the txtime already
> specifies the bandwidth implicitly.


Assuming there is no "interfering" traffic on that traffic class, yes.
Otherwise, CBS could be configured just to avoid that outbound traffic ever goes
beyond the reserved bandwidth.


> 
> The problem is when one program uses txtime and another uses CBS, then
> the CBS user will experience really wrong performance.


Good point. We'll need to adjust the launch time for controllers that behave
like the i210 then, imo.


Thanks,
Jesus


> 
> Thanks,
> Richard
> 

^ permalink raw reply

* [PATCH v2] sctp: full support for ipv6 ip_nonlocal_bind & IP_FREEBIND
From: Laszlo Toth @ 2017-10-23 17:19 UTC (permalink / raw)
  To: Vlad Yasevich, Neil Horman, linux-sctp
  Cc: network dev, David S. Miller, Xin Long

Commit 9b9742022888 ("sctp: support ipv6 nonlocal bind")
introduced support for the above options as v4 sctp did,
so patched sctp_v6_available().

In the v4 implementation it's enough, because
sctp_inet_bind_verify() just returns with sctp_v4_available().
However sctp_inet6_bind_verify() has an extra check before that
for link-local scope_id, which won't respect the above options.

Added the checks before calling ipv6_chk_addr(), but
not before the validation of scope_id.

before (w/ both options):
 ./v6test fe80::10 sctp
 bind failed, errno: 99 (Cannot assign requested address)
 ./v6test fe80::10 tcp
 bind success, errno: 0 (Success)

after (w/ both options):
 ./v6test fe80::10 sctp
 bind success, errno: 0 (Success)

Signed-off-by: Laszlo Toth <laszlth@gmail.com>
---
changed in v2:
 - fixed indentation

 net/sctp/ipv6.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 51c4887..7fe9e1d1 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -882,8 +882,10 @@ static int sctp_inet6_bind_verify(struct sctp_sock *opt, union sctp_addr *addr)
 			net = sock_net(&opt->inet.sk);
 			rcu_read_lock();
 			dev = dev_get_by_index_rcu(net, addr->v6.sin6_scope_id);
-			if (!dev ||
-			    !ipv6_chk_addr(net, &addr->v6.sin6_addr, dev, 0)) {
+			if (!dev || !(opt->inet.freebind ||
+				      net->ipv6.sysctl.ip_nonlocal_bind ||
+				      ipv6_chk_addr(net, &addr->v6.sin6_addr,
+						    dev, 0))) {
 				rcu_read_unlock();
 				return 0;
 			}
-- 
2.7.4

^ permalink raw reply related

* [PATCH] ethernet: cavium: octeon: Switch to using netdev_info().
From: Steven J. Hill @ 2017-10-23 16:43 UTC (permalink / raw)
  To: netdev; +Cc: Steven J. Hill, David Daney

Signed-off-by: Steven J. Hill <Steven.Hill@cavium.com>
Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c | 25 +++++++++++-------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
index 2887bca..3bb8fbd 100644
--- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
+++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
@@ -705,14 +705,15 @@ static int octeon_mgmt_ioctl_hwtstamp(struct net_device *netdev,
 			u64 clock_comp = (NSEC_PER_SEC << 32) /	octeon_get_io_clock_rate();
 			if (!ptp.s.ptp_en)
 				cvmx_write_csr(CVMX_MIO_PTP_CLOCK_COMP, clock_comp);
-			pr_info("PTP Clock: Using sclk reference at %lld Hz\n",
+			netdev_info(netdev,
+				"PTP Clock: Using sclk reference at %lld Hz\n",
 				(NSEC_PER_SEC << 32) / clock_comp);
 		} else {
 			/* The clock is already programmed to use a GPIO */
 			u64 clock_comp = cvmx_read_csr(CVMX_MIO_PTP_CLOCK_COMP);
-			pr_info("PTP Clock: Using GPIO %d at %lld Hz\n",
-				ptp.s.ext_clk_in,
-				(NSEC_PER_SEC << 32) / clock_comp);
+			netdev_info(netdev,
+				"PTP Clock: Using GPIO %d at %lld Hz\n",
+				ptp.s.ext_clk_in, (NSEC_PER_SEC << 32) / clock_comp);
 		}
 
 		/* Enable the clock if it wasn't done already */
@@ -925,16 +926,12 @@ static void octeon_mgmt_adjust_link(struct net_device *netdev)
 
 	spin_unlock_irqrestore(&p->lock, flags);
 
-	if (link_changed != 0) {
-		if (link_changed > 0) {
-			pr_info("%s: Link is up - %d/%s\n", netdev->name,
-				phydev->speed,
-				phydev->duplex == DUPLEX_FULL ?
-				"Full" : "Half");
-		} else {
-			pr_info("%s: Link is down\n", netdev->name);
-		}
-	}
+	if (link_changed != 0)
+		if (link_changed > 0)
+			netdev_info(netdev, "Link is up - %d/%s\n",
+				phydev->speed, phydev->duplex == DUPLEX_FULL ? "Full" : "Half");
+		else
+			netdev_info(netdev, "Link is down\n");
 }
 
 static int octeon_mgmt_init_phy(struct net_device *netdev)
-- 
2.1.4

^ permalink raw reply related

* Re: problem with rtnetlink 'reference' count
From: Florian Westphal @ 2017-10-23 16:37 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Florian Westphal, David Miller, netdev
In-Reply-To: <20171023162006.GH3165@worktop.lehotels.local>

Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Oct 23, 2017 at 05:32:00PM +0200, Florian Westphal wrote:
> 
> > >  1) it not in fact a refcount, so using refcount_t is silly
> > 
> > Your suggestion is...?
> 
> Normal atomic_t

Why?  refcount_t gives debug options to catch leaks/underflows,
atomic_t does not.

Is refcount_t only supposed to be used with dec_and_test patterns?

> To avoid the problem of te inc being observed late.
> 
> > However, this refcount_dec is misplaced anyway as it would need
> > to occur from nlcb->done() (the handler function gets stored in socket for
> > use by next recvmsg), so this change is indeed not helpful at all.
> > 
> > >  3) waiting with a schedule()/yield() loop is complete crap and subject
> > >     life-locks, imagine doing that rtnl_unregister_all() from a RT task.
> 
> > Alternatively we can of course sleep instead of schedule() but that
> > doesn't appear too appealing either (albeit it is a lot less intrusive).
> 
> That is much better than a yield loop.
> 
> > Any other idea?
> 
> This rtnetlink_rcv_msg() is called from softirq-context, right? Also,
> all that stuff happens with rcu_read_lock() held.

No, its called from process context.

I need to run now but plan to test and submit something like this:

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -263,7 +263,7 @@ void rtnl_unregister_all(int protocol)
 	synchronize_net();
 
 	while (refcount_read(&rtnl_msg_handlers_ref[protocol]) > 1)
-		schedule();
+		msleep(1);
 	kfree(handlers);
 }
 EXPORT_SYMBOL_GPL(rtnl_unregister_all);
@@ -4149,6 +4149,16 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+
+static int rtnl_dumper_done(struct netlink_callback *cb)
+{
+	unsigned int family = (unsigned long)cb->data;
+
+	refcount_dec(&rtnl_msg_handlers_ref[family]);
+	smp_mb__after_atomic();
+	return 0;
+}
+
 /* Process one rtnetlink message. */
 
 static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -4207,6 +4217,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 		}
 
 		refcount_inc(&rtnl_msg_handlers_ref[family]);
+		smp_mb__after_atomic();
 
 		if (type == RTM_GETLINK - RTM_BASE)
 			min_dump_alloc = rtnl_calcit(skb, nlh);
@@ -4217,11 +4228,12 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 		{
 			struct netlink_dump_control c = {
 				.dump		= dumpit,
+				.done		= rtnl_dumper_done,
 				.min_dump_alloc	= min_dump_alloc,
+				.data		= (void *)(unsigned long)family,
 			};
 			err = netlink_dump_start(rtnl, skb, nlh, &c);
 		}
-		refcount_dec(&rtnl_msg_handlers_ref[family]);
 		return err;
 	}
 

^ permalink raw reply

* Re: [PATCH net-next v2 5/6] devlink: Adding num MSI-X vectors per VF NVRAM config param
From: Steve Lin @ 2017-10-23 16:35 UTC (permalink / raw)
  To: Yuval Mintz
  Cc: Jiri Pirko, netdev@vger.kernel.org, Jiri Pirko,
	davem@davemloft.net, michael.chan@broadcom.com,
	linville@tuxdriver.com, gospo@broadcom.com
In-Reply-To: <AM0PR0502MB3683E269475F183C7953B8B3BF460@AM0PR0502MB3683.eurprd05.prod.outlook.com>

On Mon, Oct 23, 2017 at 10:37 AM, Yuval Mintz <yuvalm@mellanox.com> wrote:
>> >> On Fri, Oct 20, 2017 at 10:10 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> >> > Fri, Oct 20, 2017 at 04:03:55PM CEST, steven.lin1@broadcom.com wrote:
>> >> >>On Thu, Oct 19, 2017 at 5:39 PM, Jiri Pirko <jiri@resnulli.us> wrote:
>> >> >>> Thu, Oct 19, 2017 at 10:32:21PM CEST, yuvalm@mellanox.com wrote:
>> >> >>>>> Adding DEVLINK_PERM_CONFIG_MSIX_VECTORS_PER_VF
>> >> permanent
>> >> >>>>> config
>> >> >>>>> parameter.  Defines number of MSI-X vectors allocated per VF.
>> >> >>>>> Value is permanent (stored in NVRAM), so becomes the new
>> default
>> >> >>>>> value for this device.
>> >> >>>>
>> >> >>>>Sounds like you're having this enforce the same configuration for all
>> >> child VFs.
>> >> >>>
>> >> >>> Yeah, this sounds like per-port config.
>> >> >>>
>> >> >>
>> >> >>Well, it gets a little tricky here.  I assume some cards handle this
>> >> >>per-port.  Other cards might handle this per PF, where PF may not
>> >> >>always correspond 1:1 with a port.  And some cards maybe just allow a
>> >> >>single value for this parameter for the entire card, covering all
>> >> >>ports/PFs.
>> >> >>
>> >> >>To keep things simple and as general as possible, it made sense to set
>> >> >>all parameters on a per-PCI device level.  As I mentioned in my
>> >> >>cover-letter, the devices most likely to use these proposed commands
>> >> >>do not have a single "whole asic" PCI b/d/f with internal mechanism
>> >> >>for accessing ports - most expose each port (and each function on each
>> >> >>port) as a separate PCI b/d/f, with no separate "whole asic" PCI
>> >> >>b/d/f.  That's how the BCM cards work, and I think that's how the
>> MLNX
>> >> >>cards work, and others that would be likely to use these cmds.
>> >> >>
>> >> >>So, to summarize, you direct the command to the PCI b/d/f you want
>> to
>> >> >>target.  Does this make sense?
>> >> >
>> >> > So you plan to have 1 devlink instance for each vf? Not sure that
>> >> > does sound right to me :/
>> >> >
>> >>
>> >> For the commands proposed in this patchset, AFAIK they all apply on a
>> >> per-PF or broader, i.e. per-port or whole-card, granularity, since
>> >> they affect permanent config that applies at boot-up.  So, no, the VFs
>> >> don't really come into play here.
>> >
>> > Regardless of whether you're planning on having VFs as devlink instances,
>> > the actual attribute question remains -
>> > you're proposing an attribute that forces all VFs to have the same value.
>> > This probably suits your PCI core limitations but other vendors might have
>> > a different capability set, and accepting this design limitation now would
>> > muck all future extension attempts of such attributes.
>> >
>> > I think VF configurations should be planned in advance for supporting a
>> > per-VF Configuration whenever it's possible - even if not required
>> [/possible]
>> > by the one pushing the new attribute.
>> >
>>
>> The commands being added in this patch are for permanent (i.e. NVRAM)
>> config - essentially setting the new default values for various
>> features of the device at boot-up.  At that initialization time, no
>> VFs are yet instantiated.
>>
>> So my perspective was, in general (not just for our specific device /
>> design), it doesn't seem like permanent config parameters would be set
>> on individual VFs.  That was what my previous comment was trying to
>> convey.
>
> That's an odd assumption; Why should you assume there's some device
> that allows configuring persistent behavior for all VFs but think no other
> would set the same on a per-VF basis?
>
>> If that assumption is wrong, though, and there is some device that has
>> NVRAM config that is set per-VF, I assume the user would instantiate
>> the VF and then call the devlink API on the pci device corresponding
>> to the VF they with to affect, and I think the model proposed still
>> works.
>
> What would be the purpose of re-configuring a value reflected in the
> PCI device for an already instantiated VF?
>
>> Are you suggesting adding a mechanism to set NVRAM parameters on a
>> per-VF basis, without instantiating the VF first?  I would prefer not
>> adding such a mechanism unless/until there's a use case for it.
>
> The thing is that you're suggesting a new UAPI; We don't have the leisure
> of pushing a partial implementation and changing it later on.

I hope we're not talking past each other because I'm not sure we're
saying the same thing.  But if you have a device which has NVRAM
config on an individual VF basis, and you want to be able to get/set
that configuration without instantiating the VF first (i.e. without a
PCI device to operate on), then one way to handle this is with a new
attribute, DEVLINK_ATTR_PERM_CONFIG_VF_INDEX, for example.

It could be sent in the nested DEVLINK_ATTR_PERM_CONFIG attribute,
along with the existing DEVLINK_ATTR_PERM_CONFIG_PARAMETER and _VALUE,
to indicate a specific VF within the PF that you are targeting.

That seems like the type of thing that could be added later, if/when
such a device needed support, without breaking the UAPI, couldn't it?

^ permalink raw reply

* [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets
From: John Allen @ 2017-10-23 16:34 UTC (permalink / raw)
  To: netdev, Thomas Falcon, Nathan Fontenot
In-Reply-To: <0a25df7c-09d4-c6ca-7441-51535c6c3dc5@linux.vnet.ibm.com>

For all non-fatal reset conditions, the hypervisor will send a failover when
we attempt to initialize the crq and the vnic client is expected to handle
that failover instead of the existing non-fatal reset. To handle this, we
need to return from init with a return code that indicates that we have hit
this case.

Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index c2c4a5b..fa6b2ad 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1506,7 +1506,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,

 		rc = ibmvnic_init(adapter);
 		if (rc)
-			return 0;
+			return IBMVNIC_INIT_FAILED;

 		/* If the adapter was in PROBE state prior to the reset,
 		 * exit here.
@@ -1609,7 +1609,7 @@ static void __ibmvnic_reset(struct work_struct *work)
 	while (rwi) {
 		rc = do_reset(adapter, rwi, reset_state);
 		kfree(rwi);
-		if (rc)
+		if (rc && rc != IBMVNIC_INIT_FAILED)
 			break;

 		rwi = get_next_rwi(adapter);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 8de998a..b5aaa6e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -30,6 +30,8 @@
 #define IBMVNIC_DRIVER_VERSION	"1.0.1"
 #define IBMVNIC_INVALID_MAP	-1
 #define IBMVNIC_STATS_TIMEOUT	1
+#define IBMVNIC_INIT_FAILED	2
+
 /* basic structures plus 100 2k buffers */
 #define IBMVNIC_IO_ENTITLEMENT_DEFAULT	610305

^ permalink raw reply related

* [PATCH v2 net-next 1/2] ibmvnic: Update reset infrastructure to support tunable parameters
From: John Allen @ 2017-10-23 16:33 UTC (permalink / raw)
  To: netdev, Thomas Falcon, Nathan Fontenot
In-Reply-To: <0a25df7c-09d4-c6ca-7441-51535c6c3dc5@linux.vnet.ibm.com>

Update ibmvnic reset infrastructure to include a new reset option that will
allow changing of tunable parameters. There currently is no way to request
different capabilities from the vnic server on the fly so this patch
achieves this by resetting the driver and attempting to log in with the
requested changes. If the reset operation fails, the old values of the
tunable parameters are stored in the "fallback" struct and we attempt to
login with the fallback values.

Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 11eba82..c2c4a5b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -115,6 +115,7 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *,
 static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter);
 static int ibmvnic_init(struct ibmvnic_adapter *);
 static void release_crq_queue(struct ibmvnic_adapter *);
+static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p);

 struct ibmvnic_stat {
 	char name[ETH_GSTRING_LEN];
@@ -926,6 +927,11 @@ static int ibmvnic_open(struct net_device *netdev)

 	mutex_lock(&adapter->reset_lock);

+	if (adapter->desired->mac) {
+		__ibmvnic_set_mac(netdev, adapter->desired->mac);
+		adapter->desired->mac = NULL;
+	}
+
 	if (adapter->state != VNIC_CLOSED) {
 		rc = ibmvnic_login(netdev);
 		if (rc) {
@@ -1426,7 +1432,7 @@ static void ibmvnic_set_multi(struct net_device *netdev)
 	}
 }

-static int ibmvnic_set_mac(struct net_device *netdev, void *p)
+static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
 	struct sockaddr *addr = p;
@@ -1444,6 +1450,21 @@ static int ibmvnic_set_mac(struct net_device *netdev, void *p)
 	return 0;
 }

+static int ibmvnic_set_mac(struct net_device *netdev, void *p)
+{
+	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+	struct sockaddr *addr = p;
+
+	if (adapter->state != VNIC_OPEN) {
+		memcpy(adapter->desired->mac, addr, sizeof(struct sockaddr));
+		return 0;
+	}
+
+	__ibmvnic_set_mac(netdev, addr);
+
+	return 0;
+}
+
 /**
  * do_reset returns zero if we are able to keep processing reset events, or
  * non-zero if we hit a fatal error and must halt.
@@ -1470,6 +1491,13 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	if (rc)
 		return rc;

+	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
+	    adapter->wait_for_reset) {
+		release_resources(adapter);
+		release_sub_crqs(adapter);
+		release_crq_queue(adapter);
+	}
+
 	if (adapter->reset_reason != VNIC_RESET_NON_FATAL) {
 		/* remove the closed state so when we call open it appears
 		 * we are coming from the probed state.
@@ -1492,16 +1520,23 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 			return 0;
 		}

-		rc = reset_tx_pools(adapter);
-		if (rc)
-			return rc;
+		if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
+		    adapter->wait_for_reset) {
+			rc = init_resources(adapter);
+			if (rc)
+				return rc;
+		} else {
+			rc = reset_tx_pools(adapter);
+			if (rc)
+				return rc;

-		rc = reset_rx_pools(adapter);
-		if (rc)
-			return rc;
+			rc = reset_rx_pools(adapter);
+			if (rc)
+				return rc;

-		if (reset_state == VNIC_CLOSED)
-			return 0;
+			if (reset_state == VNIC_CLOSED)
+				return 0;
+		}
 	}

 	rc = __ibmvnic_open(netdev);
@@ -1580,6 +1615,12 @@ static void __ibmvnic_reset(struct work_struct *work)
 		rwi = get_next_rwi(adapter);
 	}

+	if (adapter->wait_for_reset) {
+		adapter->wait_for_reset = false;
+		adapter->reset_done_rc = rc;
+		complete(&adapter->reset_done);
+	}
+
 	if (rc) {
 		netdev_dbg(adapter->netdev, "Reset failed\n");
 		free_all_rwi(adapter);
@@ -1759,9 +1800,42 @@ static void ibmvnic_netpoll_controller(struct net_device *dev)
 }
 #endif

+static int wait_for_reset(struct ibmvnic_adapter *adapter)
+{
+	adapter->fallback->mtu = adapter->req_mtu;
+	adapter->fallback->rx_queues = adapter->req_rx_queues;
+	adapter->fallback->tx_queues = adapter->req_tx_queues;
+	adapter->fallback->rx_entries = adapter->req_rx_add_entries_per_subcrq;
+	adapter->fallback->tx_entries = adapter->req_tx_entries_per_subcrq;
+
+	init_completion(&adapter->reset_done);
+	ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
+	adapter->wait_for_reset = true;
+	wait_for_completion(&adapter->reset_done);
+
+	if (adapter->reset_done_rc) {
+		adapter->desired->mtu = adapter->fallback->mtu;
+		adapter->desired->rx_queues = adapter->fallback->rx_queues;
+		adapter->desired->tx_queues = adapter->fallback->tx_queues;
+		adapter->desired->rx_entries = adapter->fallback->rx_entries;
+		adapter->desired->tx_entries = adapter->fallback->tx_entries;
+
+		init_completion(&adapter->reset_done);
+		ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
+		wait_for_completion(&adapter->reset_done);
+	}
+	adapter->wait_for_reset = false;
+
+	return adapter->reset_done_rc;
+}
+
 static int ibmvnic_change_mtu(struct net_device *netdev, int new_mtu)
 {
-	return -EOPNOTSUPP;
+	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+
+	adapter->desired->mtu = new_mtu + ETH_HLEN;
+
+	return wait_for_reset(adapter);
 }

 static const struct net_device_ops ibmvnic_netdev_ops = {
@@ -1849,6 +1923,27 @@ static void ibmvnic_get_ringparam(struct net_device *netdev,
 	ring->rx_jumbo_pending = 0;
 }

+static int ibmvnic_set_ringparam(struct net_device *netdev,
+				 struct ethtool_ringparam *ring)
+{
+	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+
+	if (ring->rx_pending > adapter->max_rx_add_entries_per_subcrq  ||
+	    ring->tx_pending > adapter->max_tx_entries_per_subcrq) {
+		netdev_err(netdev, "Invalid request.\n");
+		netdev_err(netdev, "Max tx buffers = %llu\n",
+			   adapter->max_rx_add_entries_per_subcrq);
+		netdev_err(netdev, "Max rx buffers = %llu\n",
+			   adapter->max_tx_entries_per_subcrq);
+		return -EINVAL;
+	}
+
+	adapter->desired->rx_entries = ring->rx_pending;
+	adapter->desired->tx_entries = ring->tx_pending;
+
+	return wait_for_reset(adapter);
+}
+
 static void ibmvnic_get_channels(struct net_device *netdev,
 				 struct ethtool_channels *channels)
 {
@@ -1864,6 +1959,17 @@ static void ibmvnic_get_channels(struct net_device *netdev,
 	channels->combined_count = 0;
 }

+static int ibmvnic_set_channels(struct net_device *netdev,
+				struct ethtool_channels *channels)
+{
+	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
+
+	adapter->desired->rx_queues = channels->rx_count;
+	adapter->desired->tx_queues = channels->tx_count;
+
+	return wait_for_reset(adapter);
+}
+
 static void ibmvnic_get_strings(struct net_device *dev, u32 stringset, u8 *data)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(dev);
@@ -1960,7 +2066,9 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev,
 	.set_msglevel		= ibmvnic_set_msglevel,
 	.get_link		= ibmvnic_get_link,
 	.get_ringparam		= ibmvnic_get_ringparam,
+	.set_ringparam		= ibmvnic_set_ringparam,
 	.get_channels		= ibmvnic_get_channels,
+	.set_channels		= ibmvnic_set_channels,
 	.get_strings            = ibmvnic_get_strings,
 	.get_sset_count         = ibmvnic_get_sset_count,
 	.get_ethtool_stats	= ibmvnic_get_ethtool_stats,
@@ -2426,6 +2534,7 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry)
 {
 	struct device *dev = &adapter->vdev->dev;
 	union ibmvnic_crq crq;
+	int max_entries;

 	if (!retry) {
 		/* Sub-CRQ entries are 32 byte long */
@@ -2437,21 +2546,60 @@ static void ibmvnic_send_req_caps(struct ibmvnic_adapter *adapter, int retry)
 			return;
 		}

-		/* Get the minimum between the queried max and the entries
-		 * that fit in our PAGE_SIZE
-		 */
-		adapter->req_tx_entries_per_subcrq =
-		    adapter->max_tx_entries_per_subcrq > entries_page ?
-		    entries_page : adapter->max_tx_entries_per_subcrq;
-		adapter->req_rx_add_entries_per_subcrq =
-		    adapter->max_rx_add_entries_per_subcrq > entries_page ?
-		    entries_page : adapter->max_rx_add_entries_per_subcrq;
-
-		adapter->req_tx_queues = adapter->opt_tx_comp_sub_queues;
-		adapter->req_rx_queues = adapter->opt_rx_comp_queues;
-		adapter->req_rx_add_queues = adapter->max_rx_add_queues;
+		if (adapter->desired->mtu)
+			adapter->req_mtu = adapter->desired->mtu;
+		else
+			adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN;
+
+		if (!adapter->desired->tx_entries)
+			adapter->desired->tx_entries =
+					adapter->max_tx_entries_per_subcrq;
+		if (!adapter->desired->rx_entries)
+			adapter->desired->rx_entries =
+					adapter->max_rx_add_entries_per_subcrq;
+
+		max_entries = IBMVNIC_MAX_LTB_SIZE /
+			      (adapter->req_mtu + IBMVNIC_BUFFER_HLEN);
+
+		if ((adapter->req_mtu + IBMVNIC_BUFFER_HLEN) *
+			adapter->desired->tx_entries > IBMVNIC_MAX_LTB_SIZE) {
+			adapter->desired->tx_entries = max_entries;
+		}
+
+		if ((adapter->req_mtu + IBMVNIC_BUFFER_HLEN) *
+			adapter->desired->rx_entries > IBMVNIC_MAX_LTB_SIZE) {
+			adapter->desired->rx_entries = max_entries;
+		}

-		adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN;
+		if (adapter->desired->tx_entries)
+			adapter->req_tx_entries_per_subcrq =
+					adapter->desired->tx_entries;
+		else
+			adapter->req_tx_entries_per_subcrq =
+					adapter->max_tx_entries_per_subcrq;
+
+		if (adapter->desired->rx_entries)
+			adapter->req_rx_add_entries_per_subcrq =
+					adapter->desired->rx_entries;
+		else
+			adapter->req_rx_add_entries_per_subcrq =
+					adapter->max_rx_add_entries_per_subcrq;
+
+		if (adapter->desired->tx_queues)
+			adapter->req_tx_queues =
+					adapter->desired->tx_queues;
+		else
+			adapter->req_tx_queues =
+					adapter->opt_tx_comp_sub_queues;
+
+		if (adapter->desired->rx_queues)
+			adapter->req_rx_queues =
+					adapter->desired->rx_queues;
+		else
+			adapter->req_rx_queues =
+					adapter->opt_rx_comp_queues;
+
+		adapter->req_rx_add_queues = adapter->max_rx_add_queues;
 	}

 	memset(&crq, 0, sizeof(crq));
@@ -3272,6 +3420,7 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq,
 			    struct ibmvnic_adapter *adapter)
 {
 	struct device *dev = &adapter->vdev->dev;
+	struct net_device *netdev = adapter->netdev;
 	struct ibmvnic_login_rsp_buffer *login_rsp = adapter->login_rsp_buf;
 	struct ibmvnic_login_buffer *login = adapter->login_buf;
 	int i;
@@ -3291,6 +3440,8 @@ static int handle_login_rsp(union ibmvnic_crq *login_rsp_crq,
 		return 0;
 	}

+	netdev->mtu = adapter->req_mtu - ETH_HLEN;
+
 	netdev_dbg(adapter->netdev, "Login Response Buffer:\n");
 	for (i = 0; i < (adapter->login_rsp_buf_sz - 1) / 8 + 1; i++) {
 		netdev_dbg(adapter->netdev, "%016lx\n",
@@ -3846,7 +3997,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
 	unsigned long timeout = msecs_to_jiffies(30000);
 	int rc;

-	if (adapter->resetting) {
+	if (adapter->resetting && !adapter->wait_for_reset) {
 		rc = ibmvnic_reset_crq(adapter);
 		if (!rc)
 			rc = vio_enable_interrupts(adapter->vdev);
@@ -3880,7 +4031,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
 		return -1;
 	}

-	if (adapter->resetting)
+	if (adapter->resetting && !adapter->wait_for_reset)
 		rc = reset_sub_crq_queues(adapter);
 	else
 		rc = init_sub_crqs(adapter);
@@ -3931,6 +4082,11 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	adapter->vdev = dev;
 	adapter->netdev = netdev;

+	adapter->desired = kzalloc(sizeof(*adapter->desired), GFP_KERNEL);
+	adapter->desired->mac = kzalloc(sizeof(*adapter->desired->mac),
+					GFP_KERNEL);
+	adapter->fallback = kzalloc(sizeof(*adapter->fallback), GFP_KERNEL);
+
 	ether_addr_copy(adapter->mac_addr, mac_addr_p);
 	ether_addr_copy(netdev->dev_addr, adapter->mac_addr);
 	netdev->irq = dev->irq;
@@ -3956,6 +4112,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	} while (rc == EAGAIN);

 	netdev->mtu = adapter->req_mtu - ETH_HLEN;
+	netdev->min_mtu = adapter->min_mtu - ETH_HLEN;
+	netdev->max_mtu = adapter->max_mtu - ETH_HLEN;

 	rc = device_create_file(&dev->dev, &dev_attr_failover);
 	if (rc)
@@ -3970,6 +4128,9 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	dev_info(&dev->dev, "ibmvnic registered\n");

 	adapter->state = VNIC_PROBED;
+
+	adapter->wait_for_reset = false;
+
 	return 0;

 ibmvnic_register_fail:
@@ -3993,6 +4154,11 @@ static int ibmvnic_remove(struct vio_dev *dev)
 	mutex_lock(&adapter->reset_lock);

 	release_resources(adapter);
+
+	kfree(adapter->desired->mac);
+	kfree(adapter->desired);
+	kfree(adapter->fallback);
+
 	release_sub_crqs(adapter);
 	release_crq_queue(adapter);

diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index 7aa347a..8de998a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -42,6 +42,9 @@
 #define IBMVNIC_TSO_BUF_SZ	65536
 #define IBMVNIC_TSO_BUFS	64

+#define IBMVNIC_MAX_LTB_SIZE ((1 << (MAX_ORDER - 1)) * PAGE_SIZE)
+#define IBMVNIC_BUFFER_HLEN 500
+
 struct ibmvnic_login_buffer {
 	__be32 len;
 	__be32 version;
@@ -945,13 +948,23 @@ enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1,
 			   VNIC_RESET_MOBILITY,
 			   VNIC_RESET_FATAL,
 			   VNIC_RESET_NON_FATAL,
-			   VNIC_RESET_TIMEOUT};
+			   VNIC_RESET_TIMEOUT,
+			   VNIC_RESET_CHANGE_PARAM};

 struct ibmvnic_rwi {
 	enum ibmvnic_reset_reason reset_reason;
 	struct list_head list;
 };

+struct ibmvnic_tunables {
+	u64 rx_queues;
+	u64 tx_queues;
+	u64 rx_entries;
+	u64 tx_entries;
+	u64 mtu;
+	struct sockaddr *mac;
+};
+
 struct ibmvnic_adapter {
 	struct vio_dev *vdev;
 	struct net_device *netdev;
@@ -1012,6 +1025,10 @@ struct ibmvnic_adapter {
 	struct completion fw_done;
 	int fw_done_rc;

+	struct completion reset_done;
+	int reset_done_rc;
+	bool wait_for_reset;
+
 	/* partner capabilities */
 	u64 min_tx_queues;
 	u64 min_rx_queues;
@@ -1056,4 +1073,7 @@ struct ibmvnic_adapter {
 	struct work_struct ibmvnic_reset;
 	bool resetting;
 	bool napi_enabled, from_passive_init;
+
+	struct ibmvnic_tunables *desired;
+	struct ibmvnic_tunables *fallback;
 };

^ permalink raw reply related

* [PATCH v2 net-next 0/2] ibmvnic: Tunable parameter support
From: John Allen @ 2017-10-23 16:30 UTC (permalink / raw)
  To: netdev, Thomas Falcon, Nathan Fontenot

This series implements support for changing tunable parameters such as the
mtu, number of tx/rx queues, and number of buffers per queue via ethtool
and ifconfig.

v2: Fix conflict with Tom's recently applied TSO/SG patches

John Allen (2):
  ibmvnic: Update reset infrastructure to support tunable parameters
  ibmvnic: Fix failover error path for non-fatal resets

 drivers/net/ethernet/ibm/ibmvnic.c | 222 ++++++++++++++++++++++++++++++++-----
 drivers/net/ethernet/ibm/ibmvnic.h |  24 +++-
 2 files changed, 216 insertions(+), 30 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* Re: [PATCH v2 net-next 0/6] add a set of tracepoints to tcp stack
From: Song Liu @ 2017-10-23 16:26 UTC (permalink / raw)
  To: netdev@vger.kernel.org, davem@davemloft.net
  Cc: alexei.starovoitov@gmail.com, liu.song.a23@gmail.com,
	Eric Dumazet, hannes@stressinduktion.org,
	brendan.d.gregg@gmail.com, ncardwell@google.com, David Ahern,
	Kernel Team, xiyou.wangcong@gmail.com
In-Reply-To: <20171023162027.2880430-1-songliubraving@fb.com>

CCing key audience of these patches.

Thanks,
Song

> On Oct 23, 2017, at 9:20 AM, Song Liu <songliubraving@fb.com> wrote:
> 
> Changes from v1:
> 
> Fix build error (with ipv6 as ko) by adding EXPORT_TRACEPOINT_SYMBOL_GPL
> for trace_tcp_send_reset.
> 
> These patches add the following tracepoints to tcp stack.
> 
> tcp_send_reset
> tcp_receive_reset
> tcp_destroy_sock
> tcp_set_state
> 
> These tracepoints can be used to track TCP state changes. Such state
> changes include but are not limited to: connection establish,
> connection termination, tx and rx of RST, various retransmits.
> 
> Currently, we use the following kprobes to trace these events:
> 
> int kprobe__tcp_validate_incoming
> int kprobe__tcp_send_active_reset
> int kprobe__tcp_v4_send_reset
> int kprobe__tcp_v6_send_reset
> int kprobe__tcp_v4_destroy_sock
> int kprobe__tcp_set_state
> int kprobe__tcp_retransmit_skb
> 
> These tracepoints will help us simplify this work.
> 
> Thanks,
> Song
> 
> Song Liu (6):
>  tcp: add trace event class tcp_event_sk_skb
>  tcp: mark trace event arguments sk and skb as const
>  tcp: add tracepoint trace_tcp_send_reset
>  tcp: add tracepoint trace_tcp_receive_reset
>  tcp: add tracepoint trace_tcp_destroy_sock
>  tcp: add tracepoint trace_tcp_set_state()
> 
> include/trace/events/tcp.h | 181 ++++++++++++++++++++++++++++++++++++++++++++-
> net/core/net-traces.c      |   2 +
> net/ipv4/tcp.c             |   4 +
> net/ipv4/tcp_input.c       |   3 +
> net/ipv4/tcp_ipv4.c        |   8 +-
> net/ipv4/tcp_output.c      |   5 ++
> net/ipv6/tcp_ipv6.c        |  10 ++-
> 7 files changed, 206 insertions(+), 7 deletions(-)
> 
> --
> 2.9.5

^ permalink raw reply

* [PATCH net-next 12/12] tools: bpftool: update documentation for --json and --pretty usage
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Update the documentation to provide help about JSON output generation,
and add an example in bpftool-prog manual page.

Also reintroduce an example that was left aside when the tool was moved
from GitHub to the kernel sources, in order to show how to mount the
bpffs file system (to pin programs) inside the bpftool-prog manual page.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/Documentation/bpftool-map.rst  | 11 ++++-
 tools/bpf/bpftool/Documentation/bpftool-prog.rst | 61 +++++++++++++++++++++++-
 tools/bpf/bpftool/Documentation/bpftool.rst      | 12 ++++-
 tools/bpf/bpftool/main.c                         |  6 ++-
 tools/bpf/bpftool/main.h                         |  2 +
 tools/bpf/bpftool/map.c                          |  1 +
 tools/bpf/bpftool/prog.c                         |  1 +
 7 files changed, 88 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 3954b3ea4f26..abb9ee940b15 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -10,7 +10,9 @@ tool for inspection and simple manipulation of eBPF maps
 SYNOPSIS
 ========
 
-	**bpftool** **map** *COMMAND*
+	**bpftool** [*OPTIONS*] **map** *COMMAND*
+
+	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
 
 	*COMMANDS* :=
 	{ **show** | **dump** | **update** | **lookup** | **getnext** | **delete**
@@ -77,6 +79,13 @@ OPTIONS
 	-v, --version
 		  Print version number (similar to **bpftool version**).
 
+	-j, --json
+		  Generate JSON output. For commands that cannot produce JSON, this
+		  option has no effect.
+
+	-p, --pretty
+		  Generate human-readable JSON output. Implies **-j**.
+
 EXAMPLES
 ========
 **# bpftool map show**
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 685a19e71fec..0f25d3c39e05 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -10,6 +10,16 @@ tool for inspection and simple manipulation of eBPF progs
 SYNOPSIS
 ========
 
+	**bpftool** [*OPTIONS*] **prog** *COMMAND*
+
+	*OPTIONS* := { { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+
+	*COMMANDS* :=
+	{ **show** | **dump xlated** | **dump jited** | **pin** | **help** }
+
+MAP COMMANDS
+=============
+
 |	**bpftool** **prog show** [*PROG*]
 |	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**}]
 |	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
@@ -58,6 +68,13 @@ OPTIONS
 	-v, --version
 		  Print version number (similar to **bpftool version**).
 
+	-j, --json
+		  Generate JSON output. For commands that cannot produce JSON, this
+		  option has no effect.
+
+	-p, --pretty
+		  Generate human-readable JSON output. Implies **-j**.
+
 EXAMPLES
 ========
 **# bpftool prog show**
@@ -67,13 +84,33 @@ EXAMPLES
 	loaded_at Sep 29/20:11  uid 0
 	xlated 528B  jited 370B  memlock 4096B  map_ids 10
 
+**# bpftool --json --pretty prog show**
+
+::
+
+    {
+        "programs": [{
+                "id": 10,
+                "type": "xdp",
+                "tag": "005a3d2123620c8b",
+                "loaded_at": "Sep 29/20:11",
+                "uid": 0,
+                "bytes_xlated": 528,
+                "jited": true,
+                "bytes_jited": 370,
+                "bytes_memlock": 4096,
+                "map_ids": [10
+                ]
+            }
+        ]
+    }
+
 |
 | **# bpftool prog dump xlated id 10 file /tmp/t**
 | **# ls -l /tmp/t**
 |   -rw------- 1 root root 560 Jul 22 01:42 /tmp/t
 
-|
-| **# bpftool prog dum jited pinned /sys/fs/bpf/prog**
+**# bpftool prog dum jited tag 005a3d2123620c8b**
 
 ::
 
@@ -83,6 +120,26 @@ EXAMPLES
     sub    $0x28,%rbp
     mov    %rbx,0x0(%rbp)
 
+|
+| **# mount -t bpf none /sys/fs/bpf/**
+| **# bpftool prog pin id 10 /sys/fs/bpf/prog**
+| **# ls -l /sys/fs/bpf/**
+|   -rw------- 1 root root 0 Jul 22 01:43 prog
+
+**# bpftool prog dum jited pinned /sys/fs/bpf/prog opcodes**
+
+::
+
+    push   %rbp
+    55
+    mov    %rsp,%rbp
+    48 89 e5
+    sub    $0x228,%rsp
+    48 81 ec 28 02 00 00
+    sub    $0x28,%rbp
+    48 83 ed 28
+    mov    %rbx,0x0(%rbp)
+    48 89 5d 00
 
 
 SEE ALSO
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index 44e07799d54d..926c03d5a8da 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -10,7 +10,7 @@ tool for inspection and simple manipulation of eBPF programs and maps
 SYNOPSIS
 ========
 
-	**bpftool** *OBJECT* { *COMMAND* | **help** }
+	**bpftool** [*OPTIONS*] *OBJECT* { *COMMAND* | **help** }
 
 	**bpftool** **batch file** *FILE*
 
@@ -18,6 +18,9 @@ SYNOPSIS
 
 	*OBJECT* := { **map** | **program** }
 
+	*OPTIONS* := { { **-V** | **--version** } | { **-h** | **--help** }
+	| { **-j** | **--json** } [{ **-p** | **--pretty** }] }
+
 	*MAP-COMMANDS* :=
 	{ **show** | **dump** | **update** | **lookup** | **getnext** | **delete**
 	| **pin** | **help** }
@@ -41,6 +44,13 @@ OPTIONS
 	-v, --version
 		  Print version number (similar to **bpftool version**).
 
+	-j, --json
+		  Generate JSON output. For commands that cannot produce JSON, this
+		  option has no effect.
+
+	-p, --pretty
+		  Generate human-readable JSON output. Implies **-j**.
+
 SEE ALSO
 ========
 	**bpftool-map**\ (8), **bpftool-prog**\ (8)
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 55ba0a04c102..78d9afb74ef4 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -70,11 +70,13 @@ static int do_help(int argc, char **argv)
 	}
 
 	fprintf(stderr,
-		"Usage: %s OBJECT { COMMAND | help }\n"
+		"Usage: %s [OPTIONS] OBJECT { COMMAND | help }\n"
 		"       %s batch file FILE\n"
 		"       %s version\n"
 		"\n"
-		"       OBJECT := { prog | map }\n",
+		"       OBJECT := { prog | map }\n"
+		"       " HELP_SPEC_OPTIONS "\n"
+		"",
 		bin_name, bin_name, bin_name);
 
 	return 0;
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 04c88b55d8c7..2f94bed03a8d 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -55,6 +55,8 @@
 
 #define HELP_SPEC_PROGRAM						\
 	"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
+#define HELP_SPEC_OPTIONS						\
+	"OPTIONS := { {-j|--json} [{-p|--pretty}] }"
 
 enum bpf_obj_type {
 	BPF_OBJ_UNKNOWN,
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index a611f31f574f..e978ab23a77f 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -850,6 +850,7 @@ static int do_help(int argc, char **argv)
 		"       " HELP_SPEC_PROGRAM "\n"
 		"       VALUE := { BYTES | MAP | PROG }\n"
 		"       UPDATE_FLAGS := { any | exist | noexist }\n"
+		"       " HELP_SPEC_OPTIONS "\n"
 		"",
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e07f35ff80d1..250f80fd46aa 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -625,6 +625,7 @@ static int do_help(int argc, char **argv)
 		"       %s %s help\n"
 		"\n"
 		"       " HELP_SPEC_PROGRAM "\n"
+		"       " HELP_SPEC_OPTIONS "\n"
 		"",
 		bin_name, argv[-2], bin_name, argv[-2], bin_name, argv[-2],
 		bin_name, argv[-2], bin_name, argv[-2]);
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 11/12] tools: bpftool: add cosmetic changes for the manual pages
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Make the look-and-feel of the manual pages somewhat closer to other
manual pages, such as the ones from the utilities from iproute2, by
highlighting more keywords.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/Documentation/bpftool-map.rst  | 25 ++++++++++++------------
 tools/bpf/bpftool/Documentation/bpftool-prog.rst | 12 ++++++------
 tools/bpf/bpftool/Documentation/bpftool.rst      | 12 +++++++-----
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index 5210c4fab356..3954b3ea4f26 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -13,23 +13,24 @@ SYNOPSIS
 	**bpftool** **map** *COMMAND*
 
 	*COMMANDS* :=
-	{ show | dump | update | lookup | getnext | delete | pin | help }
+	{ **show** | **dump** | **update** | **lookup** | **getnext** | **delete**
+	| **pin** | **help** }
 
 MAP COMMANDS
 =============
 
-|	**bpftool** map show   [*MAP*]
-|	**bpftool** map dump    *MAP*
-|	**bpftool** map update  *MAP*  key *BYTES*   value *VALUE* [*UPDATE_FLAGS*]
-|	**bpftool** map lookup  *MAP*  key *BYTES*
-|	**bpftool** map getnext *MAP* [key *BYTES*]
-|	**bpftool** map delete  *MAP*  key *BYTES*
-|	**bpftool** map pin     *MAP*  *FILE*
-|	**bpftool** map help
+|	**bpftool** **map show**   [*MAP*]
+|	**bpftool** **map dump**    *MAP*
+|	**bpftool** **map update**  *MAP*  **key** *BYTES*   **value** *VALUE* [*UPDATE_FLAGS*]
+|	**bpftool** **map lookup**  *MAP*  **key** *BYTES*
+|	**bpftool** **map getnext** *MAP* [**key** *BYTES*]
+|	**bpftool** **map delete**  *MAP*  **key** *BYTES*
+|	**bpftool** **map pin**     *MAP*  *FILE*
+|	**bpftool** **map help**
 |
-|	*MAP* := { id MAP_ID | pinned FILE }
-|	*VALUE* := { BYTES | MAP | PROGRAM }
-|	*UPDATE_FLAGS* := { any | exist | noexist }
+|	*MAP* := { **id** *MAP_ID* | **pinned** *FILE* }
+|	*VALUE* := { *BYTES* | *MAP* | *PROGRAM* }
+|	*UPDATE_FLAGS* := { **any** | **exist** | **noexist** }
 
 DESCRIPTION
 ===========
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 6620a81d9dc9..685a19e71fec 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -10,13 +10,13 @@ tool for inspection and simple manipulation of eBPF progs
 SYNOPSIS
 ========
 
-|	**bpftool** prog show [*PROG*]
-|	**bpftool** prog dump xlated *PROG* [{file *FILE* | opcodes }]
-|	**bpftool** prog dump jited  *PROG* [{file *FILE* | opcodes }]
-|	**bpftool** prog pin *PROG* *FILE*
-|	**bpftool** prog help
+|	**bpftool** **prog show** [*PROG*]
+|	**bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes**}]
+|	**bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes**}]
+|	**bpftool** **prog pin** *PROG* *FILE*
+|	**bpftool** **prog help**
 |
-|	*PROG* := { id *PROG_ID* | pinned *FILE* | tag *PROG_TAG* }
+|	*PROG* := { **id** *PROG_ID* | **pinned** *FILE* | **tag** *PROG_TAG* }
 
 DESCRIPTION
 ===========
diff --git a/tools/bpf/bpftool/Documentation/bpftool.rst b/tools/bpf/bpftool/Documentation/bpftool.rst
index 9c04cd6677bd..44e07799d54d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool.rst
@@ -10,18 +10,20 @@ tool for inspection and simple manipulation of eBPF programs and maps
 SYNOPSIS
 ========
 
-	**bpftool** *OBJECT* { *COMMAND* | help }
+	**bpftool** *OBJECT* { *COMMAND* | **help** }
 
-	**bpftool** batch file *FILE*
+	**bpftool** **batch file** *FILE*
 
-	**bpftool** version
+	**bpftool** **version**
 
 	*OBJECT* := { **map** | **program** }
 
 	*MAP-COMMANDS* :=
-	{ show | dump | update | lookup | getnext | delete | pin | help }
+	{ **show** | **dump** | **update** | **lookup** | **getnext** | **delete**
+	| **pin** | **help** }
 
-	*PROG-COMMANDS* := { show | dump jited | dump xlated | pin | help }
+	*PROG-COMMANDS* := { **show** | **dump jited** | **dump xlated** | **pin**
+	| **help** }
 
 DESCRIPTION
 ===========
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 10/12] tools: bpftool: provide JSON output for all possible commands
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

As all commands can now return JSON output (possibly just a "null"
value), output of `bpftool --json batch file FILE` should also be fully
JSON compliant.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/main.c | 25 +++++++++++++++++++++----
 tools/bpf/bpftool/map.c  | 16 +++++++++++++++-
 tools/bpf/bpftool/prog.c | 12 +++++++++++-
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 9989a77fdc4a..55ba0a04c102 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -64,6 +64,11 @@ void usage(void)
 
 static int do_help(int argc, char **argv)
 {
+	if (json_output) {
+		jsonw_null(json_wtr);
+		return 0;
+	}
+
 	fprintf(stderr,
 		"Usage: %s OBJECT { COMMAND | help }\n"
 		"       %s batch file FILE\n"
@@ -77,10 +82,22 @@ static int do_help(int argc, char **argv)
 
 static int do_version(int argc, char **argv)
 {
-	printf("%s v%d.%d.%d\n", bin_name,
-	       LINUX_VERSION_CODE >> 16,
-	       LINUX_VERSION_CODE >> 8 & 0xf,
-	       LINUX_VERSION_CODE & 0xf);
+	unsigned int version[3];
+
+	version[0] = LINUX_VERSION_CODE >> 16;
+	version[1] = LINUX_VERSION_CODE >> 8 & 0xf;
+	version[2] = LINUX_VERSION_CODE & 0xf;
+
+	if (json_output) {
+		jsonw_start_object(json_wtr);
+		jsonw_name(json_wtr, "version");
+		jsonw_printf(json_wtr, "\"%u.%u.%u\"",
+			     version[0], version[1], version[2]);
+		jsonw_end_object(json_wtr);
+	} else {
+		printf("%s v%u.%u.%u\n", bin_name,
+		       version[0], version[1], version[2]);
+	}
 	return 0;
 }
 
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 86c128c433ba..a611f31f574f 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -651,6 +651,8 @@ static int do_update(int argc, char **argv)
 	free(value);
 	close(fd);
 
+	if (!err && json_output)
+		jsonw_null(json_wtr);
 	return err;
 }
 
@@ -812,16 +814,28 @@ static int do_delete(int argc, char **argv)
 	free(key);
 	close(fd);
 
+	if (!err && json_output)
+		jsonw_null(json_wtr);
 	return err;
 }
 
 static int do_pin(int argc, char **argv)
 {
-	return do_pin_any(argc, argv, bpf_map_get_fd_by_id);
+	int err;
+
+	err = do_pin_any(argc, argv, bpf_map_get_fd_by_id);
+	if (!err && json_output)
+		jsonw_null(json_wtr);
+	return err;
 }
 
 static int do_help(int argc, char **argv)
 {
+	if (json_output) {
+		jsonw_null(json_wtr);
+		return 0;
+	}
+
 	fprintf(stderr,
 		"Usage: %s %s show   [MAP]\n"
 		"       %s %s dump    MAP\n"
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 41bd5390b4fc..e07f35ff80d1 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -602,11 +602,21 @@ static int do_dump(int argc, char **argv)
 
 static int do_pin(int argc, char **argv)
 {
-	return do_pin_any(argc, argv, bpf_prog_get_fd_by_id);
+	int err;
+
+	err = do_pin_any(argc, argv, bpf_prog_get_fd_by_id);
+	if (!err && json_output)
+		jsonw_null(json_wtr);
+	return err;
 }
 
 static int do_help(int argc, char **argv)
 {
+	if (json_output) {
+		jsonw_null(json_wtr);
+		return 0;
+	}
+
 	fprintf(stderr,
 		"Usage: %s %s show [PROG]\n"
 		"       %s %s dump xlated PROG [{ file FILE | opcodes }]\n"
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 09/12] tools: bpftool: turn err() and info() macros into functions
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Turn err() and info() macros into functions.

In order to avoid naming conflicts with variables in the code, rename
them as p_err() and p_info() respectively.

The behavior of these functions is similar to the one of the macros for
plain output. However, when JSON output is requested, these macros
return a JSON-formatted "error" object instead of printing a message to
stderr.

To handle error messages correctly with JSON, a modification was brought
to their behavior nonetheless: the functions now append a end-of-line
character at the end of the message. This way, we can remove end-of-line
characters at the end of the argument strings, and not have them in the
JSON output.

All error messages are formatted to hold in a single call to p_err(), in
order to produce a single JSON field.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/bpf/bpftool/common.c | 26 +++++++-------
 tools/bpf/bpftool/main.c   | 16 ++++-----
 tools/bpf/bpftool/main.h   | 37 +++++++++++++++++---
 tools/bpf/bpftool/map.c    | 87 +++++++++++++++++++++++++---------------------
 tools/bpf/bpftool/prog.c   | 51 +++++++++++++--------------
 5 files changed, 126 insertions(+), 91 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index e7a756b8ee21..b2533f1cae3e 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -66,8 +66,8 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
 
 	fd = bpf_obj_get(path);
 	if (fd < 0) {
-		err("bpf obj get (%s): %s\n", path,
-		    errno == EACCES && !is_bpffs(dirname(path)) ?
+		p_err("bpf obj get (%s): %s", path,
+		      errno == EACCES && !is_bpffs(dirname(path)) ?
 		    "directory not in bpf file system (bpffs)" :
 		    strerror(errno));
 		return -1;
@@ -79,7 +79,7 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
 		return type;
 	}
 	if (type != exp_type) {
-		err("incorrect object type: %s\n", get_fd_type_name(type));
+		p_err("incorrect object type: %s", get_fd_type_name(type));
 		close(fd);
 		return -1;
 	}
@@ -95,14 +95,14 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
 	int fd;
 
 	if (!is_prefix(*argv, "id")) {
-		err("expected 'id' got %s\n", *argv);
+		p_err("expected 'id' got %s", *argv);
 		return -1;
 	}
 	NEXT_ARG();
 
 	id = strtoul(*argv, &endptr, 0);
 	if (*endptr) {
-		err("can't parse %s as ID\n", *argv);
+		p_err("can't parse %s as ID", *argv);
 		return -1;
 	}
 	NEXT_ARG();
@@ -112,15 +112,15 @@ int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32))
 
 	fd = get_fd_by_id(id);
 	if (fd < 0) {
-		err("can't get prog by id (%u): %s\n", id, strerror(errno));
+		p_err("can't get prog by id (%u): %s", id, strerror(errno));
 		return -1;
 	}
 
 	err = bpf_obj_pin(fd, *argv);
 	close(fd);
 	if (err) {
-		err("can't pin the object (%s): %s\n", *argv,
-		    errno == EACCES && !is_bpffs(dirname(*argv)) ?
+		p_err("can't pin the object (%s): %s", *argv,
+		      errno == EACCES && !is_bpffs(dirname(*argv)) ?
 		    "directory not in bpf file system (bpffs)" :
 		    strerror(errno));
 		return -1;
@@ -153,11 +153,11 @@ int get_fd_type(int fd)
 
 	n = readlink(path, buf, sizeof(buf));
 	if (n < 0) {
-		err("can't read link type: %s\n", strerror(errno));
+		p_err("can't read link type: %s", strerror(errno));
 		return -1;
 	}
 	if (n == sizeof(path)) {
-		err("can't read link type: path too long!\n");
+		p_err("can't read link type: path too long!");
 		return -1;
 	}
 
@@ -181,7 +181,7 @@ char *get_fdinfo(int fd, const char *key)
 
 	fdi = fopen(path, "r");
 	if (!fdi) {
-		err("can't open fdinfo: %s\n", strerror(errno));
+		p_err("can't open fdinfo: %s", strerror(errno));
 		return NULL;
 	}
 
@@ -196,7 +196,7 @@ char *get_fdinfo(int fd, const char *key)
 
 		value = strchr(line, '\t');
 		if (!value || !value[1]) {
-			err("malformed fdinfo!?\n");
+			p_err("malformed fdinfo!?");
 			free(line);
 			return NULL;
 		}
@@ -209,7 +209,7 @@ char *get_fdinfo(int fd, const char *key)
 		return line;
 	}
 
-	err("key '%s' not found in fdinfo\n", key);
+	p_err("key '%s' not found in fdinfo", key);
 	free(line);
 	fclose(fdi);
 	return NULL;
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 71b01bf73912..9989a77fdc4a 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -158,20 +158,20 @@ static int do_batch(int argc, char **argv)
 	int i;
 
 	if (argc < 2) {
-		err("too few parameters for batch\n");
+		p_err("too few parameters for batch");
 		return -1;
 	} else if (!is_prefix(*argv, "file")) {
-		err("expected 'file', got: %s\n", *argv);
+		p_err("expected 'file', got: %s", *argv);
 		return -1;
 	} else if (argc > 2) {
-		err("too many parameters for batch\n");
+		p_err("too many parameters for batch");
 		return -1;
 	}
 	NEXT_ARG();
 
 	fp = fopen(*argv, "r");
 	if (!fp) {
-		err("Can't open file (%s): %s\n", *argv, strerror(errno));
+		p_err("Can't open file (%s): %s", *argv, strerror(errno));
 		return -1;
 	}
 
@@ -189,8 +189,8 @@ static int do_batch(int argc, char **argv)
 		while (n_argv[n_argc]) {
 			n_argc++;
 			if (n_argc == ARRAY_SIZE(n_argv)) {
-				err("line %d has too many arguments, skip\n",
-				    lines);
+				p_err("line %d has too many arguments, skip",
+				      lines);
 				n_argc = 0;
 				break;
 			}
@@ -225,7 +225,7 @@ static int do_batch(int argc, char **argv)
 		perror("reading batch file failed");
 		err = -1;
 	} else {
-		info("processed %d lines\n", lines);
+		p_info("processed %d lines", lines);
 		err = 0;
 	}
 err_close:
@@ -279,7 +279,7 @@ int main(int argc, char **argv)
 	if (json_output) {
 		json_wtr = jsonw_new(stdout);
 		if (!json_wtr) {
-			err("failed to create JSON writer\n");
+			p_err("failed to create JSON writer");
 			return -1;
 		}
 		jsonw_pretty(json_wtr, pretty_output);
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 693fc9710be1..04c88b55d8c7 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -45,15 +45,11 @@
 
 #include "json_writer.h"
 
-#define err(msg...)	fprintf(stderr, "Error: " msg)
-#define warn(msg...)	fprintf(stderr, "Warning: " msg)
-#define info(msg...)	fprintf(stderr, msg)
-
 #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
 
 #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
 #define NEXT_ARGP()	({ (*argc)--; (*argv)++; if (*argc < 0) usage(); })
-#define BAD_ARG()	({ err("what is '%s'?\n", *argv); -1; })
+#define BAD_ARG()	({ p_err("what is '%s'?\n", *argv); -1; })
 
 #define BPF_TAG_FMT	"%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
 
@@ -97,4 +93,35 @@ int prog_parse_fd(int *argc, char ***argv);
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes);
 void print_hex_data_json(uint8_t *data, size_t len);
 
+static inline void p_err(const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	if (json_output) {
+		jsonw_start_object(json_wtr);
+		jsonw_name(json_wtr, "error");
+		jsonw_vprintf_enquote(json_wtr, fmt, ap);
+		jsonw_end_object(json_wtr);
+	} else {
+		fprintf(stderr, "Error: ");
+		vfprintf(stderr, fmt, ap);
+		fprintf(stderr, "\n");
+	}
+	va_end(ap);
+}
+
+static inline void p_info(const char *fmt, ...)
+{
+	va_list ap;
+
+	if (json_output)
+		return;
+
+	va_start(ap, fmt);
+	vfprintf(stderr, fmt, ap);
+	fprintf(stderr, "\n");
+	va_end(ap);
+}
+
 #endif
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 14d89bfabc66..86c128c433ba 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -81,19 +81,19 @@ static unsigned int get_possible_cpus(void)
 
 	fd = open("/sys/devices/system/cpu/possible", O_RDONLY);
 	if (fd < 0) {
-		err("can't open sysfs possible cpus\n");
+		p_err("can't open sysfs possible cpus");
 		exit(-1);
 	}
 
 	n = read(fd, buf, sizeof(buf));
 	if (n < 2) {
-		err("can't read sysfs possible cpus\n");
+		p_err("can't read sysfs possible cpus");
 		exit(-1);
 	}
 	close(fd);
 
 	if (n == sizeof(buf)) {
-		err("read sysfs possible cpus overflow\n");
+		p_err("read sysfs possible cpus overflow");
 		exit(-1);
 	}
 
@@ -161,14 +161,14 @@ static int map_parse_fd(int *argc, char ***argv)
 
 		id = strtoul(**argv, &endptr, 0);
 		if (*endptr) {
-			err("can't parse %s as ID\n", **argv);
+			p_err("can't parse %s as ID", **argv);
 			return -1;
 		}
 		NEXT_ARGP();
 
 		fd = bpf_map_get_fd_by_id(id);
 		if (fd < 0)
-			err("get map by id (%u): %s\n", id, strerror(errno));
+			p_err("get map by id (%u): %s", id, strerror(errno));
 		return fd;
 	} else if (is_prefix(**argv, "pinned")) {
 		char *path;
@@ -181,7 +181,7 @@ static int map_parse_fd(int *argc, char ***argv)
 		return open_obj_pinned_any(path, BPF_OBJ_MAP);
 	}
 
-	err("expected 'id' or 'pinned', got: '%s'?\n", **argv);
+	p_err("expected 'id' or 'pinned', got: '%s'?", **argv);
 	return -1;
 }
 
@@ -197,7 +197,7 @@ map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len)
 
 	err = bpf_obj_get_info_by_fd(fd, info, info_len);
 	if (err) {
-		err("can't get map info: %s\n", strerror(errno));
+		p_err("can't get map info: %s", strerror(errno));
 		close(fd);
 		return err;
 	}
@@ -288,14 +288,14 @@ static char **parse_bytes(char **argv, const char *name, unsigned char *val,
 	while (i < n && argv[i]) {
 		val[i] = strtoul(argv[i], &endptr, 0);
 		if (*endptr) {
-			err("error parsing byte: %s\n", argv[i]);
+			p_err("error parsing byte: %s", argv[i]);
 			return NULL;
 		}
 		i++;
 	}
 
 	if (i != n) {
-		err("%s expected %d bytes got %d\n", name, n, i);
+		p_err("%s expected %d bytes got %d", name, n, i);
 		return NULL;
 	}
 
@@ -309,16 +309,16 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 	if (!*argv) {
 		if (!key && !value)
 			return 0;
-		err("did not find %s\n", key ? "key" : "value");
+		p_err("did not find %s", key ? "key" : "value");
 		return -1;
 	}
 
 	if (is_prefix(*argv, "key")) {
 		if (!key) {
 			if (key_size)
-				err("duplicate key\n");
+				p_err("duplicate key");
 			else
-				err("unnecessary key\n");
+				p_err("unnecessary key");
 			return -1;
 		}
 
@@ -333,9 +333,9 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 
 		if (!value) {
 			if (value_size)
-				err("duplicate value\n");
+				p_err("duplicate value");
 			else
-				err("unnecessary value\n");
+				p_err("unnecessary value");
 			return -1;
 		}
 
@@ -345,11 +345,11 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 			int argc = 2;
 
 			if (value_size != 4) {
-				err("value smaller than 4B for map in map?\n");
+				p_err("value smaller than 4B for map in map?");
 				return -1;
 			}
 			if (!argv[0] || !argv[1]) {
-				err("not enough value arguments for map in map\n");
+				p_err("not enough value arguments for map in map");
 				return -1;
 			}
 
@@ -363,11 +363,11 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 			int argc = 2;
 
 			if (value_size != 4) {
-				err("value smaller than 4B for map of progs?\n");
+				p_err("value smaller than 4B for map of progs?");
 				return -1;
 			}
 			if (!argv[0] || !argv[1]) {
-				err("not enough value arguments for map of progs\n");
+				p_err("not enough value arguments for map of progs");
 				return -1;
 			}
 
@@ -388,7 +388,7 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 	} else if (is_prefix(*argv, "any") || is_prefix(*argv, "noexist") ||
 		   is_prefix(*argv, "exist")) {
 		if (!flags) {
-			err("flags specified multiple times: %s\n", *argv);
+			p_err("flags specified multiple times: %s", *argv);
 			return -1;
 		}
 
@@ -403,7 +403,7 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 				  value_size, NULL, value_fd);
 	}
 
-	err("expected key or value, got: %s\n", *argv);
+	p_err("expected key or value, got: %s", *argv);
 	return -1;
 }
 
@@ -499,22 +499,21 @@ static int do_show(int argc, char **argv)
 		if (err) {
 			if (errno == ENOENT)
 				break;
-			err("can't get next map: %s\n", strerror(errno));
-			if (errno == EINVAL)
-				err("kernel too old?\n");
+			p_err("can't get next map: %s%s", strerror(errno),
+			      errno == EINVAL ? " -- kernel too old?" : "");
 			return -1;
 		}
 
 		fd = bpf_map_get_fd_by_id(id);
 		if (fd < 0) {
-			err("can't get map by id (%u): %s\n",
-			    id, strerror(errno));
+			p_err("can't get map by id (%u): %s",
+			      id, strerror(errno));
 			return -1;
 		}
 
 		err = bpf_obj_get_info_by_fd(fd, &info, &len);
 		if (err) {
-			err("can't get map info: %s\n", strerror(errno));
+			p_err("can't get map info: %s", strerror(errno));
 			close(fd);
 			return -1;
 		}
@@ -547,7 +546,7 @@ static int do_dump(int argc, char **argv)
 		return -1;
 
 	if (map_is_map_of_maps(info.type) || map_is_map_of_progs(info.type)) {
-		err("Dumping maps of maps and program maps not supported\n");
+		p_err("Dumping maps of maps and program maps not supported");
 		close(fd);
 		return -1;
 	}
@@ -555,7 +554,7 @@ static int do_dump(int argc, char **argv)
 	key = malloc(info.key_size);
 	value = alloc_value(&info);
 	if (!key || !value) {
-		err("mem alloc failed\n");
+		p_err("mem alloc failed");
 		err = -1;
 		goto exit_free;
 	}
@@ -577,9 +576,19 @@ static int do_dump(int argc, char **argv)
 			else
 				print_entry_plain(&info, key, value);
 		} else {
-			info("can't lookup element with key: ");
-			fprint_hex(stderr, key, info.key_size, " ");
-			fprintf(stderr, "\n");
+			if (json_output) {
+				jsonw_name(json_wtr, "key");
+				print_hex_data_json(key, info.key_size);
+				jsonw_name(json_wtr, "value");
+				jsonw_start_object(json_wtr);
+				jsonw_string_field(json_wtr, "error",
+						   "can't lookup element");
+				jsonw_end_object(json_wtr);
+			} else {
+				p_info("can't lookup element with key: ");
+				fprint_hex(stderr, key, info.key_size, " ");
+				fprintf(stderr, "\n");
+			}
 		}
 
 		prev_key = key;
@@ -619,7 +628,7 @@ static int do_update(int argc, char **argv)
 	key = malloc(info.key_size);
 	value = alloc_value(&info);
 	if (!key || !value) {
-		err("mem alloc failed");
+		p_err("mem alloc failed");
 		err = -1;
 		goto exit_free;
 	}
@@ -631,7 +640,7 @@ static int do_update(int argc, char **argv)
 
 	err = bpf_map_update_elem(fd, key, value, flags);
 	if (err) {
-		err("update failed: %s\n", strerror(errno));
+		p_err("update failed: %s", strerror(errno));
 		goto exit_free;
 	}
 
@@ -663,7 +672,7 @@ static int do_lookup(int argc, char **argv)
 	key = malloc(info.key_size);
 	value = alloc_value(&info);
 	if (!key || !value) {
-		err("mem alloc failed");
+		p_err("mem alloc failed");
 		err = -1;
 		goto exit_free;
 	}
@@ -687,7 +696,7 @@ static int do_lookup(int argc, char **argv)
 			printf("\n\nNot found\n");
 		}
 	} else {
-		err("lookup failed: %s\n", strerror(errno));
+		p_err("lookup failed: %s", strerror(errno));
 	}
 
 exit_free:
@@ -716,7 +725,7 @@ static int do_getnext(int argc, char **argv)
 	key = malloc(info.key_size);
 	nextkey = malloc(info.key_size);
 	if (!key || !nextkey) {
-		err("mem alloc failed");
+		p_err("mem alloc failed");
 		err = -1;
 		goto exit_free;
 	}
@@ -733,7 +742,7 @@ static int do_getnext(int argc, char **argv)
 
 	err = bpf_map_get_next_key(fd, key, nextkey);
 	if (err) {
-		err("can't get next key: %s\n", strerror(errno));
+		p_err("can't get next key: %s", strerror(errno));
 		goto exit_free;
 	}
 
@@ -786,7 +795,7 @@ static int do_delete(int argc, char **argv)
 
 	key = malloc(info.key_size);
 	if (!key) {
-		err("mem alloc failed");
+		p_err("mem alloc failed");
 		err = -1;
 		goto exit_free;
 	}
@@ -797,7 +806,7 @@ static int do_delete(int argc, char **argv)
 
 	err = bpf_map_delete_elem(fd, key);
 	if (err)
-		err("delete failed: %s\n", strerror(errno));
+		p_err("delete failed: %s", strerror(errno));
 
 exit_free:
 	free(key);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 43e49799a624..41bd5390b4fc 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -104,21 +104,21 @@ static int prog_fd_by_tag(unsigned char *tag)
 	while (true) {
 		err = bpf_prog_get_next_id(id, &id);
 		if (err) {
-			err("%s\n", strerror(errno));
+			p_err("%s", strerror(errno));
 			return -1;
 		}
 
 		fd = bpf_prog_get_fd_by_id(id);
 		if (fd < 0) {
-			err("can't get prog by id (%u): %s\n",
-			    id, strerror(errno));
+			p_err("can't get prog by id (%u): %s",
+			      id, strerror(errno));
 			return -1;
 		}
 
 		err = bpf_obj_get_info_by_fd(fd, &info, &len);
 		if (err) {
-			err("can't get prog info (%u): %s\n",
-			    id, strerror(errno));
+			p_err("can't get prog info (%u): %s",
+			      id, strerror(errno));
 			close(fd);
 			return -1;
 		}
@@ -142,14 +142,14 @@ int prog_parse_fd(int *argc, char ***argv)
 
 		id = strtoul(**argv, &endptr, 0);
 		if (*endptr) {
-			err("can't parse %s as ID\n", **argv);
+			p_err("can't parse %s as ID", **argv);
 			return -1;
 		}
 		NEXT_ARGP();
 
 		fd = bpf_prog_get_fd_by_id(id);
 		if (fd < 0)
-			err("get by id (%u): %s\n", id, strerror(errno));
+			p_err("get by id (%u): %s", id, strerror(errno));
 		return fd;
 	} else if (is_prefix(**argv, "tag")) {
 		unsigned char tag[BPF_TAG_SIZE];
@@ -159,7 +159,7 @@ int prog_parse_fd(int *argc, char ***argv)
 		if (sscanf(**argv, BPF_TAG_FMT, tag, tag + 1, tag + 2,
 			   tag + 3, tag + 4, tag + 5, tag + 6, tag + 7)
 		    != BPF_TAG_SIZE) {
-			err("can't parse tag\n");
+			p_err("can't parse tag");
 			return -1;
 		}
 		NEXT_ARGP();
@@ -176,7 +176,7 @@ int prog_parse_fd(int *argc, char ***argv)
 		return open_obj_pinned_any(path, BPF_OBJ_PROG);
 	}
 
-	err("expected 'id', 'tag' or 'pinned', got: '%s'?\n", **argv);
+	p_err("expected 'id', 'tag' or 'pinned', got: '%s'?", **argv);
 	return -1;
 }
 
@@ -311,7 +311,7 @@ static int show_prog(int fd)
 
 	err = bpf_obj_get_info_by_fd(fd, &info, &len);
 	if (err) {
-		err("can't get prog info: %s\n", strerror(errno));
+		p_err("can't get prog info: %s", strerror(errno));
 		return -1;
 	}
 
@@ -349,17 +349,16 @@ static int do_show(int argc, char **argv)
 				err = 0;
 				break;
 			}
-			err("can't get next program: %s\n", strerror(errno));
-			if (errno == EINVAL)
-				err("kernel too old?\n");
+			p_err("can't get next program: %s%s", strerror(errno),
+			      errno == EINVAL ? " -- kernel too old?" : "");
 			err = -1;
 			break;
 		}
 
 		fd = bpf_prog_get_fd_by_id(id);
 		if (fd < 0) {
-			err("can't get prog by id (%u): %s\n",
-			    id, strerror(errno));
+			p_err("can't get prog by id (%u): %s",
+			      id, strerror(errno));
 			err = -1;
 			break;
 		}
@@ -498,7 +497,7 @@ static int do_dump(int argc, char **argv)
 		member_len = &info.xlated_prog_len;
 		member_ptr = &info.xlated_prog_insns;
 	} else {
-		err("expected 'xlated' or 'jited', got: %s\n", *argv);
+		p_err("expected 'xlated' or 'jited', got: %s", *argv);
 		return -1;
 	}
 	NEXT_ARG();
@@ -513,7 +512,7 @@ static int do_dump(int argc, char **argv)
 	if (is_prefix(*argv, "file")) {
 		NEXT_ARG();
 		if (!argc) {
-			err("expected file path\n");
+			p_err("expected file path");
 			return -1;
 		}
 
@@ -531,12 +530,12 @@ static int do_dump(int argc, char **argv)
 
 	err = bpf_obj_get_info_by_fd(fd, &info, &len);
 	if (err) {
-		err("can't get prog info: %s\n", strerror(errno));
+		p_err("can't get prog info: %s", strerror(errno));
 		return -1;
 	}
 
 	if (!*member_len) {
-		info("no instructions returned\n");
+		p_info("no instructions returned");
 		close(fd);
 		return 0;
 	}
@@ -545,7 +544,7 @@ static int do_dump(int argc, char **argv)
 
 	buf = malloc(buf_size);
 	if (!buf) {
-		err("mem alloc failed\n");
+		p_err("mem alloc failed");
 		close(fd);
 		return -1;
 	}
@@ -558,28 +557,28 @@ static int do_dump(int argc, char **argv)
 	err = bpf_obj_get_info_by_fd(fd, &info, &len);
 	close(fd);
 	if (err) {
-		err("can't get prog info: %s\n", strerror(errno));
+		p_err("can't get prog info: %s", strerror(errno));
 		goto err_free;
 	}
 
 	if (*member_len > buf_size) {
-		err("too many instructions returned\n");
+		p_err("too many instructions returned");
 		goto err_free;
 	}
 
 	if (filepath) {
 		fd = open(filepath, O_WRONLY | O_CREAT | O_TRUNC, 0600);
 		if (fd < 0) {
-			err("can't open file %s: %s\n", filepath,
-			    strerror(errno));
+			p_err("can't open file %s: %s", filepath,
+			      strerror(errno));
 			goto err_free;
 		}
 
 		n = write(fd, buf, *member_len);
 		close(fd);
 		if (n != *member_len) {
-			err("error writing output file: %s\n",
-			    n < 0 ? strerror(errno) : "short write");
+			p_err("error writing output file: %s",
+			      n < 0 ? strerror(errno) : "short write");
 			goto err_free;
 		}
 	} else {
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 08/12] tools: bpftool: add JSON output for `bpftool batch file FILE` command
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

`bpftool batch file FILE` takes FILE as an argument and executes all the
bpftool commands it finds inside (or stops if an error occurs).

To obtain a consistent JSON output, create a root JSON array, then for
each command create a new object containing two fields: one with the
command arguments, the other with the output (which is the JSON object
that the command would have produced, if called on its own).

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/main.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 14bfc17cd4de..71b01bf73912 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -155,6 +155,7 @@ static int do_batch(int argc, char **argv)
 	int n_argc;
 	FILE *fp;
 	int err;
+	int i;
 
 	if (argc < 2) {
 		err("too few parameters for batch\n");
@@ -174,6 +175,8 @@ static int do_batch(int argc, char **argv)
 		return -1;
 	}
 
+	if (json_output)
+		jsonw_start_array(json_wtr);
 	while (fgets(buf, sizeof(buf), fp)) {
 		if (strlen(buf) == sizeof(buf) - 1) {
 			errno = E2BIG;
@@ -197,7 +200,21 @@ static int do_batch(int argc, char **argv)
 		if (!n_argc)
 			continue;
 
+		if (json_output) {
+			jsonw_start_object(json_wtr);
+			jsonw_name(json_wtr, "command");
+			jsonw_start_array(json_wtr);
+			for (i = 0; i < n_argc; i++)
+				jsonw_string(json_wtr, n_argv[i]);
+			jsonw_end_array(json_wtr);
+			jsonw_name(json_wtr, "output");
+		}
+
 		err = cmd_select(cmds, n_argc, n_argv, do_help);
+
+		if (json_output)
+			jsonw_end_object(json_wtr);
+
 		if (err)
 			goto err_close;
 
@@ -214,6 +231,9 @@ static int do_batch(int argc, char **argv)
 err_close:
 	fclose(fp);
 
+	if (json_output)
+		jsonw_end_array(json_wtr);
+
 	return err;
 }
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 06/12] tools: bpftool: add JSON output for `bpftool prog dump xlated *` command
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Add a new printing function to dump translated eBPF instructions as
JSON. As for plain output, opcodes are printed only on request (when
`opcodes` is provided on the command line).

The disassembled output is generated by the same code that is used by
the kernel verifier.

Example output:

    $ bpftool --json --pretty prog dump xlated id 1
    [{
            "disasm": "(bf) r6 = r1"
        },{
            "disasm": "(61) r7 = *(u32 *)(r6 +16)"
        },{
            "disasm": "(95) exit"
        }
    ]

    $ bpftool --json --pretty prog dump xlated id 1 opcodes
    [{
            "disasm": "(bf) r6 = r1",
            "opcodes": {
                "code": "0xbf",
                "src_reg": "0x1",
                "dst_reg": "0x6",
                "off": ["0x00","0x00"
                ],
                "imm": ["0x00","0x00","0x00","0x00"
                ]
            }
        },{
            "disasm": "(61) r7 = *(u32 *)(r6 +16)",
            "opcodes": {
                "code": "0x61",
                "src_reg": "0x6",
                "dst_reg": "0x7",
                "off": ["0x10","0x00"
                ],
                "imm": ["0x00","0x00","0x00","0x00"
                ]
            }
        },{
            "disasm": "(95) exit",
            "opcodes": {
                "code": "0x95",
                "src_reg": "0x0",
                "dst_reg": "0x0",
                "off": ["0x00","0x00"
                ],
                "imm": ["0x00","0x00","0x00","0x00"
                ]
            }
        }
    ]

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/common.c      | 10 ++++++
 tools/bpf/bpftool/json_writer.c |  8 +++++
 tools/bpf/bpftool/json_writer.h |  2 ++
 tools/bpf/bpftool/main.h        |  1 +
 tools/bpf/bpftool/prog.c        | 70 +++++++++++++++++++++++++++++++++++++++--
 5 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index df8396a0c400..e7a756b8ee21 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -214,3 +214,13 @@ char *get_fdinfo(int fd, const char *key)
 	fclose(fdi);
 	return NULL;
 }
+
+void print_hex_data_json(uint8_t *data, size_t len)
+{
+	unsigned int i;
+
+	jsonw_start_array(json_wtr);
+	for (i = 0; i < len; i++)
+		jsonw_printf(json_wtr, "\"0x%02hhx\"", data[i]);
+	jsonw_end_array(json_wtr);
+}
diff --git a/tools/bpf/bpftool/json_writer.c b/tools/bpf/bpftool/json_writer.c
index 6b77d288cce2..c6eef76322ae 100644
--- a/tools/bpf/bpftool/json_writer.c
+++ b/tools/bpf/bpftool/json_writer.c
@@ -156,6 +156,14 @@ void jsonw_name(json_writer_t *self, const char *name)
 		putc(' ', self->out);
 }
 
+void jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, va_list ap)
+{
+	jsonw_eor(self);
+	putc('"', self->out);
+	vfprintf(self->out, fmt, ap);
+	putc('"', self->out);
+}
+
 void jsonw_printf(json_writer_t *self, const char *fmt, ...)
 {
 	va_list ap;
diff --git a/tools/bpf/bpftool/json_writer.h b/tools/bpf/bpftool/json_writer.h
index 1516aafba59d..0fa2fb1b6351 100644
--- a/tools/bpf/bpftool/json_writer.h
+++ b/tools/bpf/bpftool/json_writer.h
@@ -17,6 +17,7 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include <stdarg.h>
 
 /* Opaque class structure */
 typedef struct json_writer json_writer_t;
@@ -33,6 +34,7 @@ void jsonw_pretty(json_writer_t *self, bool on);
 void jsonw_name(json_writer_t *self, const char *name);
 
 /* Add value  */
+void jsonw_vprintf_enquote(json_writer_t *self, const char *fmt, va_list ap);
 void jsonw_printf(json_writer_t *self, const char *fmt, ...);
 void jsonw_string(json_writer_t *self, const char *value);
 void jsonw_bool(json_writer_t *self, bool value);
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 15927fc9fb31..693fc9710be1 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -95,5 +95,6 @@ int do_map(int argc, char **arg);
 int prog_parse_fd(int *argc, char ***argv);
 
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes);
+void print_hex_data_json(uint8_t *data, size_t len);
 
 #endif
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index f373f2baef5a..43e49799a624 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -385,7 +385,7 @@ static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
 	va_end(args);
 }
 
-static void dump_xlated(void *buf, unsigned int len, bool opcodes)
+static void dump_xlated_plain(void *buf, unsigned int len, bool opcodes)
 {
 	struct bpf_insn *insn = buf;
 	bool double_insn = false;
@@ -414,6 +414,69 @@ static void dump_xlated(void *buf, unsigned int len, bool opcodes)
 	}
 }
 
+static void print_insn_json(struct bpf_verifier_env *env, const char *fmt, ...)
+{
+	unsigned int l = strlen(fmt);
+	char chomped_fmt[l];
+	va_list args;
+
+	va_start(args, fmt);
+	if (l > 0) {
+		strncpy(chomped_fmt, fmt, l - 1);
+		chomped_fmt[l - 1] = '\0';
+	}
+	jsonw_vprintf_enquote(json_wtr, chomped_fmt, args);
+	va_end(args);
+}
+
+static void dump_xlated_json(void *buf, unsigned int len, bool opcodes)
+{
+	struct bpf_insn *insn = buf;
+	bool double_insn = false;
+	unsigned int i;
+
+	jsonw_start_array(json_wtr);
+	for (i = 0; i < len / sizeof(*insn); i++) {
+		if (double_insn) {
+			double_insn = false;
+			continue;
+		}
+		double_insn = insn[i].code == (BPF_LD | BPF_IMM | BPF_DW);
+
+		jsonw_start_object(json_wtr);
+		jsonw_name(json_wtr, "disasm");
+		print_bpf_insn(print_insn_json, NULL, insn + i, true);
+
+		if (opcodes) {
+			jsonw_name(json_wtr, "opcodes");
+			jsonw_start_object(json_wtr);
+
+			jsonw_name(json_wtr, "code");
+			jsonw_printf(json_wtr, "\"0x%02hhx\"", insn[i].code);
+
+			jsonw_name(json_wtr, "src_reg");
+			jsonw_printf(json_wtr, "\"0x%hhx\"", insn[i].src_reg);
+
+			jsonw_name(json_wtr, "dst_reg");
+			jsonw_printf(json_wtr, "\"0x%hhx\"", insn[i].dst_reg);
+
+			jsonw_name(json_wtr, "off");
+			print_hex_data_json((uint8_t *)(&insn[i].off), 2);
+
+			jsonw_name(json_wtr, "imm");
+			if (double_insn && i < len - 1)
+				print_hex_data_json((uint8_t *)(&insn[i].imm),
+						    12);
+			else
+				print_hex_data_json((uint8_t *)(&insn[i].imm),
+						    4);
+			jsonw_end_object(json_wtr);
+		}
+		jsonw_end_object(json_wtr);
+	}
+	jsonw_end_array(json_wtr);
+}
+
 static int do_dump(int argc, char **argv)
 {
 	struct bpf_prog_info info = {};
@@ -523,7 +586,10 @@ static int do_dump(int argc, char **argv)
 		if (member_len == &info.jited_prog_len)
 			disasm_print_insn(buf, *member_len, opcodes);
 		else
-			dump_xlated(buf, *member_len, opcodes);
+			if (json_output)
+				dump_xlated_json(buf, *member_len, opcodes);
+			else
+				dump_xlated_plain(buf, *member_len, opcodes);
 	}
 
 	free(buf);
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 07/12] tools: bpftool: add JSON output for `bpftool map *` commands
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Reuse the json_writer API introduced in an earlier commit to make
bpftool able to generate JSON output on
`bpftool map { show | dump | lookup | getnext }` commands. Remaining
commands produce no output.

Some functions have been spit into plain-output and JSON versions in
order to remain readable.

Outputs for sample maps have been successfully tested against a JSON
validator.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/map.c | 149 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 129 insertions(+), 20 deletions(-)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index e1004d825392..14d89bfabc66 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -205,8 +205,45 @@ map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len)
 	return fd;
 }
 
-static void print_entry(struct bpf_map_info *info, unsigned char *key,
-			unsigned char *value)
+static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
+			     unsigned char *value)
+{
+	jsonw_start_object(json_wtr);
+
+	if (!map_is_per_cpu(info->type)) {
+		jsonw_name(json_wtr, "key");
+		print_hex_data_json(key, info->key_size);
+		jsonw_name(json_wtr, "value");
+		print_hex_data_json(value, info->value_size);
+	} else {
+		unsigned int i, n;
+
+		n = get_possible_cpus();
+
+		jsonw_name(json_wtr, "key");
+		print_hex_data_json(key, info->key_size);
+
+		jsonw_name(json_wtr, "values");
+		jsonw_start_array(json_wtr);
+		for (i = 0; i < n; i++) {
+			jsonw_start_object(json_wtr);
+
+			jsonw_int_field(json_wtr, "cpu", i);
+
+			jsonw_name(json_wtr, "value");
+			print_hex_data_json(value + i * info->value_size,
+					    info->value_size);
+
+			jsonw_end_object(json_wtr);
+		}
+		jsonw_end_array(json_wtr);
+	}
+
+	jsonw_end_object(json_wtr);
+}
+
+static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
+			      unsigned char *value)
 {
 	if (!map_is_per_cpu(info->type)) {
 		bool single_line, break_names;
@@ -370,7 +407,41 @@ static int parse_elem(char **argv, struct bpf_map_info *info,
 	return -1;
 }
 
-static int show_map_close(int fd, struct bpf_map_info *info)
+static int show_map_close_json(int fd, struct bpf_map_info *info)
+{
+	char *memlock;
+
+	memlock = get_fdinfo(fd, "memlock");
+	close(fd);
+
+	jsonw_start_object(json_wtr);
+
+	jsonw_uint_field(json_wtr, "id", info->id);
+	if (info->type < ARRAY_SIZE(map_type_name))
+		jsonw_string_field(json_wtr, "type",
+				   map_type_name[info->type]);
+	else
+		jsonw_uint_field(json_wtr, "type", info->type);
+
+	if (*info->name)
+		jsonw_string_field(json_wtr, "name", info->name);
+
+	jsonw_name(json_wtr, "flags");
+	jsonw_printf(json_wtr, "%#x", info->map_flags);
+	jsonw_uint_field(json_wtr, "bytes_key", info->key_size);
+	jsonw_uint_field(json_wtr, "bytes_value", info->value_size);
+	jsonw_uint_field(json_wtr, "max_entries", info->max_entries);
+
+	if (memlock)
+		jsonw_int_field(json_wtr, "bytes_memlock", atoi(memlock));
+	free(memlock);
+
+	jsonw_end_object(json_wtr);
+
+	return 0;
+}
+
+static int show_map_close_plain(int fd, struct bpf_map_info *info)
 {
 	char *memlock;
 
@@ -412,12 +483,17 @@ static int do_show(int argc, char **argv)
 		if (fd < 0)
 			return -1;
 
-		return show_map_close(fd, &info);
+		if (json_output)
+			return show_map_close_json(fd, &info);
+		else
+			return show_map_close_plain(fd, &info);
 	}
 
 	if (argc)
 		return BAD_ARG();
 
+	if (json_output)
+		jsonw_start_array(json_wtr);
 	while (true) {
 		err = bpf_map_get_next_id(id, &id);
 		if (err) {
@@ -443,8 +519,13 @@ static int do_show(int argc, char **argv)
 			return -1;
 		}
 
-		show_map_close(fd, &info);
+		if (json_output)
+			show_map_close_json(fd, &info);
+		else
+			show_map_close_plain(fd, &info);
 	}
+	if (json_output)
+		jsonw_end_array(json_wtr);
 
 	return errno == ENOENT ? 0 : -1;
 }
@@ -480,6 +561,8 @@ static int do_dump(int argc, char **argv)
 	}
 
 	prev_key = NULL;
+	if (json_output)
+		jsonw_start_array(json_wtr);
 	while (true) {
 		err = bpf_map_get_next_key(fd, prev_key, key);
 		if (err) {
@@ -489,7 +572,10 @@ static int do_dump(int argc, char **argv)
 		}
 
 		if (!bpf_map_lookup_elem(fd, key, value)) {
-			print_entry(&info, key, value);
+			if (json_output)
+				print_entry_json(&info, key, value);
+			else
+				print_entry_plain(&info, key, value);
 		} else {
 			info("can't lookup element with key: ");
 			fprint_hex(stderr, key, info.key_size, " ");
@@ -500,7 +586,11 @@ static int do_dump(int argc, char **argv)
 		num_elems++;
 	}
 
-	printf("Found %u element%s\n", num_elems, num_elems != 1 ? "s" : "");
+	if (json_output)
+		jsonw_end_array(json_wtr);
+	else
+		printf("Found %u element%s\n", num_elems,
+		       num_elems != 1 ? "s" : "");
 
 exit_free:
 	free(key);
@@ -584,11 +674,18 @@ static int do_lookup(int argc, char **argv)
 
 	err = bpf_map_lookup_elem(fd, key, value);
 	if (!err) {
-		print_entry(&info, key, value);
+		if (json_output)
+			print_entry_json(&info, key, value);
+		else
+			print_entry_plain(&info, key, value);
 	} else if (errno == ENOENT) {
-		printf("key:\n");
-		fprint_hex(stdout, key, info.key_size, " ");
-		printf("\n\nNot found\n");
+		if (json_output) {
+			jsonw_null(json_wtr);
+		} else {
+			printf("key:\n");
+			fprint_hex(stdout, key, info.key_size, " ");
+			printf("\n\nNot found\n");
+		}
 	} else {
 		err("lookup failed: %s\n", strerror(errno));
 	}
@@ -640,18 +737,30 @@ static int do_getnext(int argc, char **argv)
 		goto exit_free;
 	}
 
-	if (key) {
-		printf("key:\n");
-		fprint_hex(stdout, key, info.key_size, " ");
-		printf("\n");
+	if (json_output) {
+		jsonw_start_object(json_wtr);
+		if (key) {
+			jsonw_name(json_wtr, "key");
+			print_hex_data_json(key, info.key_size);
+		} else {
+			jsonw_null_field(json_wtr, "key");
+		}
+		jsonw_name(json_wtr, "next_key");
+		print_hex_data_json(nextkey, info.key_size);
+		jsonw_end_object(json_wtr);
 	} else {
-		printf("key: None\n");
+		if (key) {
+			printf("key:\n");
+			fprint_hex(stdout, key, info.key_size, " ");
+			printf("\n");
+		} else {
+			printf("key: None\n");
+		}
+		printf("next key:\n");
+		fprint_hex(stdout, nextkey, info.key_size, " ");
+		printf("\n");
 	}
 
-	printf("next key:\n");
-	fprint_hex(stdout, nextkey, info.key_size, " ");
-	printf("\n");
-
 exit_free:
 	free(nextkey);
 	free(key);
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 05/12] tools: bpftool: add JSON output for `bpftool prog dump jited *` command
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Reuse the json_writer API introduced in an earlier commit to make
bpftool able to generate JSON output on `bpftool prog show *` commands.
A new printing function is created to be passed as an argument to the
disassembler.

Similarly to plain output, opcodes are printed on request.

Outputs from sample programs have been successfully tested against a
JSON validator.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/jit_disasm.c | 86 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 80 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 70e480b59e9d..5937e134e408 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -10,6 +10,7 @@
  * Licensed under the GNU General Public License, version 2.0 (GPLv2)
  */
 
+#include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -21,6 +22,9 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include "json_writer.h"
+#include "main.h"
+
 static void get_exec_path(char *tpath, size_t size)
 {
 	ssize_t len;
@@ -39,6 +43,38 @@ static void get_exec_path(char *tpath, size_t size)
 	free(path);
 }
 
+static int oper_count;
+static int fprintf_json(void *out, const char *fmt, ...)
+{
+	va_list ap;
+	char *s;
+
+	va_start(ap, fmt);
+	if (!oper_count) {
+		int i;
+
+		s = va_arg(ap, char *);
+
+		/* Strip trailing spaces */
+		i = strlen(s) - 1;
+		while (s[i] == ' ')
+			s[i--] = '\0';
+
+		jsonw_string_field(json_wtr, "operation", s);
+		jsonw_name(json_wtr, "operands");
+		jsonw_start_array(json_wtr);
+		oper_count++;
+	} else if (!strcmp(fmt, ",")) {
+		   /* Skip */
+	} else {
+		s = va_arg(ap, char *);
+		jsonw_string(json_wtr, s);
+		oper_count++;
+	}
+	va_end(ap);
+	return 0;
+}
+
 void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes)
 {
 	disassembler_ftype disassemble;
@@ -57,7 +93,12 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes)
 	assert(bfdf);
 	assert(bfd_check_format(bfdf, bfd_object));
 
-	init_disassemble_info(&info, stdout, (fprintf_ftype) fprintf);
+	if (json_output)
+		init_disassemble_info(&info, stdout,
+				      (fprintf_ftype) fprintf_json);
+	else
+		init_disassemble_info(&info, stdout,
+				      (fprintf_ftype) fprintf);
 	info.arch = bfd_get_arch(bfdf);
 	info.mach = bfd_get_mach(bfdf);
 	info.buffer = image;
@@ -68,20 +109,53 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes)
 	disassemble = disassembler(bfdf);
 	assert(disassemble);
 
+	if (json_output)
+		jsonw_start_array(json_wtr);
 	do {
-		printf("%4x:\t", pc);
+		if (json_output) {
+			jsonw_start_object(json_wtr);
+			oper_count = 0;
+			jsonw_name(json_wtr, "pc");
+			jsonw_printf(json_wtr, "\"0x%x\"", pc);
+		} else {
+			printf("%4x:\t", pc);
+		}
 
 		count = disassemble(pc, &info);
+		if (json_output) {
+			/* Operand array, was started in fprintf_json. Before
+			 * that, make sure we have a _null_ value if no operand
+			 * other than operation code was present.
+			 */
+			if (oper_count == 1)
+				jsonw_null(json_wtr);
+			jsonw_end_array(json_wtr);
+		}
 
 		if (opcodes) {
-			printf("\n\t");
-			for (i = 0; i < count; ++i)
-				printf("%02x ", (uint8_t) image[pc + i]);
+			if (json_output) {
+				jsonw_name(json_wtr, "opcodes");
+				jsonw_start_array(json_wtr);
+				for (i = 0; i < count; ++i)
+					jsonw_printf(json_wtr, "\"0x%02hhx\"",
+						     (uint8_t)image[pc + i]);
+				jsonw_end_array(json_wtr);
+			} else {
+				printf("\n\t");
+				for (i = 0; i < count; ++i)
+					printf("%02x ",
+					       (uint8_t)image[pc + i]);
+			}
 		}
-		printf("\n");
+		if (json_output)
+			jsonw_end_object(json_wtr);
+		else
+			printf("\n");
 
 		pc += count;
 	} while (count > 0 && pc < len);
+	if (json_output)
+		jsonw_end_array(json_wtr);
 
 	bfd_close(bfdf);
 }
-- 
2.14.1

^ permalink raw reply related

* [PATCH net-next 04/12] tools: bpftool: add JSON output for `bpftool prog show *` command
From: Jakub Kicinski @ 2017-10-23 16:24 UTC (permalink / raw)
  To: netdev; +Cc: oss-drivers, alexei.starovoitov, daniel, Quentin Monnet
In-Reply-To: <20171023162416.32753-1-jakub.kicinski@netronome.com>

From: Quentin Monnet <quentin.monnet@netronome.com>

Reuse the json_writer API introduced in an earlier commit to make
bpftool able to generate JSON output on `bpftool prog show *` commands.

For readability, the code from show_prog() has been split into two
functions, one for plain output, one for JSON.

Outputs from sample programs have been successfully tested against a
JSON validator.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/prog.c | 139 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 107 insertions(+), 32 deletions(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 7838206a455b..f373f2baef5a 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -195,51 +195,100 @@ static void show_prog_maps(int fd, u32 num_maps)
 	if (err || !info.nr_map_ids)
 		return;
 
-	printf("  map_ids ");
-	for (i = 0; i < info.nr_map_ids; i++)
-		printf("%u%s", map_ids[i],
-		       i == info.nr_map_ids - 1 ? "" : ",");
+	if (json_output) {
+		jsonw_name(json_wtr, "map_ids");
+		jsonw_start_array(json_wtr);
+		for (i = 0; i < info.nr_map_ids; i++)
+			jsonw_uint(json_wtr, map_ids[i]);
+		jsonw_end_array(json_wtr);
+	} else {
+		printf("  map_ids ");
+		for (i = 0; i < info.nr_map_ids; i++)
+			printf("%u%s", map_ids[i],
+			       i == info.nr_map_ids - 1 ? "" : ",");
+	}
 }
 
-static int show_prog(int fd)
+static void print_prog_json(struct bpf_prog_info *info, int fd)
 {
-	struct bpf_prog_info info = {};
-	__u32 len = sizeof(info);
 	char *memlock;
-	int err;
 
-	err = bpf_obj_get_info_by_fd(fd, &info, &len);
-	if (err) {
-		err("can't get prog info: %s\n", strerror(errno));
-		return -1;
+	jsonw_start_object(json_wtr);
+	jsonw_uint_field(json_wtr, "id", info->id);
+	if (info->type < ARRAY_SIZE(prog_type_name))
+		jsonw_string_field(json_wtr, "type",
+				   prog_type_name[info->type]);
+	else
+		jsonw_uint_field(json_wtr, "type", info->type);
+
+	if (*info->name)
+		jsonw_string_field(json_wtr, "name", info->name);
+
+	jsonw_name(json_wtr, "tag");
+	jsonw_printf(json_wtr, "\"" BPF_TAG_FMT "\"",
+		     info->tag[0], info->tag[1], info->tag[2], info->tag[3],
+		     info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
+
+	if (info->load_time) {
+		char buf[32];
+
+		print_boot_time(info->load_time, buf, sizeof(buf));
+
+		/* Piggy back on load_time, since 0 uid is a valid one */
+		jsonw_string_field(json_wtr, "loaded_at", buf);
+		jsonw_uint_field(json_wtr, "uid", info->created_by_uid);
 	}
 
-	printf("%u: ", info.id);
-	if (info.type < ARRAY_SIZE(prog_type_name))
-		printf("%s  ", prog_type_name[info.type]);
+	jsonw_uint_field(json_wtr, "bytes_xlated", info->xlated_prog_len);
+
+	if (info->jited_prog_len) {
+		jsonw_bool_field(json_wtr, "jited", true);
+		jsonw_uint_field(json_wtr, "bytes_jited", info->jited_prog_len);
+	} else {
+		jsonw_bool_field(json_wtr, "jited", false);
+	}
+
+	memlock = get_fdinfo(fd, "memlock");
+	if (memlock)
+		jsonw_int_field(json_wtr, "bytes_memlock", atoi(memlock));
+	free(memlock);
+
+	if (info->nr_map_ids)
+		show_prog_maps(fd, info->nr_map_ids);
+
+	jsonw_end_object(json_wtr);
+}
+
+static void print_prog_plain(struct bpf_prog_info *info, int fd)
+{
+	char *memlock;
+
+	printf("%u: ", info->id);
+	if (info->type < ARRAY_SIZE(prog_type_name))
+		printf("%s  ", prog_type_name[info->type]);
 	else
-		printf("type %u  ", info.type);
+		printf("type %u  ", info->type);
 
-	if (*info.name)
-		printf("name %s  ", info.name);
+	if (*info->name)
+		printf("name %s  ", info->name);
 
 	printf("tag ");
-	fprint_hex(stdout, info.tag, BPF_TAG_SIZE, "");
+	fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
 	printf("\n");
 
-	if (info.load_time) {
+	if (info->load_time) {
 		char buf[32];
 
-		print_boot_time(info.load_time, buf, sizeof(buf));
+		print_boot_time(info->load_time, buf, sizeof(buf));
 
 		/* Piggy back on load_time, since 0 uid is a valid one */
-		printf("\tloaded_at %s  uid %u\n", buf, info.created_by_uid);
+		printf("\tloaded_at %s  uid %u\n", buf, info->created_by_uid);
 	}
 
-	printf("\txlated %uB", info.xlated_prog_len);
+	printf("\txlated %uB", info->xlated_prog_len);
 
-	if (info.jited_prog_len)
-		printf("  jited %uB", info.jited_prog_len);
+	if (info->jited_prog_len)
+		printf("  jited %uB", info->jited_prog_len);
 	else
 		printf("  not jited");
 
@@ -248,16 +297,35 @@ static int show_prog(int fd)
 		printf("  memlock %sB", memlock);
 	free(memlock);
 
-	if (info.nr_map_ids)
-		show_prog_maps(fd, info.nr_map_ids);
+	if (info->nr_map_ids)
+		show_prog_maps(fd, info->nr_map_ids);
 
 	printf("\n");
+}
+
+static int show_prog(int fd)
+{
+	struct bpf_prog_info info = {};
+	__u32 len = sizeof(info);
+	int err;
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &len);
+	if (err) {
+		err("can't get prog info: %s\n", strerror(errno));
+		return -1;
+	}
+
+	if (json_output)
+		print_prog_json(&info, fd);
+	else
+		print_prog_plain(&info, fd);
 
 	return 0;
 }
 
 static int do_show(int argc, char **argv)
-{	__u32 id = 0;
+{
+	__u32 id = 0;
 	int err;
 	int fd;
 
@@ -272,6 +340,8 @@ static int do_show(int argc, char **argv)
 	if (argc)
 		return BAD_ARG();
 
+	if (json_output)
+		jsonw_start_array(json_wtr);
 	while (true) {
 		err = bpf_prog_get_next_id(id, &id);
 		if (err) {
@@ -282,23 +352,28 @@ static int do_show(int argc, char **argv)
 			err("can't get next program: %s\n", strerror(errno));
 			if (errno == EINVAL)
 				err("kernel too old?\n");
-			return -1;
+			err = -1;
+			break;
 		}
 
 		fd = bpf_prog_get_fd_by_id(id);
 		if (fd < 0) {
 			err("can't get prog by id (%u): %s\n",
 			    id, strerror(errno));
-			return -1;
+			err = -1;
+			break;
 		}
 
 		err = show_prog(fd);
 		close(fd);
 		if (err)
-			return err;
+			break;
 	}
 
-	return 0;
+	if (json_output)
+		jsonw_end_array(json_wtr);
+
+	return err;
 }
 
 static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
-- 
2.14.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox