Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] sis900: add support for ethtool's EEPROM dump
From: David Miller @ 2019-07-26 21:27 UTC (permalink / raw)
  To: sergej.benilov; +Cc: venza, netdev, andrew
In-Reply-To: <20190725194806.17964-1-sergej.benilov@googlemail.com>

From: Sergej Benilov <sergej.benilov@googlemail.com>
Date: Thu, 25 Jul 2019 21:48:06 +0200

> Implement ethtool's EEPROM dump command (ethtool -e|--eeprom-dump).
> 
> Thx to Andrew Lunn for comments.
> 
> Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH bpf-next 4/9] libbpf: add libbpf_swap_print to get previous print func
From: Stanislav Fomichev @ 2019-07-26 21:28 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, andrii.nakryiko, kernel-team
In-Reply-To: <20190726203747.1124677-5-andriin@fb.com>

On 07/26, Andrii Nakryiko wrote:
> libbpf_swap_print allows to restore previously set print function.
> This is useful when running many independent test with one default print
> function, but overriding log verbosity for particular subset of tests.
Can we change the return type of libbpf_set_print instead and return
the old function from it? Will it break ABI?

> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/libbpf.c   | 8 ++++++++
>  tools/lib/bpf/libbpf.h   | 1 +
>  tools/lib/bpf/libbpf.map | 5 +++++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8741c39adb1c..0c254b6c9685 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -79,6 +79,14 @@ void libbpf_set_print(libbpf_print_fn_t fn)
>  	__libbpf_pr = fn;
>  }
>  
> +libbpf_print_fn_t libbpf_swap_print(libbpf_print_fn_t fn)
> +{
> +	libbpf_print_fn_t old_print_fn = __libbpf_pr;
> +
> +	__libbpf_pr = fn;
> +	return old_print_fn;
> +}
> +
>  __printf(2, 3)
>  void libbpf_print(enum libbpf_print_level level, const char *format, ...)
>  {
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5cbf459ece0b..4e0aa893571f 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -58,6 +58,7 @@ typedef int (*libbpf_print_fn_t)(enum libbpf_print_level level,
>  				 const char *, va_list ap);
>  
>  LIBBPF_API void libbpf_set_print(libbpf_print_fn_t fn);
> +LIBBPF_API libbpf_print_fn_t libbpf_swap_print(libbpf_print_fn_t fn);
>  
>  /* Hide internal to user */
>  struct bpf_object;
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index f9d316e873d8..e211c38ddc43 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -184,3 +184,8 @@ LIBBPF_0.0.4 {
>  		perf_buffer__new_raw;
>  		perf_buffer__poll;
>  } LIBBPF_0.0.3;
> +
> +LIBBPF_0.0.5 {
> +	global:
> +		libbpf_swap_print;
> +} LIBBPF_0.0.4;
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: phylink: flow control on fixed-link not working.
From: Russell King - ARM Linux admin @ 2019-07-26 21:28 UTC (permalink / raw)
  To: René van Dorst; +Cc: netdev
In-Reply-To: <20190719195226.Horde.MpIE5TFL_P5-pUU6V-K6R9J@www.vdorst.com>

On Fri, Jul 19, 2019 at 07:52:26PM +0000, René van Dorst wrote:
> Hi Russel,
> 
> If I use this patch below:
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 5d0af041b8f9..a6aebaa14338 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -216,6 +216,8 @@ static int phylink_parse_fixedlink(struct phylink *pl,
>                                pl->supported, true);
>         linkmode_zero(pl->supported);
>         phylink_set(pl->supported, MII);
> +       phylink_set(pl->supported, Pause);
> +       phylink_set(pl->supported, Asym_Pause);
>         if (s) {
>                 __set_bit(s->bit, pl->supported);
>         } else {
> 
> Which is similar thing also done in phylink_parse_mode().

Yep, that's what should be there - please submit as a fix patch, thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH bpf-next 6/9] selftests/bpf: abstract away test log output
From: Stanislav Fomichev @ 2019-07-26 21:31 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, andrii.nakryiko, kernel-team
In-Reply-To: <20190726203747.1124677-7-andriin@fb.com>

On 07/26, Andrii Nakryiko wrote:
> This patch changes how test output is printed out. By default, if test
> had no errors, the only output will be a single line with test number,
> name, and verdict at the end, e.g.:
> 
>   #31 xdp:OK
> 
> If test had any errors, all log output captured during test execution
> will be output after test completes.
> 
> It's possible to force output of log with `-v` (`--verbose`) option, in
> which case output won't be buffered and will be output immediately.
> 
> To support this, individual tests are required to use helper methods for
> logging: `test__printf()` and `test__vprintf()`.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  .../selftests/bpf/prog_tests/bpf_obj_id.c     |   6 +-
>  .../bpf/prog_tests/bpf_verif_scale.c          |  31 ++--
>  .../bpf/prog_tests/get_stack_raw_tp.c         |   4 +-
>  .../selftests/bpf/prog_tests/l4lb_all.c       |   2 +-
>  .../selftests/bpf/prog_tests/map_lock.c       |  10 +-
>  .../selftests/bpf/prog_tests/send_signal.c    |   8 +-
>  .../selftests/bpf/prog_tests/spinlock.c       |   2 +-
>  .../bpf/prog_tests/stacktrace_build_id.c      |   4 +-
>  .../bpf/prog_tests/stacktrace_build_id_nmi.c  |   4 +-
>  .../selftests/bpf/prog_tests/xdp_noinline.c   |   3 +-
>  tools/testing/selftests/bpf/test_progs.c      | 135 +++++++++++++-----
>  tools/testing/selftests/bpf/test_progs.h      |  37 ++++-
>  12 files changed, 173 insertions(+), 73 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
> index cb827383db4d..fb5840a62548 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
> @@ -106,8 +106,8 @@ void test_bpf_obj_id(void)
>  		if (CHECK(err ||
>  			  prog_infos[i].type != BPF_PROG_TYPE_SOCKET_FILTER ||
>  			  info_len != sizeof(struct bpf_prog_info) ||
> -			  (jit_enabled && !prog_infos[i].jited_prog_len) ||
> -			  (jit_enabled &&
> +			  (env.jit_enabled && !prog_infos[i].jited_prog_len) ||
> +			  (env.jit_enabled &&
>  			   !memcmp(jited_insns, zeros, sizeof(zeros))) ||
>  			  !prog_infos[i].xlated_prog_len ||
>  			  !memcmp(xlated_insns, zeros, sizeof(zeros)) ||
> @@ -121,7 +121,7 @@ void test_bpf_obj_id(void)
>  			  err, errno, i,
>  			  prog_infos[i].type, BPF_PROG_TYPE_SOCKET_FILTER,
>  			  info_len, sizeof(struct bpf_prog_info),
> -			  jit_enabled,
> +			  env.jit_enabled,
>  			  prog_infos[i].jited_prog_len,
>  			  prog_infos[i].xlated_prog_len,
>  			  !!memcmp(jited_insns, zeros, sizeof(zeros)),
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> index 2c4d9ef099b4..95f012c417fe 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> @@ -4,12 +4,15 @@
>  static int libbpf_debug_print(enum libbpf_print_level level,
>  			      const char *format, va_list args)
>  {
> -	if (level != LIBBPF_DEBUG)
> -		return vfprintf(stderr, format, args);
> +	if (level != LIBBPF_DEBUG) {
> +		test__vprintf(format, args);
> +		return 0;
> +	}
>  
>  	if (!strstr(format, "verifier log"))
>  		return 0;
> -	return vfprintf(stderr, "%s", args);
> +	test__vprintf("%s", args);
> +	return 0;
>  }
>  
>  static int check_load(const char *file, enum bpf_prog_type type)
> @@ -73,32 +76,38 @@ void test_bpf_verif_scale(void)
>  	libbpf_print_fn_t old_print_fn = NULL;
>  	int err, i;
>  
> -	if (verifier_stats)
> +	if (env.verifier_stats) {
> +		test__force_log();
>  		old_print_fn = libbpf_swap_print(libbpf_debug_print);
> +	}
>  
>  	err = check_load("./loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT);
> -	printf("test_scale:loop3:%s\n", err ? (error_cnt--, "OK") : "FAIL");
> +	test__printf("test_scale:loop3:%s\n",
> +		     err ? (error_cnt--, "OK") : "FAIL");
>  
>  	for (i = 0; i < ARRAY_SIZE(sched_cls); i++) {
>  		err = check_load(sched_cls[i], BPF_PROG_TYPE_SCHED_CLS);
> -		printf("test_scale:%s:%s\n", sched_cls[i], err ? "FAIL" : "OK");
> +		test__printf("test_scale:%s:%s\n", sched_cls[i],
> +			     err ? "FAIL" : "OK");
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(raw_tp); i++) {
>  		err = check_load(raw_tp[i], BPF_PROG_TYPE_RAW_TRACEPOINT);
> -		printf("test_scale:%s:%s\n", raw_tp[i], err ? "FAIL" : "OK");
> +		test__printf("test_scale:%s:%s\n", raw_tp[i],
> +			     err ? "FAIL" : "OK");
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(cg_sysctl); i++) {
>  		err = check_load(cg_sysctl[i], BPF_PROG_TYPE_CGROUP_SYSCTL);
> -		printf("test_scale:%s:%s\n", cg_sysctl[i], err ? "FAIL" : "OK");
> +		test__printf("test_scale:%s:%s\n", cg_sysctl[i],
> +			     err ? "FAIL" : "OK");
>  	}
>  	err = check_load("./test_xdp_loop.o", BPF_PROG_TYPE_XDP);
> -	printf("test_scale:test_xdp_loop:%s\n", err ? "FAIL" : "OK");
> +	test__printf("test_scale:test_xdp_loop:%s\n", err ? "FAIL" : "OK");
>  
>  	err = check_load("./test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL);
> -	printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
> +	test__printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
>  
> -	if (verifier_stats)
> +	if (env.verifier_stats)
>  		libbpf_set_print(old_print_fn);
>  }
> diff --git a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
> index 9d73a8f932ac..3d59b3c841fe 100644
> --- a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
> @@ -41,7 +41,7 @@ static void get_stack_print_output(void *ctx, int cpu, void *data, __u32 size)
>  		 * just assume it is good if the stack is not empty.
>  		 * This could be improved in the future.
>  		 */
> -		if (jit_enabled) {
> +		if (env.jit_enabled) {
>  			found = num_stack > 0;
>  		} else {
>  			for (i = 0; i < num_stack; i++) {
> @@ -58,7 +58,7 @@ static void get_stack_print_output(void *ctx, int cpu, void *data, __u32 size)
>  		}
>  	} else {
>  		num_stack = e->kern_stack_size / sizeof(__u64);
> -		if (jit_enabled) {
> +		if (env.jit_enabled) {
>  			good_kern_stack = num_stack > 0;
>  		} else {
>  			for (i = 0; i < num_stack; i++) {
> diff --git a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
> index 20ddca830e68..5ce572c03a5f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
> +++ b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
> @@ -74,7 +74,7 @@ static void test_l4lb(const char *file)
>  	}
>  	if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
>  		error_cnt++;
> -		printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> +		test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
#define printf(...) test__printf(...) in tests.h?

A bit ugly, but no need to retrain everyone to use new printf wrappers.

>  	}
>  out:
>  	bpf_object__close(obj);
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> index ee99368c595c..2e78217ed3fd 100644
> --- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
> +++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> @@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
>  	for (i = 0; i < 10000; i++) {
>  		err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
>  		if (err) {
> -			printf("lookup failed\n");
> +			test__printf("lookup failed\n");
>  			error_cnt++;
>  			goto out;
>  		}
>  		if (vars[0] != 0) {
> -			printf("lookup #%d var[0]=%d\n", i, vars[0]);
> +			test__printf("lookup #%d var[0]=%d\n", i, vars[0]);
>  			error_cnt++;
>  			goto out;
>  		}
> @@ -22,8 +22,8 @@ static void *parallel_map_access(void *arg)
>  		for (j = 2; j < 17; j++) {
>  			if (vars[j] == rnd)
>  				continue;
> -			printf("lookup #%d var[1]=%d var[%d]=%d\n",
> -			       i, rnd, j, vars[j]);
> +			test__printf("lookup #%d var[1]=%d var[%d]=%d\n",
> +				     i, rnd, j, vars[j]);
>  			error_cnt++;
>  			goto out;
>  		}
> @@ -43,7 +43,7 @@ void test_map_lock(void)
>  
>  	err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
>  	if (err) {
> -		printf("test_map_lock:bpf_prog_load errno %d\n", errno);
> +		test__printf("test_map_lock:bpf_prog_load errno %d\n", errno);
>  		goto close_prog;
>  	}
>  	map_fd[0] = bpf_find_map(__func__, obj, "hash_map");
> diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> index 54218ee3c004..d950f4558897 100644
> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
> @@ -202,8 +202,8 @@ static int test_send_signal_nmi(void)
>  			 -1 /* cpu */, -1 /* group_fd */, 0 /* flags */);
>  	if (pmu_fd == -1) {
>  		if (errno == ENOENT) {
> -			printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
> -				__func__);
> +			test__printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
> +				     __func__);
>  			return 0;
>  		}
>  		/* Let the test fail with a more informative message */
> @@ -222,8 +222,4 @@ void test_send_signal(void)
>  	ret |= test_send_signal_tracepoint();
>  	ret |= test_send_signal_perf();
>  	ret |= test_send_signal_nmi();
> -	if (!ret)
> -		printf("test_send_signal:OK\n");
> -	else
> -		printf("test_send_signal:FAIL\n");
>  }
> diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
> index 114ebe6a438e..deb2db5b85b0 100644
> --- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
> +++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
> @@ -12,7 +12,7 @@ void test_spinlock(void)
>  
>  	err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
>  	if (err) {
> -		printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
> +		test__printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
>  		goto close_prog;
>  	}
>  	for (i = 0; i < 4; i++)
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
> index ac44fda84833..356d2c017a9c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
> @@ -109,8 +109,8 @@ void test_stacktrace_build_id(void)
>  	if (build_id_matches < 1 && retry--) {
>  		bpf_link__destroy(link);
>  		bpf_object__close(obj);
> -		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> -		       __func__);
> +		test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> +			     __func__);
>  		goto retry;
>  	}
>  
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> index 9557b7dfb782..f44f2c159714 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> @@ -140,8 +140,8 @@ void test_stacktrace_build_id_nmi(void)
>  	if (build_id_matches < 1 && retry--) {
>  		bpf_link__destroy(link);
>  		bpf_object__close(obj);
> -		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> -		       __func__);
> +		test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
> +			     __func__);
>  		goto retry;
>  	}
>  
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
> index 09e6b46f5515..b5404494b8aa 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
> @@ -75,7 +75,8 @@ void test_xdp_noinline(void)
>  	}
>  	if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
>  		error_cnt++;
> -		printf("test_xdp_noinline:FAIL:stats %lld %lld\n", bytes, pkts);
> +		test__printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
> +			     bytes, pkts);
>  	}
>  out:
>  	bpf_object__close(obj);
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 94b6951b90b3..3cf3ebda1d31 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -6,9 +6,75 @@
>  #include <argp.h>
>  #include <string.h>
>  
> +/* defined in test_progs.h */
> +struct test_env env = {
> +	.test_num_selector = -1,
> +};
>  int error_cnt, pass_cnt;
> -bool jit_enabled;
> -bool verifier_stats = false;
> +
> +struct prog_test_def {
> +	const char *test_name;
> +	int test_num;
> +	void (*run_test)(void);
> +	bool force_log;
> +	int pass_cnt;
> +	int error_cnt;
> +	bool tested;
> +};
> +
> +void test__force_log() {
> +	env.test->force_log = true;
> +}
> +
> +void test__vprintf(const char *fmt, va_list args)
> +{
> +	size_t rem_sz;
> +	int ret;
> +
> +	if (env.verbose || (env.test && env.test->force_log)) {
> +		vfprintf(stderr, fmt, args);
> +		return;
> +	}
> +
> +try_again:
> +	rem_sz = env.log_cap - env.log_cnt;
> +	if (rem_sz) {
> +		ret = vsnprintf(env.log_buf + env.log_cnt, rem_sz, fmt, args);
> +		if (ret < 0) {
> +			fprintf(stderr, "failed to log message w/ fmt '%s'\n", fmt);
> +			return;
> +		}
> +	}
> +
> +	if (ret >= rem_sz) {
> +		size_t new_sz = env.log_cap * 3 / 2;
> +		char *new_buf;
> +
> +		if (new_sz < 4096)
> +			new_sz = 4096;
> +
> +		new_buf = realloc(env.log_buf, new_sz);
> +		if (!new_buf) {
> +			fprintf(stderr, "failed to realloc log buffer: %d\n",
> +				errno);
> +			return;
> +		}
> +		env.log_buf = new_buf;
> +		env.log_cap = new_sz;
> +		goto try_again;
> +	}
> +
> +	env.log_cnt += ret + 1;
> +}
> +
> +void test__printf(const char *fmt, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, fmt);
> +	test__vprintf(fmt, args);
> +	va_end(args);
> +}
>  
>  struct ipv4_packet pkt_v4 = {
>  	.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
> @@ -163,20 +229,15 @@ void *spin_lock_thread(void *arg)
>  #include <prog_tests/tests.h>
>  #undef DEFINE_TEST
>  
> -struct prog_test_def {
> -	const char *test_name;
> -	int test_num;
> -	void (*run_test)(void);
> -};
> -
>  static struct prog_test_def prog_test_defs[] = {
> -#define DEFINE_TEST(name) {	      \
> -	.test_name = #name,	      \
> -	.run_test = &test_##name,   \
> +#define DEFINE_TEST(name) {		\
> +	.test_name = #name,		\
> +	.run_test = &test_##name,	\
>  },
>  #include <prog_tests/tests.h>
>  #undef DEFINE_TEST
>  };
> +const int prog_test_cnt = ARRAY_SIZE(prog_test_defs);
>  
>  const char *argp_program_version = "test_progs 0.1";
>  const char *argp_program_bug_address = "<bpf@vger.kernel.org>";
> @@ -186,7 +247,6 @@ enum ARG_KEYS {
>  	ARG_TEST_NUM = 'n',
>  	ARG_TEST_NAME = 't',
>  	ARG_VERIFIER_STATS = 's',
> -
>  	ARG_VERBOSE = 'v',
>  };
>  	
> @@ -202,24 +262,13 @@ static const struct argp_option opts[] = {
>  	{},
>  };
>  
> -struct test_env {
> -	int test_num_selector;
> -	const char *test_name_selector;
> -	bool verifier_stats;
> -	bool verbose;
> -	bool very_verbose;
> -};
> -
> -static struct test_env env = {
> -	.test_num_selector = -1,
> -};
> -
>  static int libbpf_print_fn(enum libbpf_print_level level,
>  			   const char *format, va_list args)
>  {
>  	if (!env.very_verbose && level == LIBBPF_DEBUG)
>  		return 0;
> -	return vfprintf(stderr, format, args);
> +	test__vprintf(format, args);
> +	return 0;
>  }
>  
>  static error_t parse_arg(int key, char *arg, struct argp_state *state)
> @@ -267,7 +316,6 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
>  	return 0;
>  }
>  
> -
>  int main(int argc, char **argv)
>  {
>  	static const struct argp argp = {
> @@ -275,7 +323,6 @@ int main(int argc, char **argv)
>  		.parser = parse_arg,
>  		.doc = argp_program_doc,
>  	};
> -	struct prog_test_def *test;
>  	int err, i;
>  
>  	err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> @@ -286,13 +333,14 @@ int main(int argc, char **argv)
>  
>  	srand(time(NULL));
>  
> -	jit_enabled = is_jit_enabled();
> +	env.jit_enabled = is_jit_enabled();
>  
> -	verifier_stats = env.verifier_stats;
> -
> -	for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
> -		test = &prog_test_defs[i];
> +	for (i = 0; i < prog_test_cnt; i++) {
> +		struct prog_test_def *test = &prog_test_defs[i];
> +		int old_pass_cnt = pass_cnt;
> +		int old_error_cnt = error_cnt;
>  
> +		env.test = test;
>  		test->test_num = i + 1;
>  
>  		if (env.test_num_selector >= 0 &&
> @@ -303,8 +351,29 @@ int main(int argc, char **argv)
>  			continue;
>  
>  		test->run_test();
> +		test->tested = true;
> +		test->pass_cnt = pass_cnt - old_pass_cnt;
> +		test->error_cnt = error_cnt - old_error_cnt;
> +		if (test->error_cnt)
> +			env.fail_cnt++;
> +		else
> +			env.succ_cnt++;
> +
> +		if (env.verbose || test->force_log || test->error_cnt) {
> +			if (env.log_cnt) {
> +				fprintf(stdout, "%s", env.log_buf);
> +				if (env.log_buf[env.log_cnt - 1] != '\n')
> +					fprintf(stdout, "\n");
> +			}
> +		}
> +		env.log_cnt = 0;
> +
> +		printf("#%d %s:%s\n", test->test_num, test->test_name,
> +		       test->error_cnt ? "FAIL" : "OK");
>  	}
> +	printf("Summary: %d PASSED, %d FAILED\n", env.succ_cnt, env.fail_cnt);
> +
> +	free(env.log_buf);
>  
> -	printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
>  	return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
>  }
> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
> index 49e0f7d85643..62f55a4231e9 100644
> --- a/tools/testing/selftests/bpf/test_progs.h
> +++ b/tools/testing/selftests/bpf/test_progs.h
> @@ -38,9 +38,33 @@ typedef __u16 __sum16;
>  #include "trace_helpers.h"
>  #include "flow_dissector_load.h"
>  
> -extern int error_cnt, pass_cnt;
> -extern bool jit_enabled;
> -extern bool verifier_stats;
> +struct prog_test_def;
> +
> +struct test_env {
> +	int test_num_selector;
> +	const char *test_name_selector;
> +	bool verifier_stats;
> +	bool verbose;
> +	bool very_verbose;
> +
> +	bool jit_enabled;
> +
> +	struct prog_test_def *test;
> +	char *log_buf;
> +	size_t log_cnt;
> +	size_t log_cap;
> +
> +	int succ_cnt;
> +	int fail_cnt;
> +};
> +
> +extern int error_cnt;
> +extern int pass_cnt;
> +extern struct test_env env;
> +
> +extern void test__printf(const char *fmt, ...);
> +extern void test__vprintf(const char *fmt, va_list args);
> +extern void test__force_log();
>  
>  #define MAGIC_BYTES 123
>  
> @@ -64,11 +88,12 @@ extern struct ipv6_packet pkt_v6;
>  	int __ret = !!(condition);					\
>  	if (__ret) {							\
>  		error_cnt++;						\
> -		printf("%s:FAIL:%s ", __func__, tag);			\
> -		printf(format);						\
> +		test__printf("%s:FAIL:%s ", __func__, tag);		\
> +		test__printf(format);					\
>  	} else {							\
>  		pass_cnt++;						\
> -		printf("%s:PASS:%s %d nsec\n", __func__, tag, duration);\
> +		test__printf("%s:PASS:%s %d nsec\n",			\
> +			      __func__, tag, duration);			\
>  	}								\
>  	__ret;								\
>  })
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [pull request][net 0/9] Mellanox, mlx5 fixes 2019-07-25
From: David Miller @ 2019-07-26 21:34 UTC (permalink / raw)
  To: saeedm; +Cc: netdev, jakub.kicinski
In-Reply-To: <20190725203618.11011-1-saeedm@mellanox.com>

From: Saeed Mahameed <saeedm@mellanox.com>
Date: Thu, 25 Jul 2019 20:36:32 +0000

> This series introduces some fixes to mlx5 driver.
> 
> 1) Ariel is addressing an issue with enacp flow counter race condition
> 2) Aya fixes ethtool speed handling
> 3) Edward fixes modify_cq hw bits alignment 
> 4) Maor fixes RDMA_RX capabilities handling
> 5) Mark reverses unregister devices order to address an issue with LAG
> 6) From Tariq,
>   - wrong max num channels indication regression
>   - TLS counters naming and documentation as suggested by Jakub
>   - kTLS, Call WARN_ONCE on netdev mismatch
> 
> There is one patch in this series that touches nfp driver to align
> TLS statistics names with latest documentation, Jakub is CC'ed.
> 
> Please pull and let me know if there is any problem.

Pulled.

> For -stable v4.9:
>   ('net/mlx5: Use reversed order when unregister devices')
> 
> For -stable v4.20
>   ('net/mlx5e: Prevent encap flow counter update async to user query')
>   ('net/mlx5: Fix modify_cq_in alignment')
> 
> For -stable v5.1
>   ('net/mlx5e: Fix matching of speed to PRM link modes')
> 
> For -stable v5.2
>   ('net/mlx5: Add missing RDMA_RX capabilities')

Queued up.

^ permalink raw reply

* Re: INFO: rcu detected stall in vhost_worker
From: Linus Torvalds @ 2019-07-26 21:35 UTC (permalink / raw)
  To: syzbot
  Cc: Jason Wang, KVM list, Linux Kbuild mailing list,
	Linux List Kernel Mailing, michal.lkml, Michael S. Tsirkin,
	Netdev, syzkaller-bugs, virtualization, Masahiro Yamada
In-Reply-To: <000000000000e87d14058e9728d7@google.com>

On Fri, Jul 26, 2019 at 8:26 AM syzbot
<syzbot+36e93b425cd6eb54fcc1@syzkaller.appspotmail.com> wrote:
>
> syzbot has bisected this bug to:
>
> commit 0ecfebd2b52404ae0c54a878c872bb93363ada36
> Author: Linus Torvalds <torvalds@linux-foundation.org>
> Date:   Sun Jul 7 22:41:56 2019 +0000
>
>      Linux 5.2

That seems very unlikely. That commit literally just changes the
EXTRAVERSION part of the version string.

So even if something actually depended on the version number, even
that wouldn't have triggered any semantic change.

              Linus

^ permalink raw reply

* Re: [patch iproute2 2/2] tc: batch: fix line/line_next processing in batch
From: Stephen Hemminger @ 2019-07-26 21:35 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, sthemmin, dsahern, alexanderk, mlxsw
In-Reply-To: <20190723112538.10977-2-jiri@resnulli.us>

On Tue, 23 Jul 2019 13:25:38 +0200
Jiri Pirko <jiri@resnulli.us> wrote:

> From: Jiri Pirko <jiri@mellanox.com>
> 
> When getcmdline fails, there is no valid string in line_next.
> So change the flow and don't process it. Alongside with that,
> free the previous line buffer and prevent memory leak.
> 
> Fixes: 485d0c6001c4 ("tc: Add batchsize feature for filter and actions")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>


This is not sufficient to avoid valgrind detected uninitialized memory.
The following changes to your patch (#2 alone) is enough to fix
the issue.

The logic here is still a mess and needs to be cleaned up to avoid
future related bugs.

From bbcc22899566556ced9692e4811aea2a38817834 Mon Sep 17 00:00:00 2001
From: Jiri Pirko <jiri@mellanox.com>
Date: Tue, 23 Jul 2019 13:25:38 +0200
Subject: [PATCH] tc: batch: fix line/line_next processing in batch

When getcmdline fails, there is no valid string in line_next.
So change the flow and don't process it. Alongside with that,
free the previous line buffer and prevent memory leak.

Also, avoid passing uninitialized memory to filters.

Fixes: 485d0c6001c4 ("tc: Add batchsize feature for filter and actions")
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 tc/tc.c | 84 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 43 insertions(+), 41 deletions(-)

diff --git a/tc/tc.c b/tc/tc.c
index 64e342dd85bf..95e5481955ad 100644
--- a/tc/tc.c
+++ b/tc/tc.c
@@ -325,11 +325,10 @@ static int batch(const char *name)
 {
 	struct batch_buf *head = NULL, *tail = NULL, *buf_pool = NULL;
 	char *largv[100], *largv_next[100];
-	char *line, *line_next = NULL;
+	char *line = NULL, *line_next = NULL;
 	bool bs_enabled = false;
 	bool lastline = false;
 	int largc, largc_next;
-	bool bs_enabled_saved;
 	bool bs_enabled_next;
 	int batchsize = 0;
 	size_t len = 0;
@@ -360,47 +359,40 @@ static int batch(const char *name)
 	largc = makeargs(line, largv, 100);
 	bs_enabled = batchsize_enabled(largc, largv);
 	do {
-		if (getcmdline(&line_next, &len, stdin) == -1)
+		if (getcmdline(&line_next, &len, stdin) == -1) {
 			lastline = true;
-
-		largc_next = makeargs(line_next, largv_next, 100);
-		bs_enabled_next = batchsize_enabled(largc_next, largv_next);
-		if (bs_enabled) {
-			struct batch_buf *buf;
-
-			buf = get_batch_buf(&buf_pool, &head, &tail);
-			if (!buf) {
-				fprintf(stderr,
-					"failed to allocate batch_buf\n");
-				return -1;
-			}
-			++batchsize;
-		}
-
-		/*
-		 * In batch mode, if we haven't accumulated enough commands
-		 * and this is not the last command and this command & next
-		 * command both support the batchsize feature, don't send the
-		 * message immediately.
-		 */
-		if (!lastline && bs_enabled && bs_enabled_next
-		    && batchsize != MSG_IOV_MAX)
-			send = false;
-		else
 			send = true;
+		} else {
+			largc_next = makeargs(line_next, largv_next, 100);
+			bs_enabled_next = batchsize_enabled(largc_next, largv_next);
+			if (bs_enabled) {
+				struct batch_buf *buf;
+
+				buf = get_batch_buf(&buf_pool, &head, &tail);
+				if (!buf) {
+					fprintf(stderr,
+						"failed to allocate batch_buf\n");
+					return -1;
+				}
+				++batchsize;
+			}
 
-		line = line_next;
-		line_next = NULL;
-		len = 0;
-		bs_enabled_saved = bs_enabled;
-		bs_enabled = bs_enabled_next;
-
-		if (largc == 0) {
-			largc = largc_next;
-			memcpy(largv, largv_next, largc * sizeof(char *));
-			continue;	/* blank line */
+			/*
+			 * In batch mode, if we haven't accumulated enough
+			 * commands and this is not the last command and this
+			 * command & next command both support the batchsize
+			 * feature, don't send the message immediately.
+			 */
+			if (bs_enabled && bs_enabled_next
+			    && batchsize != MSG_IOV_MAX)
+				send = false;
+			else
+				send = true;
 		}
 
+		if (largc == 0)
+			goto to_next_line;	/* blank line */
+
 		err = do_cmd(largc, largv, tail == NULL ? NULL : tail->buf,
 			     tail == NULL ? 0 : sizeof(tail->buf));
 		fflush(stdout);
@@ -411,10 +403,8 @@ static int batch(const char *name)
 			if (!force)
 				break;
 		}
-		largc = largc_next;
-		memcpy(largv, largv_next, largc * sizeof(char *));
 
-		if (send && bs_enabled_saved) {
+		if (send && bs_enabled) {
 			struct iovec *iov, *iovs;
 			struct batch_buf *buf;
 			struct nlmsghdr *n;
@@ -438,6 +428,18 @@ static int batch(const char *name)
 			}
 			batchsize = 0;
 		}
+
+to_next_line:
+		if (lastline)
+			continue;
+		free(line);
+		line = line_next;
+		line_next = NULL;
+		len = 0;
+		bs_enabled = bs_enabled_next;
+		largc = largc_next;
+		memcpy(largv, largv_next, largc * sizeof(char *));
+
 	} while (!lastline);
 
 	free_batch_bufs(&buf_pool);
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] sis900: add support for ethtool's EEPROM dump
From: Saeed Mahameed @ 2019-07-26 21:39 UTC (permalink / raw)
  To: venza@brownhat.org, netdev@vger.kernel.org,
	sergej.benilov@googlemail.com, andrew@lunn.ch
In-Reply-To: <20190725194806.17964-1-sergej.benilov@googlemail.com>

On Thu, 2019-07-25 at 21:48 +0200, Sergej Benilov wrote:
> Implement ethtool's EEPROM dump command (ethtool -e|--eeprom-dump).
> 
> Thx to Andrew Lunn for comments.
> 
> Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
> ---
>  drivers/net/ethernet/sis/sis900.c | 68
> +++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/net/ethernet/sis/sis900.c
> b/drivers/net/ethernet/sis/sis900.c
> index 6e07f5ebacfc..85eaccbbbac1 100644
> --- a/drivers/net/ethernet/sis/sis900.c
> +++ b/drivers/net/ethernet/sis/sis900.c
> @@ -191,6 +191,8 @@ struct sis900_private {
>  	unsigned int tx_full; /* The Tx queue is full. */
>  	u8 host_bridge_rev;
>  	u8 chipset_rev;
> +	/* EEPROM data */
> +	int eeprom_size;
>  };
>  
>  MODULE_AUTHOR("Jim Huang <cmhuang@sis.com.tw>, Ollie Lho <
> ollie@sis.com.tw>");
> @@ -475,6 +477,8 @@ static int sis900_probe(struct pci_dev *pci_dev,
>  	sis_priv->pci_dev = pci_dev;
>  	spin_lock_init(&sis_priv->lock);
>  
> +	sis_priv->eeprom_size = 24;
> + 

this value isn't changing across this patch, 
why do you need to store a constant value in private data ?

Just make a #define .. 

>  	pci_set_drvdata(pci_dev, net_dev);
>  
>  	ring_space = pci_alloc_consistent(pci_dev, TX_TOTAL_SIZE,
> &ring_dma);
> @@ -2122,6 +2126,68 @@ static void sis900_get_wol(struct net_device
> *net_dev, struct ethtool_wolinfo *w
>  	wol->supported = (WAKE_PHY | WAKE_MAGIC);
>  }
>  
> +static int sis900_get_eeprom_len(struct net_device *dev)
> +{
> +	struct sis900_private *sis_priv = netdev_priv(dev);
> +
> +	return sis_priv->eeprom_size;
> +}
> +
> +static int sis900_read_eeprom(struct net_device *net_dev, u8 *buf)
> +{
> +	struct sis900_private *sis_priv = netdev_priv(net_dev);
> +	void __iomem *ioaddr = sis_priv->ioaddr;
> +	int wait, ret = -EAGAIN;
> +	u16 signature;
> +	u16 *ebuf = (u16 *)buf;

reverse xmas tree.

> +	int i;
> +
> +	if (sis_priv->chipset_rev == SIS96x_900_REV) {
> +		sw32(mear, EEREQ);
> +		for (wait = 0; wait < 2000; wait++) {
> +			if (sr32(mear) & EEGNT) {
> +				/* read 16 bits, and index by 16 bits
> */
> +				for (i = 0; i < sis_priv->eeprom_size /
> 2; i++)
> +					ebuf[i] =
> (u16)read_eeprom(ioaddr, i);
> +				ret = 0;
> +				break;
> +			}
> +			udelay(1);
> +		}

cosmetic comment, too much indentations to my taste,
two ways to avoid this,
1) if !SIS96x_900_REV execute the else statement and early return 
2) "do while" to wait for (sr32(mear) & EEGNT) and then execute the for
loop which reads the eeprom outs side the wait loop.

> +		sw32(mear, EEDONE);
> +	} else {
> +		signature = (u16)read_eeprom(ioaddr, EEPROMSignature);
> +		if (signature != 0xffff && signature != 0x0000) {
> +			/* read 16 bits, and index by 16 bits */
> +			for (i = 0; i < sis_priv->eeprom_size / 2; i++)
> +				ebuf[i] = (u16)read_eeprom(ioaddr, i);
> +			ret = 0;
> +		}
> +	}
> +	return ret;
> +}
> +
> +#define SIS900_EEPROM_MAGIC	0xBABE
> +static int sis900_get_eeprom(struct net_device *dev, struct
> ethtool_eeprom *eeprom, u8 *data)
> +{
> +	struct sis900_private *sis_priv = netdev_priv(dev);
> +	u8 *eebuf;
> +	int res;
> +
> +	eebuf = kmalloc(sis_priv->eeprom_size, GFP_KERNEL);
> +	if (!eebuf)
> +		return -ENOMEM;
> +
> +	eeprom->magic = SIS900_EEPROM_MAGIC;
> +	spin_lock_irq(&sis_priv->lock);
> +	res = sis900_read_eeprom(dev, eebuf);
> +	spin_unlock_irq(&sis_priv->lock);
> +	if (!res)
> +		memcpy(data, eebuf + eeprom->offset, eeprom->len);
> +	kfree(eebuf);
> +	return res;
> +}
> +
>  static const struct ethtool_ops sis900_ethtool_ops = {
>  	.get_drvinfo 	= sis900_get_drvinfo,
>  	.get_msglevel	= sis900_get_msglevel,
> @@ -2132,6 +2198,8 @@ static const struct ethtool_ops
> sis900_ethtool_ops = {
>  	.set_wol	= sis900_set_wol,
>  	.get_link_ksettings = sis900_get_link_ksettings,
>  	.set_link_ksettings = sis900_set_link_ksettings,
> +	.get_eeprom_len = sis900_get_eeprom_len,
> +	.get_eeprom = sis900_get_eeprom,
>  };
>  
>  /**

^ permalink raw reply

* Re: [net 7/9] net/mlx5e: kTLS, Call WARN_ONCE on netdev mismatch
From: Saeed Mahameed @ 2019-07-26 21:41 UTC (permalink / raw)
  To: jakub.kicinski@netronome.com
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Tariq Toukan
In-Reply-To: <20190725134950.74733e62@cakuba.netronome.com>

On Thu, 2019-07-25 at 13:49 -0700, Jakub Kicinski wrote:
> On Thu, 25 Jul 2019 20:36:48 +0000, Saeed Mahameed wrote:
> > From: Tariq Toukan <tariqt@mellanox.com>
> > 
> > A netdev mismatch in the processed TLS SKB should not occur,
> > and indicates a kernel bug.
> > Add WARN_ONCE to spot such cases.
> > 
> > Fixes: d2ead1f360e8 ("net/mlx5e: Add kTLS TX HW offload support")
> > Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > index ea032f54197e..3766545ce259 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c
> > @@ -412,7 +412,7 @@ struct sk_buff *mlx5e_ktls_handle_tx_skb(struct
> > net_device *netdev,
> >  		goto out;
> >  
> >  	tls_ctx = tls_get_ctx(skb->sk);
> > -	if (unlikely(tls_ctx->netdev != netdev))
> > +	if (unlikely(WARN_ON_ONCE(tls_ctx->netdev != netdev)))
> 
> Ah, nit: the unlikely is probably unnecessary but that's no big deal.
> 
> #define WARN_ON_ONCE(condition) ({			\
> 	static int __warned;				\
> 	int __ret_warn_once = !!(condition);		\
> 							\
> 	if (unlikely(__ret_warn_once && !__warned)) {	\
> 		__warned = true;			\
> 		WARN_ON(1);				\
> 	}						\
> 	unlikely(__ret_warn_once);			\
> })
> 

indeed, i see Dave already accepted this, will fix in net-next.

Thanks Jakub !


^ permalink raw reply

* Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Andrii Nakryiko @ 2019-07-26 21:42 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <20190726212152.GA24397@mini-arch>

On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > Apprently listing header as a normal dependency for a binary output
> > makes it go through compilation as if it was C code. This currently
> > works without a problem, but in subsequent commits causes problems for
> > differently generated test.h for test_progs. Marking those headers as
> > order-only dependency solves the issue.
> Are you sure it will not result in a situation where
> test_progs/test_maps is not regenerated if tests.h is updated.
>
> If I read the following doc correctly, order deps make sense for
> directories only:
> https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
>
> Can you maybe double check it with:
> * make
> * add new prog_tests/test_something.c
> * make
> to see if the binary is regenerated with test_something.c?

Yeah, tested that, it triggers test_progs rebuild.

Ordering is still preserved, because test.h is dependency of
test_progs.c, which is dependency of test_progs binary, so that's why
it works.

As to why .h file is compiled as C file, I have no idea and ideally
that should be fixed somehow.

I also started with just removing header as dependency completely
(because it's indirect dependency of test_progs.c), but that broke the
build logic. Dunno, too much magic... This works, tested many-many
times, so I was satisfied enough :)

>
> Maybe fix the problem of header compilation by having '#ifndef
> DECLARE_TEST #define DECLARE_TEST() #endif' in tests.h instead?

That's ugly, I'd like to avoid doing that.

>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 11c9c62c3362..bb66cc4a7f34 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -235,7 +235,7 @@ PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h
> >  PROG_TESTS_FILES := $(wildcard prog_tests/*.c)
> >  test_progs.c: $(PROG_TESTS_H)
> >  $(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS)
> > -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_H) $(PROG_TESTS_FILES)
> > +$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(PROG_TESTS_H)
> >  $(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
> >       $(shell ( cd prog_tests/; \
> >                 echo '/* Generated header, do not edit */'; \
> > @@ -256,7 +256,7 @@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
> >  MAP_TESTS_FILES := $(wildcard map_tests/*.c)
> >  test_maps.c: $(MAP_TESTS_H)
> >  $(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
> > -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_H) $(MAP_TESTS_FILES)
> > +$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H)
> >  $(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR)
> >       $(shell ( cd map_tests/; \
> >                 echo '/* Generated header, do not edit */'; \
> > @@ -277,7 +277,7 @@ VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h
> >  VERIFIER_TEST_FILES := $(wildcard verifier/*.c)
> >  test_verifier.c: $(VERIFIER_TESTS_H)
> >  $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS)
> > -$(OUTPUT)/test_verifier: test_verifier.c $(VERIFIER_TESTS_H)
> > +$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H)
> >  $(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR)
> >       $(shell ( cd verifier/; \
> >                 echo '/* Generated header, do not edit */'; \
> > --
> > 2.17.1
> >

^ permalink raw reply

* Re: [PATCH bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs
From: Andrii Nakryiko @ 2019-07-26 21:45 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <20190726212547.GB24397@mini-arch>

On Fri, Jul 26, 2019 at 2:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > Add ability to specify either test number or test name substring to
> > narrow down a set of test to run.
> >
> > Usage:
> > sudo ./test_progs -n 1
> > sudo ./test_progs -t attach_probe
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  tools/testing/selftests/bpf/test_progs.c | 43 +++++++++++++++++++++---
> >  1 file changed, 39 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index eea88ba59225..6e04b9f83777 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -4,6 +4,7 @@

[...]

> >
> >  static error_t parse_arg(int key, char *arg, struct argp_state *state)
> >  {
> >       struct test_env *env = state->input;
> >
> >       switch (key) {
> [..]
> > +     case ARG_TEST_NUM: {
> > +             int test_num;
> > +
> > +             errno = 0;
> > +             test_num = strtol(arg, NULL, 10);
> > +             if (errno)
> > +                     return -errno;
> > +             env->test_num_selector = test_num;
> > +             break;
> > +     }
> Do you think it's really useful? I agree about running by name (I

Special request from Alexei :) But in one of the follow up patches, I
extended this to allow to specify arbitrary subset of tests, e.g.:
1,2,5-10,7-8. So in that regard, it's more powerful than selecting by
name and gives you ultimate freedom.

> usually used grep -v in the Makefile :-), but I'm not sure about running
> by number.
>
> Or is the idea is that you can just copy-paste this number from the
> test_progs output to rerun the tests? In this case, why not copy-paste
> the name instead?

Both were simple to support, I didn't want to dictate one right way to
do this :)

>
> > +     case ARG_TEST_NAME:
> > +             env->test_name_selector = arg;
> > +             break;
> >       case ARG_VERIFIER_STATS:
> >               env->verifier_stats = true;
> >               break;
> > @@ -223,7 +248,7 @@ int main(int argc, char **argv)
> >               .parser = parse_arg,
> >               .doc = argp_program_doc,
> >       };
> > -     const struct prog_test_def *def;
> > +     struct prog_test_def *test;
> >       int err, i;
> >
> >       err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> > @@ -237,8 +262,18 @@ int main(int argc, char **argv)
> >       verifier_stats = env.verifier_stats;
> >
> >       for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
> > -             def = &prog_test_defs[i];
> > -             def->run_test();
> > +             test = &prog_test_defs[i];
> > +
> > +             test->test_num = i + 1;
> > +
> > +             if (env.test_num_selector >= 0 &&
> > +                 test->test_num != env.test_num_selector)
> > +                     continue;
> > +             if (env.test_name_selector &&
> > +                 !strstr(test->test_name, env.test_name_selector))
> > +                     continue;
> > +
> > +             test->run_test();
> >       }
> >
> >       printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> > --
> > 2.17.1
> >

^ permalink raw reply

* Re: [PATCH bpf-next 4/9] libbpf: add libbpf_swap_print to get previous print func
From: Andrii Nakryiko @ 2019-07-26 21:47 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <20190726212818.GC24397@mini-arch>

On Fri, Jul 26, 2019 at 2:28 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > libbpf_swap_print allows to restore previously set print function.
> > This is useful when running many independent test with one default print
> > function, but overriding log verbosity for particular subset of tests.
> Can we change the return type of libbpf_set_print instead and return
> the old function from it? Will it break ABI?

Yeah, thought about that, but I wasn't sure about ABI breakage. It
seems like it shouldn't, so I'll just change libbpf_set_print
signature instead.

>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  tools/lib/bpf/libbpf.c   | 8 ++++++++
> >  tools/lib/bpf/libbpf.h   | 1 +
> >  tools/lib/bpf/libbpf.map | 5 +++++
> >  3 files changed, 14 insertions(+)
> >

[...]

^ permalink raw reply

* Re: [PATCH bpf-next 6/9] selftests/bpf: abstract away test log output
From: Andrii Nakryiko @ 2019-07-26 21:51 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <20190726213104.GD24397@mini-arch>

On Fri, Jul 26, 2019 at 2:31 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/26, Andrii Nakryiko wrote:
> > This patch changes how test output is printed out. By default, if test
> > had no errors, the only output will be a single line with test number,
> > name, and verdict at the end, e.g.:
> >
> >   #31 xdp:OK
> >
> > If test had any errors, all log output captured during test execution
> > will be output after test completes.
> >
> > It's possible to force output of log with `-v` (`--verbose`) option, in
> > which case output won't be buffered and will be output immediately.
> >
> > To support this, individual tests are required to use helper methods for
> > logging: `test__printf()` and `test__vprintf()`.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  .../selftests/bpf/prog_tests/bpf_obj_id.c     |   6 +-
> >  .../bpf/prog_tests/bpf_verif_scale.c          |  31 ++--
> >  .../bpf/prog_tests/get_stack_raw_tp.c         |   4 +-
> >  .../selftests/bpf/prog_tests/l4lb_all.c       |   2 +-
> >  .../selftests/bpf/prog_tests/map_lock.c       |  10 +-
> >  .../selftests/bpf/prog_tests/send_signal.c    |   8 +-
> >  .../selftests/bpf/prog_tests/spinlock.c       |   2 +-
> >  .../bpf/prog_tests/stacktrace_build_id.c      |   4 +-
> >  .../bpf/prog_tests/stacktrace_build_id_nmi.c  |   4 +-
> >  .../selftests/bpf/prog_tests/xdp_noinline.c   |   3 +-
> >  tools/testing/selftests/bpf/test_progs.c      | 135 +++++++++++++-----
> >  tools/testing/selftests/bpf/test_progs.h      |  37 ++++-
> >  12 files changed, 173 insertions(+), 73 deletions(-)
> >

[...]

> >               error_cnt++;
> > -             printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> > +             test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> #define printf(...) test__printf(...) in tests.h?
>
> A bit ugly, but no need to retrain everyone to use new printf wrappers.

I try to reduce amount of magic and surprising things, not add new
ones :) I also led by example and converted all current instances of
printf usage to test__printf, so anyone new will just copy/paste good
example, hopefully. Even if not, this non-buffered output will be
immediately obvious to anyone who just runs `sudo ./test_progs`. And
author of new test with this problem should hopefully be the first and
the only one to catch and fix this.

>
> >       }
> >  out:
> >       bpf_object__close(obj);
> > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > index ee99368c595c..2e78217ed3fd 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > @@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)

[...]

^ permalink raw reply

* [PATCH] iplink: document 'change' option to ip link
From: Stephen Hemminger @ 2019-07-26 21:59 UTC (permalink / raw)
  To: mcroce; +Cc: netdev, Stephen Hemminger
In-Reply-To: <20190724191218.11757-1-mcroce@redhat.com>

Add the command alias "change" to man page.
Don't show it on usage, since it is not commonly used.

Reported-off-by: Matteo Croce <mcroce@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 man/man8/ip-link.8.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 883d88077d66..a8ae72d2b097 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1815,6 +1815,11 @@ can move the system to an unpredictable state. The solution
 is to avoid changing several parameters with one
 .B ip link set
 call.
+The modifier
+.B change
+is equivalent to
+.BR "set" .
+
 
 .TP
 .BI dev " DEVICE "
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH V2 net-next 07/11] net: hns3: adds debug messages to identify eth down cause
From: Saeed Mahameed @ 2019-07-26 22:00 UTC (permalink / raw)
  To: tanhuazhong@huawei.com, davem@davemloft.net
  Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
	salil.mehta@huawei.com, linuxarm@huawei.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	liuyonglong@huawei.com
In-Reply-To: <1564111502-15504-8-git-send-email-tanhuazhong@huawei.com>

On Fri, 2019-07-26 at 11:24 +0800, Huazhong Tan wrote:
> From: Yonglong Liu <liuyonglong@huawei.com>
> 
> Some times just see the eth interface have been down/up via
> dmesg, but can not know why the eth down. So adds some debug
> messages to identify the cause for this.
> 
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
>  drivers/net/ethernet/hisilicon/hns3/hns3_enet.c    | 24
> ++++++++++++++++++++
>  drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 26
> ++++++++++++++++++++++
>  .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 14 ++++++++++++
>  3 files changed, 64 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> index 4d58c53..2e30cfa 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> @@ -459,6 +459,10 @@ static int hns3_nic_net_open(struct net_device
> *netdev)
>  		h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
>  
>  	hns3_config_xps(priv);
> +
> +	if (netif_msg_drv(h))
> +		netdev_info(netdev, "net open\n");
> +

to make sure this is only intended for debug, and to avoid repetition.
#define hns3_dbg(__dev, format, args...)			\
({								\
	if (netif_msg_drv(h))					\
		netdev_info(h->netdev, format, ##args);         \
})
#endif


^ permalink raw reply

* Re: [PATCH bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Stanislav Fomichev @ 2019-07-26 22:01 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzYDvZENJqrT0KKpHbfHNCdObB9p4ZcJqQj3+rM_1ESF3g@mail.gmail.com>

On 07/26, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:21 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > Apprently listing header as a normal dependency for a binary output
> > > makes it go through compilation as if it was C code. This currently
> > > works without a problem, but in subsequent commits causes problems for
> > > differently generated test.h for test_progs. Marking those headers as
> > > order-only dependency solves the issue.
> > Are you sure it will not result in a situation where
> > test_progs/test_maps is not regenerated if tests.h is updated.
> >
> > If I read the following doc correctly, order deps make sense for
> > directories only:
> > https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
> >
> > Can you maybe double check it with:
> > * make
> > * add new prog_tests/test_something.c
> > * make
> > to see if the binary is regenerated with test_something.c?
> 
> Yeah, tested that, it triggers test_progs rebuild.
> 
> Ordering is still preserved, because test.h is dependency of
> test_progs.c, which is dependency of test_progs binary, so that's why
> it works.
> 
> As to why .h file is compiled as C file, I have no idea and ideally
> that should be fixed somehow.
I guess that's because it's a prerequisite and we have a target that
puts all prerequisites when calling CC:

test_progs: a.c b.c tests.h
	gcc a.c b.c tests.h -o test_progs

So gcc compiles each input file.

I'm not actually sure why default dependency system that uses 'gcc -M'
is not working for us (see scripts/Kbuild.include) and we need to manually
add tests.h dependency. But that's outside of the scope..

> I also started with just removing header as dependency completely
> (because it's indirect dependency of test_progs.c), but that broke the
> build logic. Dunno, too much magic... This works, tested many-many
> times, so I was satisfied enough :)
Yeah, that's my only concern, too much magic already and we add
quite a bit more.

> > Maybe fix the problem of header compilation by having '#ifndef
> > DECLARE_TEST #define DECLARE_TEST() #endif' in tests.h instead?
> 
> That's ugly, I'd like to avoid doing that.
That's your call, but I'm not sure what's uglier: complicating already
complex make rules or making a header self contained.

> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > >  tools/testing/selftests/bpf/Makefile | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 11c9c62c3362..bb66cc4a7f34 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -235,7 +235,7 @@ PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h
> > >  PROG_TESTS_FILES := $(wildcard prog_tests/*.c)
> > >  test_progs.c: $(PROG_TESTS_H)
> > >  $(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS)
> > > -$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_H) $(PROG_TESTS_FILES)
> > > +$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(PROG_TESTS_H)
> > >  $(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
> > >       $(shell ( cd prog_tests/; \
> > >                 echo '/* Generated header, do not edit */'; \
> > > @@ -256,7 +256,7 @@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
> > >  MAP_TESTS_FILES := $(wildcard map_tests/*.c)
> > >  test_maps.c: $(MAP_TESTS_H)
> > >  $(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
> > > -$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_H) $(MAP_TESTS_FILES)
> > > +$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H)
> > >  $(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR)
> > >       $(shell ( cd map_tests/; \
> > >                 echo '/* Generated header, do not edit */'; \
> > > @@ -277,7 +277,7 @@ VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h
> > >  VERIFIER_TEST_FILES := $(wildcard verifier/*.c)
> > >  test_verifier.c: $(VERIFIER_TESTS_H)
> > >  $(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS)
> > > -$(OUTPUT)/test_verifier: test_verifier.c $(VERIFIER_TESTS_H)
> > > +$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H)
> > >  $(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR)
> > >       $(shell ( cd verifier/; \
> > >                 echo '/* Generated header, do not edit */'; \
> > > --
> > > 2.17.1
> > >

^ permalink raw reply

* Re: [PATCH v2] iproute2: devlink: port from sys/queue.h to list.h
From: Stephen Hemminger @ 2019-07-26 22:02 UTC (permalink / raw)
  To: Sergei Trofimovich; +Cc: netdev
In-Reply-To: <20190726210105.25458-1-slyfox@gentoo.org>

On Fri, 26 Jul 2019 22:01:05 +0100
Sergei Trofimovich <slyfox@gentoo.org> wrote:

> sys/queue.h does not exist on linux-musl targets and fails build as:
> 
>     devlink.c:28:10: fatal error: sys/queue.h: No such file or directory
>        28 | #include <sys/queue.h>
>           |          ^~~~~~~~~~~~~
> 
> The change ports to list.h API and drops dependency of 'sys/queue.h'.
> The API maps one-to-one.
> 
> Build-tested on linux-musl and linux-glibc.
> 
> Bug: https://bugs.gentoo.org/690486
> CC: Stephen Hemminger <stephen@networkplumber.org>
> CC: netdev@vger.kernel.org
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

Looks good, thanks for following through.
Applied.

^ permalink raw reply

* Re: [PATCH bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs
From: Stanislav Fomichev @ 2019-07-26 22:03 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzZRhHTo+vUFkmLnjPxTL8oi6Fi0zrhvhA6JbY_afU3_Nw@mail.gmail.com>

On 07/26, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:25 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > Add ability to specify either test number or test name substring to
> > > narrow down a set of test to run.
> > >
> > > Usage:
> > > sudo ./test_progs -n 1
> > > sudo ./test_progs -t attach_probe
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > >  tools/testing/selftests/bpf/test_progs.c | 43 +++++++++++++++++++++---
> > >  1 file changed, 39 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > > index eea88ba59225..6e04b9f83777 100644
> > > --- a/tools/testing/selftests/bpf/test_progs.c
> > > +++ b/tools/testing/selftests/bpf/test_progs.c
> > > @@ -4,6 +4,7 @@
> 
> [...]
> 
> > >
> > >  static error_t parse_arg(int key, char *arg, struct argp_state *state)
> > >  {
> > >       struct test_env *env = state->input;
> > >
> > >       switch (key) {
> > [..]
> > > +     case ARG_TEST_NUM: {
> > > +             int test_num;
> > > +
> > > +             errno = 0;
> > > +             test_num = strtol(arg, NULL, 10);
> > > +             if (errno)
> > > +                     return -errno;
> > > +             env->test_num_selector = test_num;
> > > +             break;
> > > +     }
> > Do you think it's really useful? I agree about running by name (I
> 
> Special request from Alexei :) But in one of the follow up patches, I
> extended this to allow to specify arbitrary subset of tests, e.g.:
> 1,2,5-10,7-8. So in that regard, it's more powerful than selecting by
> name and gives you ultimate freedom.
I guess I didn't read the series close enough; that '1,2,3' mode does seem
quite useful indeed!

> 
> > usually used grep -v in the Makefile :-), but I'm not sure about running
> > by number.
> >
> > Or is the idea is that you can just copy-paste this number from the
> > test_progs output to rerun the tests? In this case, why not copy-paste
> > the name instead?
> 
> Both were simple to support, I didn't want to dictate one right way to
> do this :)
> 
> >
> > > +     case ARG_TEST_NAME:
> > > +             env->test_name_selector = arg;
> > > +             break;
> > >       case ARG_VERIFIER_STATS:
> > >               env->verifier_stats = true;
> > >               break;
> > > @@ -223,7 +248,7 @@ int main(int argc, char **argv)
> > >               .parser = parse_arg,
> > >               .doc = argp_program_doc,
> > >       };
> > > -     const struct prog_test_def *def;
> > > +     struct prog_test_def *test;
> > >       int err, i;
> > >
> > >       err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> > > @@ -237,8 +262,18 @@ int main(int argc, char **argv)
> > >       verifier_stats = env.verifier_stats;
> > >
> > >       for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
> > > -             def = &prog_test_defs[i];
> > > -             def->run_test();
> > > +             test = &prog_test_defs[i];
> > > +
> > > +             test->test_num = i + 1;
> > > +
> > > +             if (env.test_num_selector >= 0 &&
> > > +                 test->test_num != env.test_num_selector)
> > > +                     continue;
> > > +             if (env.test_name_selector &&
> > > +                 !strstr(test->test_name, env.test_name_selector))
> > > +                     continue;
> > > +
> > > +             test->run_test();
> > >       }
> > >
> > >       printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> > > --
> > > 2.17.1
> > >

^ permalink raw reply

* Re: [PATCH] iplink_can: fix format output of clock with flag -details
From: Stephen Hemminger @ 2019-07-26 22:04 UTC (permalink / raw)
  To: Antonio Borneo; +Cc: netdev
In-Reply-To: <20190726130609.27704-1-borneo.antonio@gmail.com>

On Fri, 26 Jul 2019 15:06:09 +0200
Antonio Borneo <borneo.antonio@gmail.com> wrote:

> The command
> 	ip -details link show can0
> prints in the last line the value of the clock frequency attached
> to the name of the following value "numtxqueues", e.g.
> 	clock 49500000numtxqueues 1 numrxqueues 1 gso_max_size
> 	 65536 gso_max_segs 65535
> 
> Add the missing space after the clock value.
> 
> Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>

Applied, but CAN looks like it doesn't support single line output correctly.

^ permalink raw reply

* [PATCH] gigaset: stop maintaining seperately
From: Paul Bolle @ 2019-07-26 22:05 UTC (permalink / raw)
  To: David Miller
  Cc: Tilman Schmidt, Hansjoerg Lipp, Arnd Bergmann, Karsten Keil,
	netdev, linux-kernel

The Dutch consumer grade ISDN network will be shut down on September 1,
2019. This means I'll be converted to some sort of VOIP shortly. At that
point it would be unwise to try to maintain the gigaset driver, even for
odd fixes as I do. So I'll stop maintaining it as a seperate driver and
bump support to CAPI in staging. De facto this means the driver will be
unmaintained, since no-one seems to be working on CAPI.

I've lighty tested the hardware specific modules of this driver (bas-gigaset,
ser-gigaset, and usb-gigaset) for v5.3-rc1. The basic functionality appears to
be working. It's unclear whether anyone still cares. I'm aware of only one
person sort of using the driver a few years ago.

Thanks to Karsten Keil for the ISDN subsystems gigaset was using (I4L and
CAPI). And many thanks to Hansjoerg Lipp and Tilman Schmidt for writing and
upstreaming this driver.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
 MAINTAINERS | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 783569e3c4b4..e99afbd13355 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6822,13 +6822,6 @@ F:	Documentation/filesystems/gfs2*.txt
 F:	fs/gfs2/
 F:	include/uapi/linux/gfs2_ondisk.h
 
-GIGASET ISDN DRIVERS
-M:	Paul Bolle <pebolle@tiscali.nl>
-L:	gigaset307x-common@lists.sourceforge.net
-W:	http://gigaset307x.sourceforge.net/
-S:	Odd Fixes
-F:	drivers/staging/isdn/gigaset/
-
 GNSS SUBSYSTEM
 M:	Johan Hovold <johan@kernel.org>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/johan/gnss.git
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH V2 net-next 07/11] net: hns3: adds debug messages to identify eth down cause
From: Joe Perches @ 2019-07-26 22:18 UTC (permalink / raw)
  To: Saeed Mahameed, tanhuazhong@huawei.com, davem@davemloft.net
  Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
	salil.mehta@huawei.com, linuxarm@huawei.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	liuyonglong@huawei.com
In-Reply-To: <a32ca755bfd69046cf89aeacbf67fd16313de768.camel@mellanox.com>

On Fri, 2019-07-26 at 22:00 +0000, Saeed Mahameed wrote:
> On Fri, 2019-07-26 at 11:24 +0800, Huazhong Tan wrote:
> > From: Yonglong Liu <liuyonglong@huawei.com>
> > 
> > Some times just see the eth interface have been down/up via
> > dmesg, but can not know why the eth down. So adds some debug
> > messages to identify the cause for this.
[]
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> []
> > @@ -459,6 +459,10 @@ static int hns3_nic_net_open(struct net_device
> > *netdev)
> >  		h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
> >  
> >  	hns3_config_xps(priv);
> > +
> > +	if (netif_msg_drv(h))
> > +		netdev_info(netdev, "net open\n");
> > +
> 
> to make sure this is only intended for debug, and to avoid repetition.
> #define hns3_dbg(__dev, format, args...)			\
> ({								\
> 	if (netif_msg_drv(h))					\
> 		netdev_info(h->netdev, format, ##args);         \
> })

	netif_dbg(h, drv, h->netdev, "net open\n")



^ permalink raw reply

* Re: [PATCH bpf-next 6/9] selftests/bpf: abstract away test log output
From: Stanislav Fomichev @ 2019-07-26 22:26 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzaVCdHT_U+m7niJLsSmbf+M9DrFjf_PNOmQQZvuHsr9Xg@mail.gmail.com>

On 07/26, Andrii Nakryiko wrote:
> On Fri, Jul 26, 2019 at 2:31 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > On 07/26, Andrii Nakryiko wrote:
> > > This patch changes how test output is printed out. By default, if test
> > > had no errors, the only output will be a single line with test number,
> > > name, and verdict at the end, e.g.:
> > >
> > >   #31 xdp:OK
> > >
> > > If test had any errors, all log output captured during test execution
> > > will be output after test completes.
> > >
> > > It's possible to force output of log with `-v` (`--verbose`) option, in
> > > which case output won't be buffered and will be output immediately.
> > >
> > > To support this, individual tests are required to use helper methods for
> > > logging: `test__printf()` and `test__vprintf()`.
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > >  .../selftests/bpf/prog_tests/bpf_obj_id.c     |   6 +-
> > >  .../bpf/prog_tests/bpf_verif_scale.c          |  31 ++--
> > >  .../bpf/prog_tests/get_stack_raw_tp.c         |   4 +-
> > >  .../selftests/bpf/prog_tests/l4lb_all.c       |   2 +-
> > >  .../selftests/bpf/prog_tests/map_lock.c       |  10 +-
> > >  .../selftests/bpf/prog_tests/send_signal.c    |   8 +-
> > >  .../selftests/bpf/prog_tests/spinlock.c       |   2 +-
> > >  .../bpf/prog_tests/stacktrace_build_id.c      |   4 +-
> > >  .../bpf/prog_tests/stacktrace_build_id_nmi.c  |   4 +-
> > >  .../selftests/bpf/prog_tests/xdp_noinline.c   |   3 +-
> > >  tools/testing/selftests/bpf/test_progs.c      | 135 +++++++++++++-----
> > >  tools/testing/selftests/bpf/test_progs.h      |  37 ++++-
> > >  12 files changed, 173 insertions(+), 73 deletions(-)
> > >
> 
> [...]
> 
> > >               error_cnt++;
> > > -             printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> > > +             test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
> > #define printf(...) test__printf(...) in tests.h?
> >
> > A bit ugly, but no need to retrain everyone to use new printf wrappers.
> 
> I try to reduce amount of magic and surprising things, not add new
> ones :) I also led by example and converted all current instances of
> printf usage to test__printf, so anyone new will just copy/paste good
> example, hopefully. Even if not, this non-buffered output will be
> immediately obvious to anyone who just runs `sudo ./test_progs`.

[..]
> And
> author of new test with this problem should hopefully be the first and
> the only one to catch and fix this.
Yeah, that is my only concern, that regular printfs will eventually
creep in. It's already confusing to go to/from printf/printk.

2c:

I'm coming from a perspective of tools/testing/selftests/kselftest.h
which is supposed to be a generic framework with custom
printf variants (ksft_print_msg), but I still see a bunch of tests
calling printf :-/

	grep -ril ksft_exit_fail_msg selftests/ | xargs -n1 grep -w printf

Since we don't expect regular buffered io from the tests anyway
it might be easier just to add a bit of magic and call it a day.

> > >       }
> > >  out:
> > >       bpf_object__close(obj);
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > > index ee99368c595c..2e78217ed3fd 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
> > > @@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
> 
> [...]

^ permalink raw reply

* Re: [PATCH net-next v2] mlx4/en_netdev: allow offloading VXLAN over VLAN
From: Saeed Mahameed @ 2019-07-26 22:47 UTC (permalink / raw)
  To: dcaratti@redhat.com, netdev@vger.kernel.org, davem@davemloft.net,
	Tariq Toukan, pabeni@redhat.com, marcelo.leitner@gmail.com
In-Reply-To: <2beb05557960e04aa588ecc09e9ee5e5a19fc651.1564164688.git.dcaratti@redhat.com>

On Fri, 2019-07-26 at 20:18 +0200, Davide Caratti wrote:
> ConnectX-3 Pro can offload transmission of VLAN packets with VXLAN
> inside:
> enable tunnel offloads in dev->vlan_features, like it's done with
> other
> NIC drivers (e.g. be2net and ixgbe).
> 
> It's no more necessary to change dev->hw_enc_features when VXLAN are
> added
> or removed, since .ndo_features_check() already checks for VXLAN
> packet
> where the UDP destination port matches the configured value. Just set
> dev->hw_enc_features when the NIC is initialized, so that overlying
> VLAN
> can correctly inherit the tunnel offload capabilities.
> 
> Changes since v1:
> - avoid flipping hw_enc_features, instead of calling netdev
> notifiers,
>   thanks to Saeed Mahameed
> - squash two patches into a single one
> 
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply

* Re: [PATCH v2 bpf] libbpf: fix missing __WORDSIZE definition
From: Andrii Nakryiko @ 2019-07-26 23:01 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Daniel Borkmann, Andrii Nakryiko, bpf, Networking,
	Alexei Starovoitov, Arnaldo Carvalho de Melo, Kernel Team
In-Reply-To: <20190726204937.GD24867@kernel.org>

On Fri, Jul 26, 2019 at 1:49 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Jul 18, 2019 at 10:30:21AM -0700, Andrii Nakryiko escreveu:
> > hashmap.h depends on __WORDSIZE being defined. It is defined by
> > glibc/musl in different headers. It's an explicit goal for musl to be
> > "non-detectable" at compilation time, so instead include glibc header if
> > glibc is explicitly detected and fall back to musl header otherwise.
> >
> > Fixes: e3b924224028 ("libbpf: add resizable non-thread safe internal hashmap")
> > Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Couldn't find ths in the bpf tree, please consider applying it:
>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Arnaldo, I somehow got impression that you were going to pull this
into your perf tree. Can you please confirm that it wasn't pulled into
your tree, so that Alexei can apply it to bpf tree? Thanks!


>
>
> - Arnaldo
>
> > ---
> >  tools/lib/bpf/hashmap.h | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/tools/lib/bpf/hashmap.h b/tools/lib/bpf/hashmap.h
> > index 03748a742146..bae8879cdf58 100644
> > --- a/tools/lib/bpf/hashmap.h
> > +++ b/tools/lib/bpf/hashmap.h
> > @@ -10,6 +10,11 @@
> >
> >  #include <stdbool.h>
> >  #include <stddef.h>
> > +#ifdef __GLIBC__
> > +#include <bits/wordsize.h>
> > +#else
> > +#include <bits/reg.h>
> > +#endif
> >  #include "libbpf_internal.h"
> >
> >  static inline size_t hash_bits(size_t h, int bits)
> > --
> > 2.17.1
>
> --
>
> - Arnaldo

^ permalink raw reply

* Re: [PATCH net-next 08/11] net: hns3: add interrupt affinity support for misc interrupt
From: Saeed Mahameed @ 2019-07-26 23:18 UTC (permalink / raw)
  To: linyunsheng@huawei.com, tanhuazhong@huawei.com,
	davem@davemloft.net
  Cc: lipeng321@huawei.com, yisen.zhuang@huawei.com,
	salil.mehta@huawei.com, linuxarm@huawei.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <db2d081f-b892-b141-7fa5-44e66dd97eb9@huawei.com>

On Thu, 2019-07-25 at 10:05 +0800, Yunsheng Lin wrote:
> On 2019/7/25 3:24, Saeed Mahameed wrote:
> > On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
> > > From: Yunsheng Lin <linyunsheng@huawei.com>
> > > 

[...]
> > > 
> > > +static void hclge_irq_affinity_notify(struct irq_affinity_notify
> > > *notify,
> > > +				      const cpumask_t *mask)
> > > +{
> > > +	struct hclge_dev *hdev = container_of(notify, struct hclge_dev,
> > > +					      affinity_notify);
> > > +
> > > +	cpumask_copy(&hdev->affinity_mask, mask);
> > > +	del_timer_sync(&hdev->service_timer);
> > > +	hdev->service_timer.expires = jiffies + HZ;
> > > +	add_timer_on(&hdev->service_timer, cpumask_first(&hdev-
> > > > affinity_mask));
> > > +}
> > 
> > I don't see any relation between your misc irq vector and &hdev-
> > > service_timer, to me this looks like an abuse of the irq affinity
> > > API
> > to allow the user to move the service timer affinity.
> 
> Hi, thanks for reviewing.
> 
> hdev->service_timer is used to schedule the periodic work
> queue hdev->service_task, we want all the management work
> queue including hdev->service_task to bind to the same cpu
> to improve cache and power efficiency, it is better to move
> service timer affinity according to that.
> 
> The hdev->service_task is changed to delay work queue in
> next patch " net: hns3: make hclge_service use delayed workqueue",
> So the affinity in the timer of the delay work queue is automatically
> set to the affinity of the delay work queue, we will move the
> "make hclge_service use delayed workqueue" patch before the
> "add interrupt affinity support for misc interrupt" patch, so
> we do not have to set service timer affinity explicitly.
> 
> Also, There is there work queues(mbx_service_task, service_task,
> rst_service_task) in the hns3 driver, we plan to combine them
> to one or two workqueue to improve efficiency and readability.
> 

So just to make it clear, you have 3 deferred works, 2 are triggered by
the misc irq and 1 is periodic, you want them all on the same core and
you want to control their affinity via the irq affinity ? for works #1
and  #2 you get that for free since the irq is triggering them, but for
work #3 (the periodic one) you need to manually move it when the irq
affinity changes.

I guess i am ok with this, since moving the irq affinity isn't only
required by the periodic work but also for the other works that the irq
is actually triggering (so it is not an actual abuse, sort of .. )




^ permalink raw reply


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