Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v6] selftests: add headers_install to lib.mk
From: Shuah Khan @ 2018-09-05 14:13 UTC (permalink / raw)
  To: Anders Roxell, yamada.masahiro, michal.lkml, bamv2005, brgl,
	pbonzini, akpm, rppt, aarcange
  Cc: linux-kbuild, linux-kernel, linux-kselftest, netdev, Shuah Khan
In-Reply-To: <20180904104721.27535-1-anders.roxell@linaro.org>

On 09/04/2018 04:47 AM, Anders Roxell wrote:
> If the kernel headers aren't installed we can't build all the tests.
> Add a new make target rule 'khdr' in the file lib.mk to generate the
> kernel headers and that gets include for every test-dir Makefile that
> includes lib.mk If the testdir in turn have its own sub-dirs the
> top_srcdir needs to be set to the linux-rootdir to be able to generate
> the kernel headers.
> 
> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> Reviewed-by: Fathi Boudra <fathi.boudra@linaro.org>
> ---
> 
> I sent this (v5) a month ago and wondered if it got lost. Resending
> unchanged.
> 
> Cheers,
> Anders
> 
>  Makefile                                           | 14 +-------------
>  scripts/subarch.include                            | 13 +++++++++++++
>  tools/testing/selftests/android/Makefile           |  2 +-
>  tools/testing/selftests/android/ion/Makefile       |  2 ++
>  tools/testing/selftests/futex/functional/Makefile  |  1 +
>  tools/testing/selftests/gpio/Makefile              |  7 ++-----
>  tools/testing/selftests/kvm/Makefile               |  7 ++-----
>  tools/testing/selftests/lib.mk                     | 12 ++++++++++++
>  tools/testing/selftests/net/Makefile               |  1 +
>  .../selftests/networking/timestamping/Makefile     |  1 +
>  tools/testing/selftests/vm/Makefile                |  4 ----
>  11 files changed, 36 insertions(+), 28 deletions(-)
>  create mode 100644 scripts/subarch.include
> 
> diff --git a/Makefile b/Makefile
> index 19948e556941..9c781e77fa24 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -299,19 +299,7 @@ KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
>  KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
>  export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
>  
> -# SUBARCH tells the usermode build what the underlying arch is.  That is set
> -# first, and if a usermode build is happening, the "ARCH=um" on the command
> -# line overrides the setting of ARCH below.  If a native build is happening,
> -# then ARCH is assigned, getting whatever value it gets normally, and
> -# SUBARCH is subsequently ignored.
> -
> -SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
> -				  -e s/sun4u/sparc64/ \
> -				  -e s/arm.*/arm/ -e s/sa110/arm/ \
> -				  -e s/s390x/s390/ -e s/parisc64/parisc/ \
> -				  -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
> -				  -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
> -				  -e s/riscv.*/riscv/)
> +include scripts/subarch.include
>  
>  # Cross compiling and selecting different set of gcc/bin-utils
>  # ---------------------------------------------------------------------------
> diff --git a/scripts/subarch.include b/scripts/subarch.include
> new file mode 100644
> index 000000000000..650682821126
> --- /dev/null
> +++ b/scripts/subarch.include
> @@ -0,0 +1,13 @@
> +# SUBARCH tells the usermode build what the underlying arch is.  That is set
> +# first, and if a usermode build is happening, the "ARCH=um" on the command
> +# line overrides the setting of ARCH below.  If a native build is happening,
> +# then ARCH is assigned, getting whatever value it gets normally, and
> +# SUBARCH is subsequently ignored.
> +
> +SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
> +				  -e s/sun4u/sparc64/ \
> +				  -e s/arm.*/arm/ -e s/sa110/arm/ \
> +				  -e s/s390x/s390/ -e s/parisc64/parisc/ \
> +				  -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
> +				  -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
> +				  -e s/riscv.*/riscv/)
> diff --git a/tools/testing/selftests/android/Makefile b/tools/testing/selftests/android/Makefile
> index 72c25a3cb658..d9a725478375 100644
> --- a/tools/testing/selftests/android/Makefile
> +++ b/tools/testing/selftests/android/Makefile
> @@ -6,7 +6,7 @@ TEST_PROGS := run.sh
>  
>  include ../lib.mk
>  
> -all:
> +all: khdr
>  	@for DIR in $(SUBDIRS); do		\
>  		BUILD_TARGET=$(OUTPUT)/$$DIR;	\
>  		mkdir $$BUILD_TARGET  -p;	\
> diff --git a/tools/testing/selftests/android/ion/Makefile b/tools/testing/selftests/android/ion/Makefile
> index e03695287f76..88cfe88e466f 100644
> --- a/tools/testing/selftests/android/ion/Makefile
> +++ b/tools/testing/selftests/android/ion/Makefile
> @@ -10,6 +10,8 @@ $(TEST_GEN_FILES): ipcsocket.c ionutils.c
>  
>  TEST_PROGS := ion_test.sh
>  
> +KSFT_KHDR_INSTALL := 1
> +top_srcdir = ../../../../..
>  include ../../lib.mk
>  
>  $(OUTPUT)/ionapp_export: ionapp_export.c ipcsocket.c ionutils.c
> diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
> index ff8feca49746..ad1eeb14fda7 100644
> --- a/tools/testing/selftests/futex/functional/Makefile
> +++ b/tools/testing/selftests/futex/functional/Makefile
> @@ -18,6 +18,7 @@ TEST_GEN_FILES := \
>  
>  TEST_PROGS := run.sh
>  
> +top_srcdir = ../../../../..
>  include ../../lib.mk
>  
>  $(TEST_GEN_FILES): $(HEADERS)
> diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
> index 1bbb47565c55..4665cdbf1a8d 100644
> --- a/tools/testing/selftests/gpio/Makefile
> +++ b/tools/testing/selftests/gpio/Makefile
> @@ -21,11 +21,8 @@ endef
>  CFLAGS += -O2 -g -std=gnu99 -Wall -I../../../../usr/include/
>  LDLIBS += -lmount -I/usr/include/libmount
>  
> -$(BINARIES): ../../../gpio/gpio-utils.o ../../../../usr/include/linux/gpio.h
> +$(BINARIES):| khdr
> +$(BINARIES): ../../../gpio/gpio-utils.o
>  
>  ../../../gpio/gpio-utils.o:
>  	make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
> -
> -../../../../usr/include/linux/gpio.h:
> -	make -C ../../../.. headers_install INSTALL_HDR_PATH=$(shell pwd)/../../../../usr/
> -
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 03b0f551bedf..87d1a8488af8 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -37,9 +37,6 @@ $(LIBKVM_OBJ): $(OUTPUT)/%.o: %.c
>  $(OUTPUT)/libkvm.a: $(LIBKVM_OBJ)
>  	$(AR) crs $@ $^
>  
> -$(LINUX_HDR_PATH):
> -	make -C $(top_srcdir) headers_install
> -
> -all: $(STATIC_LIBS) $(LINUX_HDR_PATH)
> +all: $(STATIC_LIBS)
>  $(TEST_GEN_PROGS): $(STATIC_LIBS)
> -$(TEST_GEN_PROGS) $(LIBKVM_OBJ): | $(LINUX_HDR_PATH)
> +$(STATIC_LIBS):| khdr
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 17ab36605a8e..0a8e75886224 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -16,8 +16,20 @@ TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
>  TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
>  TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
>  
> +top_srcdir ?= ../../../..
> +include $(top_srcdir)/scripts/subarch.include
> +ARCH		?= $(SUBARCH)
> +
>  all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
>  
> +.PHONY: khdr
> +khdr:
> +	make ARCH=$(ARCH) -C $(top_srcdir) headers_install
> +
> +ifdef KSFT_KHDR_INSTALL
> +$(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES):| khdr
> +endif
> +
>  .ONESHELL:
>  define RUN_TEST_PRINT_RESULT
>  	TEST_HDR_MSG="selftests: "`basename $$PWD`:" $$BASENAME_TEST";	\
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index cccdb2295567..256d82d5fa87 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -15,6 +15,7 @@ TEST_GEN_FILES += udpgso udpgso_bench_tx udpgso_bench_rx ip_defrag
>  TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>  TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
>  
> +KSFT_KHDR_INSTALL := 1
>  include ../lib.mk
>  
>  $(OUTPUT)/reuseport_bpf_numa: LDFLAGS += -lnuma
> diff --git a/tools/testing/selftests/networking/timestamping/Makefile b/tools/testing/selftests/networking/timestamping/Makefile
> index a728040edbe1..14cfcf006936 100644
> --- a/tools/testing/selftests/networking/timestamping/Makefile
> +++ b/tools/testing/selftests/networking/timestamping/Makefile
> @@ -5,6 +5,7 @@ TEST_PROGS := hwtstamp_config rxtimestamp timestamping txtimestamp
>  
>  all: $(TEST_PROGS)
>  
> +top_srcdir = ../../../../..
>  include ../../lib.mk
>  
>  clean:
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index 9881876d2aa0..e94b7b14bcb2 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -26,10 +26,6 @@ TEST_PROGS := run_vmtests
>  
>  include ../lib.mk
>  
> -$(OUTPUT)/userfaultfd: ../../../../usr/include/linux/kernel.h
>  $(OUTPUT)/userfaultfd: LDLIBS += -lpthread
>  
>  $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
> -
> -../../../../usr/include/linux/kernel.h:
> -	make -C ../../../.. headers_install
> 

I queued this up in linux-kselftest next to be included in my next
pull request.

thanks,
-- Shuah

^ permalink raw reply

* Re: [PATCH net] devlink: Fix devlink_param_driverinit_value_set() stub return code
From: David Ahern @ 2018-09-05 14:26 UTC (permalink / raw)
  To: Moshe Shemesh, David S. Miller; +Cc: Jiri Pirko, netdev, linux-kernel
In-Reply-To: <7fd426d1-61a9-7f45-bf26-05bd9e180277@mellanox.com>

On 9/5/18 6:48 AM, Moshe Shemesh wrote:
> 
> 
> On 9/4/2018 7:13 PM, David Ahern wrote:
>> On 9/4/18 7:04 AM, Moshe Shemesh wrote:
>>> The stub function returned -EOPNOTSUPP while CONFIG_NET_DEVLINK is off.
>>> It caused false warning during driver load. Driver needs to update
>>> devlink on a parameter value if devlink module is there, if not it
>>> doesn't need any error code.
>>>
>>> Fixes: ec01aeb1803e ("devlink: Add support for get/set driverinit
>>> value")
>>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>>> ---
>>>   include/net/devlink.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>>> index b9b89d6..b467357 100644
>>> --- a/include/net/devlink.h
>>> +++ b/include/net/devlink.h
>>> @@ -781,7 +781,7 @@ static inline bool
>>> devlink_dpipe_table_counter_enabled(struct devlink *devlink,
>>>   devlink_param_driverinit_value_set(struct devlink *devlink, u32
>>> param_id,
>>>                      union devlink_param_value init_val)
>>>   {
>>> -    return -EOPNOTSUPP;
>>> +    return 0;
>>>   }
>>>     static inline void
>>>
>>
>> This should be handled by the driver -- check for -EOPNOTSUPP and not
>> log an error.
> 
> This is a stub inline function.
> The return value would be ambiguous as the non-stub function can also
> return -EOPNOTSUPP, in case the driver-init mode is not supported for a
> parameter.
>>
>> devlink is generic infrastructure. If a call is made and the operation
>> is not supported, then devlink should return an error.
> 
> In general the stub functions should take care that the driver won't
> feel the missing code as much as possible. That's why
> devlink_params_register() returns success and so should this function.
>>

A driver is accessing core infrastructure to set a value; core infra can
not because the code is not compiled in. The driver should be told that
the option is not enabled, and it is at the moment with the -EOPNOTSUPP
return code.

That is similar to devlink_dpipe_table_resource_set which returns
-EOPNOTSUPP when devlink is compiled out.

^ permalink raw reply

* Re: Allocation failure with subsequent kernel crash
From: tedheadster @ 2018-09-05 14:39 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Matthew Whitehead, Alexei Starovoitov, Linux Kernel Mailing List,
	netdev, Ingo Molnar, Thomas Gleixner, Alexei Starovoitov
In-Reply-To: <05825446-e8aa-a983-3fc6-4dc8e81cba57@iogearbox.net>

> I've been looking into it a bit today and still am. Given you've seen
> this on x86_32 and also on older kernels, I presume JIT was not involved
> (/proc/sys/net/core/bpf_jit_enable is 0). Do you run any specific workload
> until you trigger this (e.g. fuzzer on BPF), or any specific event that
> triggers at that time after ~5hrs? Or only systemd on idle machine? Have
> you managed to reproduce this also elsewhere? Bisect seems indeed painful
> but would help tremendously; perhaps also dumping the BPF insns that are
> loaded at that point in time.

Daniel,
  I've been trying for days to bisect this, but it is hard to
reproduce. However, I did have a question.

The crash is happening when bpf_prog_load() hits an error case and
then jumps to free_used_maps(prog->aux). However, I don't see an
obvious place where the 'aux' field gets initialized in
bpf_prog_load(). So it might easily be zero/null.

Could that explain the crash due to "unable to handle kernel NULL
pointer dereference"?

- Matthew

^ permalink raw reply

* [PATCH net] mlxsw: spectrum_buffers: Set up a dedicated pool for BUM traffic
From: Petr Machata @ 2018-09-05 10:16 UTC (permalink / raw)
  To: netdev; +Cc: jiri, idosch, davem

MC-aware mode was recently enabled by mlxsw on Spectrum switches in
commit 7b8195306694 ("mlxsw: spectrum: Configure MC-aware mode on mlxsw
ports"). Unfortunately, testing has shown that the fix is incomplete and
in the presented form actually makes the problem even worse, because any
amount of MC traffic causes UC disruption.

The reason for this is that currently, mlxsw configures the MC-specific
TCs (8..15) to map to pool 0. It also configures a maximum buffer size
of 0, but for MC traffic that maximum is disregarded and not part of the
quota. Therefore MC traffic is always admitted to the egress buffer.

Fix the configuration by directing the MC TCs into pool 15, which is
dedicated to MC traffic and recognized as such by the silicon.

Fixes: 7b8195306694 ("mlxsw: spectrum: Configure MC-aware mode on mlxsw ports")
Signed-off-by: Petr Machata <petrm@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
index 4327487553c5..3589432d1643 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_buffers.c
@@ -337,14 +337,14 @@ static const struct mlxsw_sp_sb_cm mlxsw_sp_sb_cms_egress[] = {
 	MLXSW_SP_SB_CM(1500, 9, 0),
 	MLXSW_SP_SB_CM(1500, 9, 0),
 	MLXSW_SP_SB_CM(1500, 9, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
-	MLXSW_SP_SB_CM(0, 0, 0),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
+	MLXSW_SP_SB_CM(0, 140000, 15),
 	MLXSW_SP_SB_CM(1, 0xff, 0),
 };
 
-- 
2.4.11

^ permalink raw reply related

* Re: [PATCH net-next] net: lan743x_ptp: make function lan743x_ptp_set_sync_ts_insert() static
From: David Miller @ 2018-09-05 15:07 UTC (permalink / raw)
  To: yuehaibing; +Cc: bryan.whitehead, UNGLinuxDriver, linux-kernel, netdev
In-Reply-To: <20180905122437.19756-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 5 Sep 2018 20:24:37 +0800

> Fixes the following sparse warning:
> 
> drivers/net/ethernet/microchip/lan743x_ptp.c:980:6: warning:
>  symbol 'lan743x_ptp_set_sync_ts_insert' was not declared. Should it be static?
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH net] be2net: Fix memory leak in be_cmd_get_profile_config()
From: David Miller @ 2018-09-05 15:08 UTC (permalink / raw)
  To: poros
  Cc: netdev, ivecera, sathya.perla, ajit.khaparde,
	sriharsha.basavapatna, somnath.kotur, linux-kernel
In-Reply-To: <20180905123746.7399-1-poros@redhat.com>

From: Petr Oros <poros@redhat.com>
Date: Wed,  5 Sep 2018 14:37:45 +0200

> DMA allocated memory is lost in be_cmd_get_profile_config() when we
> call it with non-NULL port_res parameter.
> 
> Signed-off-by: Petr Oros <poros@redhat.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH V3] net: qca_spi: Fix race condition in spi transfers
From: David Miller @ 2018-09-05 15:10 UTC (permalink / raw)
  To: stefan.wahren; +Cc: netdev, linux-kernel
In-Reply-To: <1536153798-16494-1-git-send-email-stefan.wahren@i2se.com>

From: Stefan Wahren <stefan.wahren@i2se.com>
Date: Wed,  5 Sep 2018 15:23:18 +0200

> With performance optimization the spi transfer and messages of basic
> register operations like qcaspi_read_register moved into the private
> driver structure. But they weren't protected against mutual access
> (e.g. between driver kthread and ethtool). So dumping the QCA7000
> registers via ethtool during network traffic could make spi_sync
> hang forever, because the completion in spi_message is overwritten.
> 
> So revert the optimization completely.
> 
> Fixes: 291ab06ecf676 ("net: qualcomm: new Ethernet over SPI driver for QCA700")
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* 4.18.6 dl_seq_start [xt_hashlimit]  unable to handle kernel NULL pointer dereference at 0000000000000050
From: Sami Farin @ 2018-09-05 10:55 UTC (permalink / raw)
  To: Linux Networking Mailing List; +Cc: Christoph Hellwig

4.17 worked ok, this with 32 GB Ryzen system.

BUG: unable to handle kernel NULL pointer dereference at 0000000000000050
PGD 0 P4D 0 
Oops: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 PID: 6303 Comm: grep Tainted: G                T 4.18.6+ #16
Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./X370 Taichi, BIOS P4.60 03/02/2018
RIP: 0010:dl_seq_start+0x11/0x60 [xt_hashlimit]
Code: ff 5d 41 5c 41 5d 41 5e 41 5f c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 89 f5 53 48 8b 87 d8 00 00 00 <48> 8b 78 50 e8 36 3b 6f de 48 89 c3 48 8d 78 48 e8 ca e6 0a df 8b 
RSP: 0018:ffffa79e88befde0 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffa79e88befe18 RDI: ffff9a64733417a0
RBP: ffffa79e88befe18 R08: 0000000000000000 R09: 00000000657547bf
R10: ffffffff9f2bf98a R11: ffff9a6470f5a6c0 R12: ffffa79e88befeb0
R13: ffff9a6471879200 R14: ffff9a6471879200 R15: ffff9a64733417a0
FS:  00007f6798784740(0000) GS:ffff9a649ea00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000050 CR3: 00000007f335c000 CR4: 00000000003406f0
Call Trace:
 seq_read+0xc0/0x470
 proc_reg_read+0x49/0x70
 vfs_read+0x8a/0x140
 ksys_read+0x52/0xc0
 do_syscall_64+0x6f/0x353
 ? trace_hardirqs_off_thunk+0x1a/0x1c
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7f67980eee21
Code: fe ff ff 50 48 8d 3d 46 b6 09 00 e8 f9 04 02 00 66 0f 1f 84 00 00 00 00 00 48 8d 05 c1 3b 2d 00 8b 00 85 c0 75 13 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 57 c3 66 0f 1f 44 00 00 41 54 49 89 d4 55 48 
RSP: 002b:00007ffc314f7a68 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 00007f67980eee21
RDX: 0000000000008000 RSI: 000055f0d317f000 RDI: 0000000000000003
RBP: 0000000000008000 R08: 0000000000000000 R09: 0000000000009008
R10: 0000000000000000 R11: 0000000000000246 R12: 000055f0d317f000
R13: 0000000000000003 R14: 000055f0d317e830 R15: 0000000000000003
Modules linked in: arptable_filter arp_tables nfnetlink_acct ip6table_mangle nf_log_ipv6 xt_hl ip6t_REJECT nf_reject_ipv6 ip6t_rt ip6table_filter ip6_tables ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat iptable_raw xt_mark xt_connmark iptable_mangle nf_log_ipv4 nf_log_common xt_LOG xt_length xt_limit ipt_REJECT nf_reject_ipv4 nf_conntrack_ipv4 nf_defrag_ipv4 xt_connlimit nf_conncount xt_multiport xt_hashlimit xt_owner xt_set xt_conntrack iptable_filter ip_set_bitmap_port ip_set_hash_mac ip_set_hash_net ip_set nf_conntrack_netlink nfnetlink bnep hwmon_vid iwlmvm snd_usb_audio snd_usbmidi_lib snd_hwdep snd_rawmidi mac80211 iwlwifi btusb btrtl kvm_amd btbcm btintel bluetooth kvm cfg80211 ecdh_generic irqbypass sp5100_tco wmi_bmof k10temp i2c_piix4 snd_hda_codec_realtek rfkill snd_hda_codec_ge
 neric
 snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core rtc_cmos acpi_cpufreq binfmt_misc snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device snd_pcm tcp_cubic tcp_westwood br_netfilter bridge stp llc ip_tables scsi_transport_iscsi algif_skcipher af_alg uas usb_storage usbhid mxm_wmi ccp igb xhci_pci xhci_hcd usbcore usb_common wmi button 8021q mrp sunrpc snd_timer snd soundcore fuse tun xt_tcpudp x_tables tcp_bbr nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack sch_fq_codel sch_htb sch_pie analog gameport joydev i2c_dev ecryptfs autofs4 amdkfd amd_iommu_v2 [last unloaded: pcspkr]
CR2: 0000000000000050
---[ end trace 0e097a943554aa36 ]---


-- 
Do what you love because life is too short for anything else.
https://samifar.in/

^ permalink raw reply

* [PATCH net-next] net/mlx5e: Make function mlx5i_grp_sw_update_stats() static
From: Wei Yongjun @ 2018-09-05 11:16 UTC (permalink / raw)
  To: Saeed Mahameed, Leon Romanovsky, Jason Gunthorpe, Or Gerlitz,
	Tariq Toukan, Feras Daoud, Alex Vesker, Erez Shitrit
  Cc: Wei Yongjun, netdev, linux-rdma, kernel-janitors

Fixes the following sparse warning:

drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:119:6: warning:
 symbol 'mlx5i_grp_sw_update_stats' was not declared. Should it be static?

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
index 48886b3..3dd9f88 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c
@@ -116,7 +116,7 @@ static void mlx5i_cleanup(struct mlx5e_priv *priv)
 	/* Do nothing .. */
 }
 
-void mlx5i_grp_sw_update_stats(struct mlx5e_priv *priv)
+static void mlx5i_grp_sw_update_stats(struct mlx5e_priv *priv)
 {
 	struct mlx5e_sw_stats s = { 0 };
 	int i, j;

^ permalink raw reply related

* [PATCH net-next v1] net/tls: Set count of SG entries if sk_alloc_sg returns -ENOSPC
From: Vakul Garg @ 2018-09-05 16:27 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg

tls_sw_sendmsg() allocates plaintext and encrypted SG entries using
function sk_alloc_sg(). In case the number of SG entries hit
MAX_SKB_FRAGS, sk_alloc_sg() returns -ENOSPC and sets the variable for
current SG index to '0'. This leads to calling of function
tls_push_record() with 'sg_encrypted_num_elem = 0' and later causes
kernel crash. To fix this, set the number of SG elements to the number
of elements in plaintext/encrypted SG arrays in case sk_alloc_sg()
returns -ENOSPC.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 net/tls/tls_sw.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index be4f2e990f9f..2dad3dc7be60 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -263,6 +263,9 @@ static int alloc_encrypted_sg(struct sock *sk, int len)
 			 &ctx->sg_encrypted_num_elem,
 			 &ctx->sg_encrypted_size, 0);
 
+	if (rc == -ENOSPC)
+		ctx->sg_encrypted_num_elem = ARRAY_SIZE(ctx->sg_encrypted_data);
+
 	return rc;
 }
 
@@ -276,6 +279,9 @@ static int alloc_plaintext_sg(struct sock *sk, int len)
 			 &ctx->sg_plaintext_num_elem, &ctx->sg_plaintext_size,
 			 tls_ctx->pending_open_record_frags);
 
+	if (rc == -ENOSPC)
+		ctx->sg_plaintext_num_elem = ARRAY_SIZE(ctx->sg_plaintext_data);
+
 	return rc;
 }
 
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH net-next] net/mlx5e: Make function mlx5i_grp_sw_update_stats() static
From: Leon Romanovsky @ 2018-09-05 11:25 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: Saeed Mahameed, Jason Gunthorpe, Or Gerlitz, Tariq Toukan,
	Feras Daoud, Alex Vesker, Erez Shitrit, netdev, linux-rdma,
	kernel-janitors
In-Reply-To: <1536146162-132732-1-git-send-email-weiyongjun1@huawei.com>

[-- Attachment #1: Type: text/plain, Size: 487 bytes --]

On Wed, Sep 05, 2018 at 11:16:02AM +0000, Wei Yongjun wrote:
> Fixes the following sparse warning:
>
> drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c:119:6: warning:
>  symbol 'mlx5i_grp_sw_update_stats' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* [PATCH net-next] bnxt_en: remove set but not used variable 'addr_type'
From: YueHaibing @ 2018-09-05 11:44 UTC (permalink / raw)
  To: Michael Chan, David S. Miller; +Cc: YueHaibing, netdev, kernel-janitors

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c: In function 'bnxt_tc_parse_flow':
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:186:6: warning:
 variable 'addr_type' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 092c817..f4ba9b3 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -181,7 +181,6 @@ static int bnxt_tc_parse_flow(struct bnxt *bp,
 			      struct bnxt_tc_flow *flow)
 {
 	struct flow_dissector *dissector = tc_flow_cmd->dissector;
-	u16 addr_type = 0;
 
 	/* KEY_CONTROL and KEY_BASIC are needed for forming a meaningful key */
 	if ((dissector->used_keys & BIT(FLOW_DISSECTOR_KEY_CONTROL)) == 0 ||
@@ -191,13 +190,6 @@ static int bnxt_tc_parse_flow(struct bnxt *bp,
 		return -EOPNOTSUPP;
 	}
 
-	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_CONTROL)) {
-		struct flow_dissector_key_control *key =
-			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_CONTROL);
-
-		addr_type = key->addr_type;
-	}
-
 	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_BASIC)) {
 		struct flow_dissector_key_basic *key =
 			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_BASIC);
@@ -293,13 +285,6 @@ static int bnxt_tc_parse_flow(struct bnxt *bp,
 		flow->l4_mask.icmp.code = mask->code;
 	}
 
-	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
-		struct flow_dissector_key_control *key =
-			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_ENC_CONTROL);
-
-		addr_type = key->addr_type;
-	}
-
 	if (dissector_uses_key(dissector, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS)) {
 		struct flow_dissector_key_ipv4_addrs *key =
 			GET_KEY(tc_flow_cmd, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS);

^ permalink raw reply related

* RE: [PATCH v2 1/2] net: ethernet: i40e: fix build error
From: Keller, Jacob E @ 2018-09-05 16:52 UTC (permalink / raw)
  To: Wang Dongsheng, Kirsher, Jeffrey T,
	sergei.shtylyov@cogentembedded.com
  Cc: davem@davemloft.net, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1536114430-21356-1-git-send-email-dongsheng.wang@hxt-semitech.com>



> -----Original Message-----
> From: Wang Dongsheng [mailto:dongsheng.wang@hxt-semitech.com]
> Sent: Tuesday, September 04, 2018 7:27 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sergei.shtylyov@cogentembedded.com
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wang Dongsheng <dongsheng.wang@hxt-
> semitech.com>
> Subject: [PATCH v2 1/2] net: ethernet: i40e: fix build error
> 
> Remove "inline" from __i40e_add_stat_strings.
> 
> In file included from
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:9:0:
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function
> ‘__i40e_add_stat_strings’:
> drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h:193:20: error:
> function ‘__i40e_add_stat_strings’ can never be inlined because it uses
> variable argument lists
>  static inline void __i40e_add_stat_strings(u8 **p, const struct
> 					    i40e_stats stats[],
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

Thanks for the fix.

A bit off topic, but these two files in the i40e and i40evf share some code. Is there a good mechanism for sharing these between the two drivers that would allow the modules to be independent? That would be ideal.

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
> v2:
> 1. Move function.
> 2. Include a new patch at [2/2].
> 
> ---
>  .../net/ethernet/intel/i40e/i40e_ethtool.c    | 24 ++++++++++++++++++
>  .../ethernet/intel/i40e/i40e_ethtool_stats.h  | 25 ++-----------------
>  2 files changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index d7d3974beca2..f4a70d67a80a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1821,6 +1821,30 @@ static void i40e_get_ethtool_stats(struct net_device
> *netdev,
>  		  "ethtool stats count mismatch!");
>  }
> 
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < size; i++) {
> +		va_list args;
> +
> +		va_start(args, size);
> +		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> +		*p += ETH_GSTRING_LEN;
> +		va_end(args);
> +	}
> +}
> +
>  /**
>   * i40e_get_stat_strings - copy stat strings into supplied buffer
>   * @netdev: the netdev to collect strings for
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> index bba1cb0b658f..0874c352136a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40e_add_queue_stats(u64 **data, struct i40e_ring
> *ring)
>  	*data += size;
>  }
> 
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static inline void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> -				    const unsigned int size, ...)
> -{
> -	unsigned int i;
> -
> -	for (i = 0; i < size; i++) {
> -		va_list args;
> -
> -		va_start(args, size);
> -		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> -		*p += ETH_GSTRING_LEN;
> -		va_end(args);
> -	}
> -}
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...);
> 
>  /**
>   * 40e_add_stat_strings - copy stat strings into ethtool buffer
> --
> 2.18.0


^ permalink raw reply

* RE: [PATCH v2 2/2] net: ethernet: i40evf: fix potential build error
From: Keller, Jacob E @ 2018-09-05 16:52 UTC (permalink / raw)
  To: Wang Dongsheng, Kirsher, Jeffrey T,
	sergei.shtylyov@cogentembedded.com
  Cc: davem@davemloft.net, intel-wired-lan@lists.osuosl.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1536114430-21356-2-git-send-email-dongsheng.wang@hxt-semitech.com>



> -----Original Message-----
> From: Wang Dongsheng [mailto:dongsheng.wang@hxt-semitech.com]
> Sent: Tuesday, September 04, 2018 7:27 PM
> To: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>;
> sergei.shtylyov@cogentembedded.com
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>; davem@davemloft.net; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Wang Dongsheng <dongsheng.wang@hxt-
> semitech.com>
> Subject: [PATCH v2 2/2] net: ethernet: i40evf: fix potential build error
> 
> Can't have non-inline function in a header file.
> There is a risk of "Multiple definition" from cross-including.
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

> ---
>  .../intel/i40evf/i40e_ethtool_stats.h         | 25 ++-----------------
>  .../ethernet/intel/i40evf/i40evf_ethtool.c    | 24 ++++++++++++++++++
>  2 files changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> index 60b595dd8c39..d70a071f065f 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> +++ b/drivers/net/ethernet/intel/i40evf/i40e_ethtool_stats.h
> @@ -181,29 +181,8 @@ i40evf_add_queue_stats(u64 **data, struct i40e_ring
> *ring)
>  	*data += size;
>  }
> 
> -/**
> - * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> - * @p: ethtool supplied buffer
> - * @stats: stat definitions array
> - * @size: size of the stats array
> - *
> - * Format and copy the strings described by stats into the buffer pointed at
> - * by p.
> - **/
> -static void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> -				    const unsigned int size, ...)
> -{
> -	unsigned int i;
> -
> -	for (i = 0; i < size; i++) {
> -		va_list args;
> -
> -		va_start(args, size);
> -		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> -		*p += ETH_GSTRING_LEN;
> -		va_end(args);
> -	}
> -}
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...);
> 
>  /**
>   * 40e_add_stat_strings - copy stat strings into ethtool buffer
> diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> index 9319971c5c92..c9a54f6de61e 100644
> --- a/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40evf/i40evf_ethtool.c
> @@ -171,6 +171,30 @@ static void i40evf_get_priv_flag_strings(struct
> net_device *netdev, u8 *data)
>  	}
>  }
> 
> +/**
> + * __i40e_add_stat_strings - copy stat strings into ethtool buffer
> + * @p: ethtool supplied buffer
> + * @stats: stat definitions array
> + * @size: size of the stats array
> + *
> + * Format and copy the strings described by stats into the buffer pointed at
> + * by p.
> + **/
> +void __i40e_add_stat_strings(u8 **p, const struct i40e_stats stats[],
> +			     const unsigned int size, ...)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < size; i++) {
> +		va_list args;
> +
> +		va_start(args, size);
> +		vsnprintf(*p, ETH_GSTRING_LEN, stats[i].stat_string, args);
> +		*p += ETH_GSTRING_LEN;
> +		va_end(args);
> +	}
> +}
> +
>  /**
>   * i40evf_get_stat_strings - Get stat strings
>   * @netdev: network interface device structure
> --
> 2.18.0

^ permalink raw reply

* Re: [RFC PATCH net-next V2 0/6] XDP rx handler
From: David Ahern @ 2018-09-05 17:20 UTC (permalink / raw)
  To: Jason Wang, Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, netdev, linux-kernel, ast, daniel, mst
In-Reply-To: <9a1d9340-8fd0-4e27-0938-adf361fe6939@redhat.com>

[ sorry for the delay; focused on the nexthop RFC ]

On 8/20/18 12:34 AM, Jason Wang wrote:
> 
> 
> On 2018年08月18日 05:15, David Ahern wrote:
>> On 8/15/18 9:34 PM, Jason Wang wrote:
>>> I may miss something but BPF forbids loop. Without a loop how can we
>>> make sure all stacked devices is enumerated correctly without knowing
>>> the topology in advance?
>> netdev_for_each_upper_dev_rcu
>>
>> BPF helpers allow programs to do lookups in kernel tables, in this case
>> the ability to find an upper device that would receive the packet.
> 
> So if I understand correctly, you mean using
> netdev_for_each_upper_dev_rcu() inside a BPF helper? If yes, I think we
> may still need device specific logic. E.g for macvlan,
> netdev_for_each_upper_dev_rcu() enumerates all macvlan devices on top a
> lower device. But what we need is one of the macvlan that matches the
> dst mac address which is similar to what XDP rx handler did. And it
> would become more complicated if we have multiple layers of device.

My device lookup helper takes the base port index (starting device),
vlan protocol, vlan tag and dest mac. So, yes, the mac address is used
to uniquely identify the stacked device.

> 
> So let's consider a simple case, consider we have 5 macvlan devices:
> 
> macvlan0: doing some packet filtering before passing packets to TCP/IP
> stack
> macvlan1: modify packets and redirect to another interface
> macvlan2: modify packets and transmit packet back through XDP_TX
> macvlan3: deliver packets to AF_XDP
> macvtap0: deliver packets raw XDP to VM
> 
> So, with XDP rx handler, what we need to just to attach five different
> XDP programs to each macvlan device. Your idea is to do all things in
> the root device XDP program. This looks complicated and not flexible
> since it needs to care a lot of things, e.g adding/removing
> actions/policies. And XDP program needs to call BPF helper that use
> netdev_for_each_upper_dev_rcu() to work correctly with stacked device.
> 

Stacking on top of a nic port can have all kinds of combinations of
vlans, bonds, bridges, vlans on bonds and bridges, macvlans, etc. I
suspect trying to install a program for layer 3 forwarding on each one
and iteratively running the programs would kill the performance gained
from forwarding with xdp.

^ permalink raw reply

* [PATCH net 0/3] tls: don't leave keys in kernel memory
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Aviad Yehezkel, Boris Pismenny, Dave Watson,
	Ilya Lesokhin

There are a few places where the RX/TX key for a TLS socket is copied
to kernel memory. This series clears those memory areas when they're no
longer needed.

Sabrina Dubroca (3):
  tls: don't copy the key out of tls12_crypto_info_aes_gcm_128
  tls: clear key material from kernel memory when do_tls_setsockopt_conf
    fails
  tls: zero the crypto information from tls_context before freeing

 include/net/tls.h  |  1 +
 net/tls/tls_main.c | 16 +++++++++++++---
 net/tls/tls_sw.c   |  5 +----
 3 files changed, 15 insertions(+), 7 deletions(-)

-- 
2.18.0

^ permalink raw reply

* [PATCH net 1/3] tls: don't copy the key out of tls12_crypto_info_aes_gcm_128
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel,
	Dave Watson
In-Reply-To: <cover.1536152698.git.sd@queasysnail.net>

There's no need to copy the key to an on-stack buffer before calling
crypto_aead_setkey().

Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_sw.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 52fbe727d7c1..268053b04563 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1130,7 +1130,6 @@ void tls_sw_free_resources_rx(struct sock *sk)
 
 int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 {
-	char keyval[TLS_CIPHER_AES_GCM_128_KEY_SIZE];
 	struct tls_crypto_info *crypto_info;
 	struct tls12_crypto_info_aes_gcm_128 *gcm_128_info;
 	struct tls_sw_context_tx *sw_ctx_tx = NULL;
@@ -1259,9 +1258,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
 
 	ctx->push_pending_record = tls_sw_push_pending_record;
 
-	memcpy(keyval, gcm_128_info->key, TLS_CIPHER_AES_GCM_128_KEY_SIZE);
-
-	rc = crypto_aead_setkey(*aead, keyval,
+	rc = crypto_aead_setkey(*aead, gcm_128_info->key,
 				TLS_CIPHER_AES_GCM_128_KEY_SIZE);
 	if (rc)
 		goto free_aead;
-- 
2.18.0

^ permalink raw reply related

* [PATCH net 2/3] tls: clear key material from kernel memory when do_tls_setsockopt_conf fails
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel,
	Dave Watson
In-Reply-To: <cover.1536152698.git.sd@queasysnail.net>

Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/tls/tls_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 180b6640e531..0d432d025471 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -499,7 +499,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
 	goto out;
 
 err_crypto_info:
-	memset(crypto_info, 0, sizeof(*crypto_info));
+	memzero_explicit(crypto_info, sizeof(struct tls12_crypto_info_aes_gcm_128));
 out:
 	return rc;
 }
-- 
2.18.0

^ permalink raw reply related

* [PATCH net 3/3] tls: zero the crypto information from tls_context before freeing
From: Sabrina Dubroca @ 2018-09-05 13:21 UTC (permalink / raw)
  To: netdev
  Cc: Sabrina Dubroca, Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel,
	Dave Watson
In-Reply-To: <cover.1536152698.git.sd@queasysnail.net>

This contains key material in crypto_send_aes_gcm_128 and
crypto_recv_aes_gcm_128.

Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 include/net/tls.h  |  1 +
 net/tls/tls_main.c | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/net/tls.h b/include/net/tls.h
index d5c683e8bb22..2010d23112f9 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -180,6 +180,7 @@ struct tls_context {
 		struct tls_crypto_info crypto_recv;
 		struct tls12_crypto_info_aes_gcm_128 crypto_recv_aes_gcm_128;
 	};
+	char tls_crypto_ctx_end[0];
 
 	struct list_head list;
 	struct net_device *netdev;
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 0d432d025471..d3a57c0b2182 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -241,6 +241,16 @@ static void tls_write_space(struct sock *sk)
 	ctx->sk_write_space(sk);
 }
 
+static void tls_ctx_free(struct tls_context *ctx)
+{
+	if (!ctx)
+		return;
+
+	memzero_explicit(&ctx->crypto_send,
+			 offsetof(struct tls_context, tls_crypto_ctx_end));
+	kfree(ctx);
+}
+
 static void tls_sk_proto_close(struct sock *sk, long timeout)
 {
 	struct tls_context *ctx = tls_get_ctx(sk);
@@ -294,7 +304,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
 #else
 	{
 #endif
-		kfree(ctx);
+		tls_ctx_free(ctx);
 		ctx = NULL;
 	}
 
@@ -305,7 +315,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
 	 * for sk->sk_prot->unhash [tls_hw_unhash]
 	 */
 	if (free_ctx)
-		kfree(ctx);
+		tls_ctx_free(ctx);
 }
 
 static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
-- 
2.18.0

^ permalink raw reply related

* Re: [oss-drivers] Re: [RFC bpf-next PATCH] samples/bpf: xdp1 add XDP hardware offload option
From: Jesper Dangaard Brouer @ 2018-09-05 13:41 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Nick Viljoen, oss-drivers, netdev, John W. Linville, jhsiao,
	Quentin Monnet, brouer
In-Reply-To: <20180904185957.0d344534@cakuba>

On Tue, 4 Sep 2018 18:59:57 +0200
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:

> On Tue, 4 Sep 2018 18:49:33 +0200, Jesper Dangaard Brouer wrote:
> > On Tue, 4 Sep 2018 17:09:12 +0200
> > Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> >   
> > > On Tue, 04 Sep 2018 16:59:19 +0200, Jesper Dangaard Brouer wrote:    
> > > > Trying to use XDP hardware offloading via XDP_FLAGS_HW_MODE
> > > > and setting the ifindex in prog_load_attr.ifindex before
> > > > loading the BPF code via bpf_prog_load_xattr().
> > > > 
> > > > This unfortunately does not seem to work...
> > > > - Am I doing something wrong?
> > > > 
> > > > Notice, I also disable the map BPF_MAP_TYPE_PERCPU_ARRAY
> > > > to make sure it was not related to the map (not supporting
> > > > offloading).
> > > > 
> > > > Failed with:
> > > >  # ./xdp1 -O $(</sys/class/net/enp130s0np1/ifindex)
> > > >  libbpf: load bpf program failed: Invalid argument
> > > >  libbpf: failed to load program 'xdp1'
> > > >  libbpf: failed to load object './xdp1_kern.o'
> > > > 
> > > > Tested on kernel 4.18.0-2.el8.x86_64 with driver nfp
> > > >  Ethernet controller: Netronome Systems, Inc. Device 4000      
> > > 
> > > Are you running the BPF capable FW?
> > > 
> > > https://help.netronome.com/support/solutions/articles/36000050009-agilio-ebpf-2-0-6-extended-berkeley-packet-filter    
> > 
> > I'm likely not running the correct firmware...
> > 
> > Can you tell me, with the ethtool -i output, if I'm running the
> > appropriate firmware?
> > 
> > # ethtool -i enp129s0np1
> > driver: nfp
> > version: 4.18.0-2.el8.x86_64 SMP mod_unl
> > firmware-version: 0.0.3.5 0.21 nic-2.0.7 nic
> > expansion-rom-version: 
> > bus-info: 0000:81:00.0
> > supports-statistics: yes
> > supports-test: no
> > supports-eeprom-access: no
> > supports-register-dump: yes
> > supports-priv-flags: no  
> 
> Yup, the BPF firmware says bpf in firmware version.

Downloaded: agilio-bpf-firmware-2.0.6.121-1.noarch.rpm

RPM install it:
 rpm -hiv  agilio-bpf-firmware-2.0.6.121-1.noarch.rpm

# rpm -ql agilio-bpf-firmware
/opt/netronome/firmware/agilio-bpf
/opt/netronome/firmware/agilio-bpf/nic_AMDA0058-0011_2x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0058-0012_2x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0078-0011_1x100.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0081-0001_1x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0081-0001_4x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0096-0001_2x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0097-0001_2x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0097-0001_4x10_1x40.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0097-0001_8x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0099-0001_1x10_1x25.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0099-0001_2x10.nffw
/opt/netronome/firmware/agilio-bpf/nic_AMDA0099-0001_2x25.nffw

Netronome: Basic Firmware User Guide
 https://help.netronome.com/support/solutions/articles/36000049975-basic-firmware-user-guide

A section says after installing the firmware, unload and reload the
driver kernel module will upgrade the firmware.

 ## reload driver to load new firmware
 rmmod nfp; modprobe nfp

Firmware upgrade worked! :-) This is by-far the easiest firmware
upgrade I've experienced.  And I don't have to reboot the machine :-)

 # ethtool -i enp130s0np1  | grep firmware-version
 firmware-version: 0.0.3.5 0.21 bpf-2.0.6.121 ebpf

Notice the bpf/ebpf strings in firmware-version.

 
> > If this is a firmware version case, then we should really improve the
> > errors we are giving the user, the -EINVAL can be anything.
> > 
> >  "libbpf: load bpf program failed: Invalid argument"  
> 
> That is true.

Same goes for improving the error message, when loading a map type that
offload cannot handle.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH v3 2/3] IB/ipoib: Use dev_port to expose network interface port numbers
From: Leon Romanovsky @ 2018-09-05 13:41 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180903161316.25121-3-ar@cs.msu.ru>

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

On Mon, Sep 03, 2018 at 07:13:15PM +0300, Arseny Maslennikov wrote:
> Some InfiniBand network devices have multiple ports on the same PCI
> function. This initializes the `dev_port' sysfs field of those
> network interfaces with their port number.
>
> Prior to this the kernel erroneously used the `dev_id' sysfs
> field of those network interfaces to convey the port number to userspace.
>
> The use of `dev_id' was considered correct until Linux 3.15,
> when another field, `dev_port', was defined for this particular
> purpose and `dev_id' was reserved for distinguishing stacked ifaces
> (e.g: VLANs) with the same hardware address as their parent device.
>
> Similar fixes to net/mlx4_en and many other drivers, which started
> exporting this information through `dev_id' before 3.15, were accepted
> into the kernel 4 years ago.
> See 76a066f2a2a0 (`net/mlx4_en: Expose port number through sysfs').
>
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 2 ++
>  1 file changed, 2 insertions(+)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH bpf-next] bpf/verifier: properly clear union members after a ctx read
From: Edward Cree @ 2018-09-05 13:47 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: daniel, ast, netdev
In-Reply-To: <20180905022323.6lkmq2kmv5ejwy3c@ast-mbp.dhcp.thefacebook.com>

On 05/09/18 03:23, Alexei Starovoitov wrote:
> So would you agree it's fair to add
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> ?
Sure.  Though I don't think it needs backporting, as it's a conservative
 bug (i.e. it merely prevents pruning, but that's safe security-wise).
> How about patch like the following:
> ------------
> From 422fd975ed78645ab67d2eb50ff6e1ff6fb3de32 Mon Sep 17 00:00:00 2001
> From: Alexei Starovoitov <ast@kernel.org>
> Date: Tue, 4 Sep 2018 19:13:44 -0700
> Subject: [PATCH] bpf/verifier: fix verifier instability
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Debugged-by: Edward Cree <ecree@solarflare.com> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Edward Cree <ecree@solarflare.com>

^ permalink raw reply

* RE: [PATCH net 3/3] tls: zero the crypto information from tls_context before freeing
From: Vakul Garg @ 2018-09-05 13:48 UTC (permalink / raw)
  To: Sabrina Dubroca, netdev@vger.kernel.org
  Cc: Boris Pismenny, Ilya Lesokhin, Aviad Yehezkel, Dave Watson
In-Reply-To: <c2b629a06486cd9c17a42cf2efe11ad26d8f9ef6.1536152698.git.sd@queasysnail.net>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Sabrina Dubroca
> Sent: Wednesday, September 5, 2018 6:52 PM
> To: netdev@vger.kernel.org
> Cc: Sabrina Dubroca <sd@queasysnail.net>; Boris Pismenny
> <borisp@mellanox.com>; Ilya Lesokhin <ilyal@mellanox.com>; Aviad
> Yehezkel <aviadye@mellanox.com>; Dave Watson <davejwatson@fb.com>
> Subject: [PATCH net 3/3] tls: zero the crypto information from tls_context
> before freeing
> 
> This contains key material in crypto_send_aes_gcm_128 and
> crypto_recv_aes_gcm_128.
> 
> Fixes: 3c4d7559159b ("tls: kernel TLS support")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>  include/net/tls.h  |  1 +
>  net/tls/tls_main.c | 14 ++++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/tls.h b/include/net/tls.h index
> d5c683e8bb22..2010d23112f9 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -180,6 +180,7 @@ struct tls_context {
>  		struct tls_crypto_info crypto_recv;
>  		struct tls12_crypto_info_aes_gcm_128
> crypto_recv_aes_gcm_128;
>  	};
> +	char tls_crypto_ctx_end[0];

This makes the key zeroization dependent upon the position of unions in structure.
Can you zeroise the crypto_send, crypto_recv separately using two memzero_explicit calls? 

> 
>  	struct list_head list;
>  	struct net_device *netdev;
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index
> 0d432d025471..d3a57c0b2182 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -241,6 +241,16 @@ static void tls_write_space(struct sock *sk)
>  	ctx->sk_write_space(sk);
>  }
> 
> +static void tls_ctx_free(struct tls_context *ctx) {
> +	if (!ctx)
> +		return;
> +
> +	memzero_explicit(&ctx->crypto_send,
> +			 offsetof(struct tls_context, tls_crypto_ctx_end));
> +	kfree(ctx);
> +}
> +
>  static void tls_sk_proto_close(struct sock *sk, long timeout)  {
>  	struct tls_context *ctx = tls_get_ctx(sk); @@ -294,7 +304,7 @@
> static void tls_sk_proto_close(struct sock *sk, long timeout)  #else
>  	{
>  #endif
> -		kfree(ctx);
> +		tls_ctx_free(ctx);
>  		ctx = NULL;
>  	}
> 
> @@ -305,7 +315,7 @@ static void tls_sk_proto_close(struct sock *sk, long
> timeout)
>  	 * for sk->sk_prot->unhash [tls_hw_unhash]
>  	 */
>  	if (free_ctx)
> -		kfree(ctx);
> +		tls_ctx_free(ctx);
>  }
> 
>  static int do_tls_getsockopt_tx(struct sock *sk, char __user *optval,
> --
> 2.18.0

^ permalink raw reply

* Re: [PATCH v3 3/3] IB/ipoib: Log sysfs 'dev_id' accesses from userspace
From: Leon Romanovsky @ 2018-09-05 13:50 UTC (permalink / raw)
  To: Arseny Maslennikov; +Cc: linux-rdma, Doug Ledford, Jason Gunthorpe, netdev
In-Reply-To: <20180903161316.25121-4-ar@cs.msu.ru>

[-- Attachment #1: Type: text/plain, Size: 2324 bytes --]

On Mon, Sep 03, 2018 at 07:13:16PM +0300, Arseny Maslennikov wrote:
> Signed-off-by: Arseny Maslennikov <ar@cs.msu.ru>
> ---
>  drivers/infiniband/ulp/ipoib/ipoib_main.c | 38 +++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 30f840f874b3..7386e5bde3d3 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -2386,6 +2386,42 @@ int ipoib_add_pkey_attr(struct net_device *dev)
>  	return device_create_file(&dev->dev, &dev_attr_pkey);
>  }
>
> +/*
> + * We erroneously exposed the iface's port number in the dev_id
> + * sysfs field long after dev_port was introduced for that purpose[1],
> + * and we need to stop everyone from relying on that.
> + * Let's overload the shower routine for the dev_id file here
> + * to gently bring the issue up.
> + *
> + * [1] https://www.spinics.net/lists/netdev/msg272123.html
> + */
> +static ssize_t dev_id_show(struct device *dev,
> +			   struct device_attribute *attr, char *buf)
> +{
> +	struct net_device *ndev = to_net_dev(dev);
> +	ssize_t ret = -EINVAL;
> +
> +	if (ndev->dev_id == ndev->dev_port) {
> +		netdev_info_once(ndev,
> +			"\"%s\" wants to know my dev_id. "
> +			"Should it look at dev_port instead?\n",
> +			current->comm);
> +		netdev_info_once(ndev,
> +			"See Documentation/ABI/testing/sysfs-class-net for more info.\n");
> +	}
> +
> +	ret = sprintf(buf, "%#x\n", ndev->dev_id);
> +
> +	return ret;
> +}
> +static DEVICE_ATTR_RO(dev_id);
> +

I don't see this field among exposed by IPoIB, why should we expose it now?

> +int ipoib_intercept_dev_id_attr(struct net_device *dev)
> +{
> +	device_remove_file(&dev->dev, &dev_attr_dev_id);
> +	return device_create_file(&dev->dev, &dev_attr_dev_id);

Why isn't enough to rely on netdev code?

> +}
> +
>  static struct net_device *ipoib_add_port(const char *format,
>  					 struct ib_device *hca, u8 port)
>  {
> @@ -2427,6 +2463,8 @@ static struct net_device *ipoib_add_port(const char *format,
>  	 */
>  	ndev->priv_destructor = ipoib_intf_free;
>
> +	if (ipoib_intercept_dev_id_attr(ndev))
> +		goto sysfs_failed;
>  	if (ipoib_cm_add_mode_attr(ndev))
>  		goto sysfs_failed;
>  	if (ipoib_add_pkey_attr(ndev))
> --
> 2.19.0.rc1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH net 2/3] tls: clear key material from kernel memory when do_tls_setsockopt_conf fails
From: Boris Pismenny @ 2018-09-05 13:53 UTC (permalink / raw)
  To: Sabrina Dubroca, netdev; +Cc: Ilya Lesokhin, Aviad Yehezkel, Dave Watson
In-Reply-To: <695fc3449c2ba927e97abd48b452a31ec73d6c61.1536152698.git.sd@queasysnail.net>

Hi Sabrina,

On 9/5/2018 4:21 PM, Sabrina Dubroca wrote:
> Fixes: 3c4d7559159b ("tls: kernel TLS support")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> ---
>   net/tls/tls_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 180b6640e531..0d432d025471 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -499,7 +499,7 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
>   	goto out;
>   
>   err_crypto_info:
> -	memset(crypto_info, 0, sizeof(*crypto_info));
> +	memzero_explicit(crypto_info, sizeof(struct tls12_crypto_info_aes_gcm_128));

Besides the key, there are other (not secret) information in 
tls12_crypto_info_aes_gcm_128. I'd prefer you do not delete it to enable 
users to obtain it (using getsockopt) in case we decide to implement a 
fallback to userspace in the future. Such a fallback must obtain the 
kernel's iv, and record sequence number.

Thanks,
Boris.

^ 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