Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Ard Biesheuvel @ 2018-09-11 10:08 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Linux Kernel Mailing List, <netdev@vger.kernel.org>,
	David S. Miller, Greg Kroah-Hartman, Andy Lutomirski,
	Samuel Neves, Jean-Philippe Aumasson,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE
In-Reply-To: <20180911010838.8818-3-Jason@zx2c4.com>

On 11 September 2018 at 03:08, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Zinc stands for "Zinc Is Neat Crypto" or "Zinc as IN Crypto" or maybe
> just "Zx2c4's INsane Cryptolib." It's also short, easy to type, and
> plays nicely with the recent trend of naming crypto libraries after
> elements. The guiding principle is "don't overdo it". It's less of a
> library and more of a directory tree for organizing well-curated direct
> implementations of cryptography primitives.
>
> Zinc is a new cryptography API that is much more minimal and lower-level
> than the current one. It intends to complement it and provide a basis
> upon which the current crypto API might build, as the provider of
> software implementations of cryptographic primitives. It is motivated by
> three primary observations in crypto API design:
>
>   * Highly composable "cipher modes" and related abstractions from
>     90s cryptographers did not turn out to be as terrific an idea as
>     hoped, leading to a host of API misuse problems.
>
>   * Most programmers are afraid of crypto code, and so prefer to
>     integrate it into libraries in a highly abstracted manner, so as to
>     shield themselves from implementation details. Cryptographers, on
>     the other hand, prefer simple direct implementations, which they're
>     able to verify for high assurance and optimize in accordance with
>     their expertise.
>
>   * Overly abstracted and flexible cryptography APIs lead to a host of
>     dangerous problems and performance issues. The kernel is in the
>     business usually not of coming up with new uses of crypto, but
>     rather implementing various constructions, which means it essentially
>     needs a library of primitives, not a highly abstracted enterprise-ready
>     pluggable system, with a few particular exceptions.
>
> This last observation has seen itself play out several times over and
> over again within the kernel:
>
>   * The perennial move of actual primitives away from crypto/ and into
>     lib/, so that users can actually call these functions directly with
>     no overhead and without lots of allocations, function pointers,
>     string specifier parsing, and general clunkiness. For example:
>     sha256, chacha20, siphash, sha1, and so forth live in lib/ rather
>     than in crypto/. Zinc intends to stop the cluttering of lib/ and
>     introduce these direct primitives into their proper place, lib/zinc/.
>
>   * An abundance of misuse bugs with the present crypto API that have
>     been very unpleasant to clean up.
>
>   * A hesitance to even use cryptography, because of the overhead and
>     headaches involved in accessing the routines.
>
> Zinc goes in a rather different direction. Rather than providing a
> thoroughly designed and abstracted API, Zinc gives you simple functions,
> which implement some primitive, or some particular and specific
> construction of primitives. It is not dynamic in the least, though one
> could imagine implementing a complex dynamic dispatch mechanism (such as
> the current crypto API) on top of these basic functions. After all,
> dynamic dispatch is usually needed for applications with cipher agility,
> such as IPsec, dm-crypt, AF_ALG, and so forth, and the existing crypto
> API will continue to play that role. However, Zinc will provide a non-
> haphazard way of directly utilizing crypto routines in applications
> that do have neither the need nor desire for abstraction and dynamic
> dispatch.
>
> It also organizes the implementations in a simple, straight-forward,
> and direct manner, making it enjoyable and intuitive to work on.
> Rather than moving optimized assembly implementations into arch/, it
> keeps them all together in lib/zinc/, making it simple and obvious to
> compare and contrast what's happening. This is, notably, exactly what
> the lib/raid6/ tree does, and that seems to work out rather well. It's
> also the pattern of most successful crypto libraries. The architecture-
> specific glue-code is made a part of each translation unit, rather than
> being in a separate one, so that generic and architecture-optimized code
> are combined at compile-time, and incompatibility branches compiled out by
> the optimizer.
>
> All implementations have been extensively tested and fuzzed, and are
> selected for their quality, trustworthiness, and performance. Wherever
> possible and performant, formally verified implementations are used,
> such as those from HACL* [1] and Fiat-Crypto [2]. The routines also take
> special care to zero out secrets using memzero_explicit (and future work
> is planned to have gcc do this more reliably and performantly with
> compiler plugins). The performance of the selected implementations is
> state-of-the-art and unrivaled on a broad array of hardware, though of
> course we will continue to fine tune these to the hardware demands
> needed by kernel contributors. Each implementation also comes with
> extensive self-tests and crafted test vectors, pulled from various
> places such as Wycheproof [9].
>
> Regularity of function signatures is important, so that users can easily
> "guess" the name of the function they want. Though, individual
> primitives are oftentimes not trivially interchangeable, having been
> designed for different things and requiring different parameters and
> semantics, and so the function signatures they provide will directly
> reflect the realities of the primitives' usages, rather than hiding it
> behind (inevitably leaky) abstractions. Also, in contrast to the current
> crypto API, Zinc functions can work on stack buffers, and can be called
> with different keys, without requiring allocations or locking.
>
> SIMD is used automatically when available, though some routines may
> benefit from either having their SIMD disabled for particular
> invocations, or to have the SIMD initialization calls amortized over
> several invocations of the function, and so Zinc utilizes function
> signatures enabling that in conjunction with the recently introduced
> simd_context_t.
>
> More generally, Zinc provides function signatures that allow just what
> is required by the various callers. This isn't to say that users of the
> functions will be permitted to pollute the function semantics with weird
> particular needs, but we are trying very hard not to overdo it, and that
> means looking carefully at what's actually necessary, and doing just that,
> and not much more than that. Remember: practicality and cleanliness rather
> than over-zealous infrastructure.
>
> Zinc provides also an opening for the best implementers in academia to
> contribute their time and effort to the kernel, by being sufficiently
> simple and inviting. In discussing this commit with some of the best and
> brightest over the last few years, there are many who are eager to
> devote rare talent and energy to this effort.
>
> Following the merging of this, I expect for the primitives that
> currently exist in lib/ to work their way into lib/zinc/, after intense
> scrutiny of each implementation, potentially replacing them with either
> formally-verified implementations, or better studied and faster
> state-of-the-art implementations.
>
> Also following the merging of this, I expect for the old crypto API
> implementations to be ported over to use Zinc for their software-based
> implementations.
>
> As Zinc is simply library code, its config options are un-menued, with
> the exception of CONFIG_ZINC_DEBUG, which enables various selftests and
> BUG_ONs.
>

In spite of the wall of text, you fail to point out exactly why the
existing AEAD API in unsuitable, and why fixing it is not an option.

As I pointed out in a previous version, I don't think we need a
separate crypto API/library in the kernel, and I don't think you have
convinced anyone else yet either.

Perhaps you can devote /your/ rare talent and energy to improving what
we already have for everybody's sake, rather than providing a
completely separate crypto stack that only benefits WireGuard (unless
you yourself port the existing crypto API software algorithms to this
crypto stack first and present *that* work as a convincing case in
itself)

I won't go into the 1000s lines of generated assembly again - you
already know my position on that topic.

Please refrain from sending a v4 with just a couple of more tweaks on
top - these are fundamental issues that require discussion before
there is any chance of this being merged.


> [1] https://github.com/project-everest/hacl-star
> [2] https://github.com/mit-plv/fiat-crypto
> [3] https://cr.yp.to/ecdh.html
> [4] https://cr.yp.to/chacha.html
> [5] https://cr.yp.to/snuffle/xsalsa-20081128.pdf
> [6] https://cr.yp.to/mac.html
> [7] https://blake2.net/
> [8] https://tools.ietf.org/html/rfc8439
> [9] https://github.com/google/wycheproof
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Samuel Neves <sneves@dei.uc.pt>
> Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
> Cc: linux-crypto@vger.kernel.org
> ---
>  MAINTAINERS       |  8 ++++++++
>  lib/Kconfig       |  2 ++
>  lib/Makefile      |  2 ++
>  lib/zinc/Kconfig  | 20 ++++++++++++++++++++
>  lib/zinc/Makefile |  8 ++++++++
>  lib/zinc/main.c   | 31 +++++++++++++++++++++++++++++++
>  6 files changed, 71 insertions(+)
>  create mode 100644 lib/zinc/Kconfig
>  create mode 100644 lib/zinc/Makefile
>  create mode 100644 lib/zinc/main.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ef884b883c3..d2092e52320d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16160,6 +16160,14 @@ Q:     https://patchwork.linuxtv.org/project/linux-media/list/
>  S:     Maintained
>  F:     drivers/media/dvb-frontends/zd1301_demod*
>
> +ZINC CRYPTOGRAPHY LIBRARY
> +M:     Jason A. Donenfeld <Jason@zx2c4.com>
> +M:     Samuel Neves <sneves@dei.uc.pt>
> +S:     Maintained
> +F:     lib/zinc/
> +F:     include/zinc/
> +L:     linux-crypto@vger.kernel.org
> +
>  ZPOOL COMPRESSED PAGE STORAGE API
>  M:     Dan Streetman <ddstreet@ieee.org>
>  L:     linux-mm@kvack.org
> diff --git a/lib/Kconfig b/lib/Kconfig
> index a3928d4438b5..3e6848269c66 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -485,6 +485,8 @@ config GLOB_SELFTEST
>           module load) by a small amount, so you're welcome to play with
>           it, but you probably don't need it.
>
> +source "lib/zinc/Kconfig"
> +
>  #
>  # Netlink attribute parsing support is select'ed if needed
>  #
> diff --git a/lib/Makefile b/lib/Makefile
> index ca3f7ebb900d..3f16e35d2c11 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -214,6 +214,8 @@ obj-$(CONFIG_PERCPU_TEST) += percpu_test.o
>
>  obj-$(CONFIG_ASN1) += asn1_decoder.o
>
> +obj-$(CONFIG_ZINC) += zinc/
> +
>  obj-$(CONFIG_FONT_SUPPORT) += fonts/
>
>  obj-$(CONFIG_PRIME_NUMBERS) += prime_numbers.o
> diff --git a/lib/zinc/Kconfig b/lib/zinc/Kconfig
> new file mode 100644
> index 000000000000..aa4f8d449d6b
> --- /dev/null
> +++ b/lib/zinc/Kconfig
> @@ -0,0 +1,20 @@
> +config ZINC
> +       tristate
> +       select CRYPTO_BLKCIPHER
> +       select VFP
> +       select VFPv3
> +       select NEON
> +       select KERNEL_MODE_NEON
> +
> +config ZINC_DEBUG
> +       bool "Zinc cryptography library debugging and self-tests"
> +       depends on ZINC
> +       help
> +         This builds a series of self-tests for the Zinc crypto library, which
> +         help diagnose any cryptographic algorithm implementation issues that
> +         might be at the root cause of potential bugs. It also adds various
> +         debugging traps.
> +
> +         Unless you're developing and testing cryptographic routines, or are
> +         especially paranoid about correctness on your hardware, you may say
> +         N here.
> diff --git a/lib/zinc/Makefile b/lib/zinc/Makefile
> new file mode 100644
> index 000000000000..dad47573de42
> --- /dev/null
> +++ b/lib/zinc/Makefile
> @@ -0,0 +1,8 @@
> +ccflags-y := -O3
> +ccflags-y += -Wframe-larger-than=8192
> +ccflags-y += -D'pr_fmt(fmt)=KBUILD_MODNAME ": " fmt'
> +ccflags-$(CONFIG_ZINC_DEBUG) += -DDEBUG
> +
> +zinc-y += main.o
> +
> +obj-$(CONFIG_ZINC) := zinc.o
> diff --git a/lib/zinc/main.c b/lib/zinc/main.c
> new file mode 100644
> index 000000000000..ceece33ff5a7
> --- /dev/null
> +++ b/lib/zinc/main.c
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +
> +#ifdef DEBUG
> +#define selftest(which) do { \
> +       if (!which ## _selftest()) \
> +               return -ENOTRECOVERABLE; \
> +} while (0)
> +#else
> +#define selftest(which)
> +#endif
> +
> +static int __init mod_init(void)
> +{
> +       return 0;
> +}
> +
> +static void __exit mod_exit(void)
> +{
> +}
> +
> +module_init(mod_init);
> +module_exit(mod_exit);
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Zinc cryptography library");
> +MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
> --
> 2.18.0
>

^ permalink raw reply

* Re: [PATCH 0/3] xen-netback: hash mapping hanling adjustments
From: Wei Liu @ 2018-09-11 10:16 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Paul Durrant, Wei Liu, davem, xen-devel, netdev
In-Reply-To: <5B9778D702000078001E7069@prv1-mh.provo.novell.com>

On Tue, Sep 11, 2018 at 02:12:07AM -0600, Jan Beulich wrote:
> >>> On 28.08.18 at 16:54,  wrote:
> > First and foremost the fix for XSA-270. On top of that further changes
> > which looked desirable to me while investigating that XSA.
> > 
> > 1: fix input validation in xenvif_set_hash_mapping()
> > 2: validate queue numbers in xenvif_set_hash_mapping()
> > 3: handle page straddling in xenvif_set_hash_mapping()
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> What is the way forward here? I've got R-b-s from Paul for all three
> patches, and a minor change request on patch 2 from Wei. I'm not
> really certain what to do in this case (hints appreciated), but could
> at least the security fix (patch 1) be applied immediately?

If you happen to resend, please make the adjustment; otherwise I'm fine
with the patches as they are. I don't want to block useful things on
cosmetic issues.

Wei.

^ permalink raw reply

* Re: unexpected GRO/veth behavior
From: Eric Dumazet @ 2018-09-11 10:27 UTC (permalink / raw)
  To: Paolo Abeni, netdev; +Cc: eric.dumazet, Toshiaki Makita
In-Reply-To: <bcea5cafe139e2c561f036f5b3e2598d7d0dd970.camel@redhat.com>



On 09/10/2018 11:54 PM, Paolo Abeni wrote:
> Hi,
> 
> On Mon, 2018-09-10 at 16:44 +0200, Paolo Abeni wrote:
>> while testing some local patches I observed that the TCP tput in the
>> following scenario:
>>
>> # the following enable napi on veth0, so that we can trigger the
>> # GRO path with namespaces
>> ip netns add test
>> ip link add type veth
>> ip link set dev veth0 netns test
>> ip -n test link set lo up
>> ip -n test link set veth0 up
>> ip -n test addr add dev veth0 172.16.1.2/24
>> ip link set dev veth1 up
>> ip addr add dev veth1 172.16.1.1/24
>> IDX=`ip netns exec test cat /sys/class/net/veth0/ifindex`
>>
>> # 'xdp_pass' is a NO-OP XDP program that simply return XDP_PASS
>> ip netns exec test ./xdp_pass $IDX &
>> taskset 0x2 ip netns exec test iperf3 -s -i 60 &
>> taskset 0x1 iperf3 -c 172.16.1.2 -t 60 -i 60
> 
> In the same scenario, using instead:
> 
> iperf3 -c 172.16.1.2 -t 600  -i 60 -N -l 10K
> 
> I hit the following splat, on a recent, unpatched net-next:
> 
> [  362.098904] refcount_t overflow at skb_set_owner_w+0x5e/0xa0 in iperf3[1644], uid/euid: 0/0
> [  362.108239] WARNING: CPU: 0 PID: 1644 at kernel/panic.c:648 refcount_error_report+0xa0/0xa4
> [  362.117547] Modules linked in: tcp_diag inet_diag veth intel_rapl sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel intel_cstate intel_uncore intel_rapl_perf ipmi_ssif iTCO_wdt sg ipmi_si iTCO_vendor_support ipmi_devintf mxm_wmi ipmi_msghandler pcspkr dcdbas mei_me wmi mei lpc_ich acpi_power_meter pcc_cpufreq xfs libcrc32c sd_mod mgag200 drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ixgbe igb ttm ahci mdio libahci ptp crc32c_intel drm pps_core libata i2c_algo_bit dca dm_mirror dm_region_hash dm_log dm_mod
> [  362.176622] CPU: 0 PID: 1644 Comm: iperf3 Not tainted 4.19.0-rc2.vanilla+ #2025
> [  362.184777] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.1.7 06/16/2016
> [  362.193124] RIP: 0010:refcount_error_report+0xa0/0xa4
> [  362.198758] Code: 08 00 00 48 8b 95 80 00 00 00 49 8d 8c 24 80 0a 00 00 41 89 c1 44 89 2c 24 48 89 de 48 c7 c7 18 4d e7 9d 31 c0 e8 30 fa ff ff <0f> 0b eb 88 0f 1f 44 00 00 55 48 89 e5 41 56 41 55 41 54 49 89 fc
> [  362.219711] RSP: 0018:ffff9ee6ff603c20 EFLAGS: 00010282
> [  362.225538] RAX: 0000000000000000 RBX: ffffffff9de83e10 RCX: 0000000000000000
> [  362.233497] RDX: 0000000000000001 RSI: ffff9ee6ff6167d8 RDI: ffff9ee6ff6167d8
> [  362.241457] RBP: ffff9ee6ff603d78 R08: 0000000000000490 R09: 0000000000000004
> [  362.249416] R10: 0000000000000000 R11: ffff9ee6ff603990 R12: ffff9ee664b94500
> [  362.257377] R13: 0000000000000000 R14: 0000000000000004 R15: ffffffff9de615f9
> [  362.265337] FS:  00007f1d22d28740(0000) GS:ffff9ee6ff600000(0000) knlGS:0000000000000000
> [  362.274363] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  362.280773] CR2: 00007f1d222f35d0 CR3: 0000001fddfec003 CR4: 00000000001606f0
> [  362.288733] Call Trace:
> [  362.291459]  <IRQ>
> [  362.293702]  ex_handler_refcount+0x4e/0x80
> [  362.298269]  fixup_exception+0x35/0x40
> [  362.302451]  do_trap+0x109/0x150
> [  362.306048]  do_error_trap+0xd5/0x130
> [  362.315766]  invalid_op+0x14/0x20
> [  362.319460] RIP: 0010:skb_set_owner_w+0x5e/0xa0
> [  362.324512] Code: ef ff ff 74 49 48 c7 43 60 20 7b 4a 9d 8b 85 f4 01 00 00 85 c0 75 16 8b 83 e0 00 00 00 f0 01 85 44 01 00 00 0f 88 d8 23 16 00 <5b> 5d c3 80 8b 91 00 00 00 01 8b 85 f4 01 00 00 89 83 a4 00 00 00
> [  362.345465] RSP: 0018:ffff9ee6ff603e20 EFLAGS: 00010a86
> [  362.351291] RAX: 0000000000001100 RBX: ffff9ee65deec700 RCX: ffff9ee65e829244
> [  362.359250] RDX: 0000000000000100 RSI: ffff9ee65e829100 RDI: ffff9ee65deec700
> [  362.367210] RBP: ffff9ee65e829100 R08: 000000000002a380 R09: 0000000000000000
> [  362.375169] R10: 0000000000000002 R11: fffff1a4bf77bb00 R12: ffffc0754661d000
> [  362.383130] R13: ffff9ee65deec200 R14: ffff9ee65f597000 R15: 00000000000000aa
> [  362.391092]  veth_xdp_rcv+0x4e4/0x890 [veth]
> [  362.399357]  veth_poll+0x4d/0x17a [veth]
> [  362.403731]  net_rx_action+0x2af/0x3f0
> [  362.407912]  __do_softirq+0xdd/0x29e
> [  362.411897]  do_softirq_own_stack+0x2a/0x40
> [  362.416561]  </IRQ>
> [  362.418899]  do_softirq+0x4b/0x70
> [  362.422594]  __local_bh_enable_ip+0x50/0x60
> [  362.427258]  ip_finish_output2+0x16a/0x390
> [  362.431824]  ip_output+0x71/0xe0
> [  362.440670]  __tcp_transmit_skb+0x583/0xab0
> [  362.445333]  tcp_write_xmit+0x247/0xfb0
> [  362.449609]  __tcp_push_pending_frames+0x2d/0xd0
> [  362.454760]  tcp_sendmsg_locked+0x857/0xd30
> [  362.459424]  tcp_sendmsg+0x27/0x40
> [  362.463216]  sock_sendmsg+0x36/0x50
> [  362.467104]  sock_write_iter+0x87/0x100
> [  362.471382]  __vfs_write+0x112/0x1a0
> [  362.475369]  vfs_write+0xad/0x1a0
> [  362.479062]  ksys_write+0x52/0xc0
> [  362.482759]  do_syscall_64+0x5b/0x180
> [  362.486841]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [  362.492473] RIP: 0033:0x7f1d22293238
> [  362.496458] Code: 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 c5 54 2d 00 8b 00 85 c0 75 17 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 41 54 49 89 d4 55
> [  362.517409] RSP: 002b:00007ffebaef8008 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> [  362.525855] RAX: ffffffffffffffda RBX: 0000000000002800 RCX: 00007f1d22293238
> [  362.533816] RDX: 0000000000002800 RSI: 00007f1d22d36000 RDI: 0000000000000005
> [  362.541775] RBP: 00007f1d22d36000 R08: 00000002db777a30 R09: 0000562b70712b20
> [  362.549734] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000005
> [  362.557693] R13: 0000000000002800 R14: 00007ffebaef8060 R15: 0000562b70712260
> 
> The problem, AFAICS, is that the GRO path changes the cumulative
> truesize of the skbs entering such code path without updating
> sk_wmem_alloc. The posted code tries to keep unchanged such cumulative
> truesize.
> 

As I said, skbs entering GRO should not have skb->sk set.
GRO fully owns skbs.

No need to convince us.

For some reason, Toshiaki Makita added XDP and GRO, and broke veth



> I *think* we can hit a similar condition with a tun device in IFF_NAPI
> mode.

Why ?

tun_get_user() does not attach skb to a socket, that would be quite useless since skb
is entering input path and would be orphaned right away.


Fix would probably be :

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8d679c8b7f25c753d77cfb8821d9d2528c9c9048..96bd94480942b469403abf017f9f9d5be1e23ef5 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -602,9 +602,10 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
                        skb = veth_xdp_rcv_skb(rq, ptr, xdp_xmit);
                }
 
-               if (skb)
+               if (skb) {
+                       skb_orphan(skb);
                        napi_gro_receive(&rq->xdp_napi, skb);
-
+               }
                done++;
        }
 

^ permalink raw reply related

* [PATCH RFC] net: qca_spi: Introduce write register verification
From: Stefan Wahren @ 2018-09-11 15:26 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel, Stefan Wahren, Michael Heimpold

The SPI protocol for the QCA7000 doesn't have any fault detection.
In order to increase the drivers reliability in noisy environments,
we could implement a write verification inspired by the enc28j60.
This should avoid situations were the driver wrongly assumes the
receive interrupt is enabled and miss all incoming packets.

This function is disabled per default and can be controlled via module
parameter wr_verify.

Signed-off-by: Michael Heimpold <michael.heimpold@i2se.com>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/net/ethernet/qualcomm/qca_7k.c    | 34 +++++++++++++++++++++++++++++--
 drivers/net/ethernet/qualcomm/qca_7k.h    |  2 +-
 drivers/net/ethernet/qualcomm/qca_debug.c |  1 +
 drivers/net/ethernet/qualcomm/qca_spi.c   | 28 ++++++++++++++++++-------
 drivers/net/ethernet/qualcomm/qca_spi.h   |  1 +
 5 files changed, 56 insertions(+), 10 deletions(-)

Hi,
this patch requires "net: qca_spi: Fix race condition in spi transfers"
to be applied.

Stefan

diff --git a/drivers/net/ethernet/qualcomm/qca_7k.c b/drivers/net/ethernet/qualcomm/qca_7k.c
index 6c8543f..4292c89 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.c
+++ b/drivers/net/ethernet/qualcomm/qca_7k.c
@@ -81,8 +81,8 @@ qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result)
 	return ret;
 }
 
-int
-qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
+static int
+__qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
 {
 	__be16 tx_data[2];
 	struct spi_transfer transfer[2];
@@ -117,3 +117,33 @@ qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value)
 
 	return ret;
 }
+
+int
+qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value, int retry)
+{
+	int ret, i = 0;
+	u16 confirmed;
+
+	do {
+		ret = __qcaspi_write_register(qca, reg, value);
+		if (ret)
+			return ret;
+
+		if (!retry)
+			return 0;
+
+		ret = qcaspi_read_register(qca, reg, &confirmed);
+		if (ret)
+			return ret;
+
+		ret = confirmed != value;
+		if (!ret)
+			return 0;
+
+		i++;
+		qca->stats.write_verify_failed++;
+
+	} while (i <= retry);
+
+	return ret;
+}
diff --git a/drivers/net/ethernet/qualcomm/qca_7k.h b/drivers/net/ethernet/qualcomm/qca_7k.h
index 27124c2..356de8e 100644
--- a/drivers/net/ethernet/qualcomm/qca_7k.h
+++ b/drivers/net/ethernet/qualcomm/qca_7k.h
@@ -66,6 +66,6 @@
 
 void qcaspi_spi_error(struct qcaspi *qca);
 int qcaspi_read_register(struct qcaspi *qca, u16 reg, u16 *result);
-int qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value);
+int qcaspi_write_register(struct qcaspi *qca, u16 reg, u16 value, int retry);
 
 #endif /* _QCA_7K_H */
diff --git a/drivers/net/ethernet/qualcomm/qca_debug.c b/drivers/net/ethernet/qualcomm/qca_debug.c
index 51d89c8..a9f1bc0 100644
--- a/drivers/net/ethernet/qualcomm/qca_debug.c
+++ b/drivers/net/ethernet/qualcomm/qca_debug.c
@@ -60,6 +60,7 @@ static const char qcaspi_gstrings_stats[][ETH_GSTRING_LEN] = {
 	"Write buffer misses",
 	"Transmit ring full",
 	"SPI errors",
+	"Write verify errors",
 };
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 66b775d..d531050 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -69,6 +69,12 @@ static int qcaspi_pluggable = QCASPI_PLUGGABLE_MIN;
 module_param(qcaspi_pluggable, int, 0);
 MODULE_PARM_DESC(qcaspi_pluggable, "Pluggable SPI connection (yes/no).");
 
+#define QCASPI_WRITE_VERIFY_MIN 0
+#define QCASPI_WRITE_VERIFY_MAX 3
+static int wr_verify = QCASPI_WRITE_VERIFY_MIN;
+module_param(wr_verify, int, 0);
+MODULE_PARM_DESC(wr_verify, "SPI register write verify trails. Use 0-3.");
+
 #define QCASPI_TX_TIMEOUT (1 * HZ)
 #define QCASPI_QCA7K_REBOOT_TIME_MS 1000
 
@@ -77,7 +83,7 @@ start_spi_intr_handling(struct qcaspi *qca, u16 *intr_cause)
 {
 	*intr_cause = 0;
 
-	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0);
+	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
 	qcaspi_read_register(qca, SPI_REG_INTR_CAUSE, intr_cause);
 	netdev_dbg(qca->net_dev, "interrupts: 0x%04x\n", *intr_cause);
 }
@@ -90,8 +96,8 @@ end_spi_intr_handling(struct qcaspi *qca, u16 intr_cause)
 			   SPI_INT_RDBUF_ERR |
 			   SPI_INT_WRBUF_ERR);
 
-	qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause);
-	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable);
+	qcaspi_write_register(qca, SPI_REG_INTR_CAUSE, intr_cause, 0);
+	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, intr_enable, wr_verify);
 	netdev_dbg(qca->net_dev, "acking int: 0x%04x\n", intr_cause);
 }
 
@@ -239,7 +245,7 @@ qcaspi_tx_frame(struct qcaspi *qca, struct sk_buff *skb)
 
 	len = skb->len;
 
-	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, len);
+	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, len, wr_verify);
 	if (qca->legacy_mode)
 		qcaspi_tx_cmd(qca, QCA7K_SPI_WRITE | QCA7K_SPI_EXTERNAL);
 
@@ -345,6 +351,7 @@ qcaspi_receive(struct qcaspi *qca)
 
 	/* Read the packet size. */
 	qcaspi_read_register(qca, SPI_REG_RDBUF_BYTE_AVA, &available);
+
 	netdev_dbg(net_dev, "qcaspi_receive: SPI_REG_RDBUF_BYTE_AVA: Value: %08x\n",
 		   available);
 
@@ -353,7 +360,7 @@ qcaspi_receive(struct qcaspi *qca)
 		return -1;
 	}
 
-	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, available);
+	qcaspi_write_register(qca, SPI_REG_BFR_SIZE, available, wr_verify);
 
 	if (qca->legacy_mode)
 		qcaspi_tx_cmd(qca, QCA7K_SPI_READ | QCA7K_SPI_EXTERNAL);
@@ -524,7 +531,7 @@ qcaspi_qca7k_sync(struct qcaspi *qca, int event)
 		netdev_dbg(qca->net_dev, "sync: resetting device.\n");
 		qcaspi_read_register(qca, SPI_REG_SPI_CONFIG, &spi_config);
 		spi_config |= QCASPI_SLAVE_RESET_BIT;
-		qcaspi_write_register(qca, SPI_REG_SPI_CONFIG, spi_config);
+		qcaspi_write_register(qca, SPI_REG_SPI_CONFIG, spi_config, 0);
 
 		qca->sync = QCASPI_SYNC_RESET;
 		qca->stats.trig_reset++;
@@ -684,7 +691,7 @@ qcaspi_netdev_close(struct net_device *dev)
 
 	netif_stop_queue(dev);
 
-	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0);
+	qcaspi_write_register(qca, SPI_REG_INTR_ENABLE, 0, wr_verify);
 	free_irq(qca->spi_dev->irq, qca);
 
 	kthread_stop(qca->spi_thread);
@@ -904,6 +911,13 @@ qca_spi_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
+	if (wr_verify < QCASPI_WRITE_VERIFY_MIN ||
+	    wr_verify > QCASPI_WRITE_VERIFY_MAX) {
+		dev_err(&spi->dev, "Invalid write verify: %d\n",
+			wr_verify);
+		return -EINVAL;
+	}
+
 	dev_info(&spi->dev, "ver=%s, clkspeed=%d, burst_len=%d, pluggable=%d\n",
 		 QCASPI_DRV_VERSION,
 		 qcaspi_clkspeed,
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.h b/drivers/net/ethernet/qualcomm/qca_spi.h
index fc0e987..2d2c497 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.h
+++ b/drivers/net/ethernet/qualcomm/qca_spi.h
@@ -73,6 +73,7 @@ struct qcaspi_stats {
 	u64 write_buf_miss;
 	u64 ring_full;
 	u64 spi_err;
+	u64 write_verify_failed;
 };
 
 struct qcaspi {
-- 
2.7.4

^ permalink raw reply related

* Re: kernels > v4.12 oops/crash with ipsec-traffic: bisected to b838d5e1c5b6e57b10ec8af2268824041e3ea911: ipv4: mark DST_NOGC and remove the operation of dst_free()
From: Steffen Klassert @ 2018-09-11 10:33 UTC (permalink / raw)
  To: Kristian Evensen
  Cc: linux, Network Development, weiwan, Tobias Hommel, edumazet
In-Reply-To: <CAKfDRXg7EJeoO--QFFEJLtSPqRVngy4etSQHjSkQwNnhc4RFRA@mail.gmail.com>

On Mon, Sep 10, 2018 at 10:18:47AM +0200, Kristian Evensen wrote:
> Hi,
> 
> Thanks everyone for all the effort in debugging this issue.
> 
> On Mon, Sep 10, 2018 at 8:39 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > The easy fix that could be backported to stable would be
> > to check skb->dst for NULL and drop the packet in that case.
> 
> Thought I should just chime in and say that we deployed this
> work-around when we started observing the error back in June. Since
> then we have not seen any crashes. Also, we have instrumented some of
> our kernels to count the number of times the error is hit (overall +
> consecutive). Compared to the overall number of packets, the error
> happens very rarely. With our workloads, we on average see the error
> once every couple of days.

Thanks for letting us know!

I plan to fix this in the ipsec tree with:

Subject: [PATCH RFC] xfrm: Fix NULL pointer dereference when skb_dst_force clears
 the dst_entry.

Since commit 222d7dbd258d ("net: prevent dst uses after free")
skb_dst_force() might clear the dst_entry attached to the skb.
The xfrm code don't expect this to happen, so we crash with
a NULL pointer dereference in this case. Fix it by checking
skb_dst(skb) for NULL after skb_dst_force() and drop the packet
in cast the dst_entry was cleared.

Fixes: 222d7dbd258d ("net: prevent dst uses after free")
Reported-by: Tobias Hommel <netdev-list@genoetigt.de>
Reported-by: Kristian Evensen <kristian.evensen@gmail.com>
Reported-by: Wolfgang Walter <linux@stwm.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_output.c | 4 ++++
 net/xfrm/xfrm_policy.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 89b178a78dc7..36d15a38ce5e 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -101,6 +101,10 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 		spin_unlock_bh(&x->lock);
 
 		skb_dst_force(skb);
+		if (!skb_dst(skb)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+			goto error_nolock;
+		}
 
 		if (xfrm_offload(skb)) {
 			x->type_offload->encap(x, skb);
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 7c5e8978aeaa..626e0f4d1749 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2548,6 +2548,10 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 	}
 
 	skb_dst_force(skb);
+	if (!skb_dst(skb)) {
+		XFRM_INC_STATS(net, LINUX_MIB_XFRMFWDHDRERROR);
+		return 0;
+	}
 
 	dst = xfrm_lookup(net, skb_dst(skb), &fl, NULL, XFRM_LOOKUP_QUEUE);
 	if (IS_ERR(dst)) {
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCHv2 iproute2] bridge/mdb: fix missing new line when show bridge mdb
From: Phil Sutter @ 2018-09-11 11:01 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, David Ahern
In-Reply-To: <1536629195-12540-1-git-send-email-liuhangbin@gmail.com>

Hi Hangbin,

On Tue, Sep 11, 2018 at 09:26:35AM +0800, Hangbin Liu wrote:
[...]
> +	if (!is_json_context() && !show_stats)
> +		print_string(PRINT_FP, NULL, "\n", NULL);

There is no need to check for !is_json_context() here. You give a type
of PRINT_FP which won't lead to output if JSON is active. For reference,
check print_color_string() in lib/json_print.c and _IS_JSON_CONTEXT
macro.

Cheers, Phil

^ permalink raw reply

* Re: unexpected GRO/veth behavior
From: Toshiaki Makita @ 2018-09-11 11:07 UTC (permalink / raw)
  To: Eric Dumazet, Paolo Abeni, netdev
In-Reply-To: <ecb079cc-033d-0726-75ce-00eada2ae765@gmail.com>

On 2018/09/11 19:27, Eric Dumazet wrote:
...
> Fix would probably be :
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 8d679c8b7f25c753d77cfb8821d9d2528c9c9048..96bd94480942b469403abf017f9f9d5be1e23ef5 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -602,9 +602,10 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
>                         skb = veth_xdp_rcv_skb(rq, ptr, xdp_xmit);
>                 }
>  
> -               if (skb)
> +               if (skb) {
> +                       skb_orphan(skb);
>                         napi_gro_receive(&rq->xdp_napi, skb);
> -
> +               }
>                 done++;
>         }

Considering commit 9c4c3252 ("skbuff: preserve sock reference when
scrubbing the skb.") I'm not sure if we should unconditionally orphan
the skb here.
I was thinking I should call netif_receive_skb() for such packets
instead of napi_gro_receive().

-- 
Toshiaki Makita

^ permalink raw reply

* tools/bpf regression causing samples/bpf/ to hang
From: Björn Töpel @ 2018-09-11 11:11 UTC (permalink / raw)
  To: Netdev, yhs; +Cc: ast, Daniel Borkmann, Jesper Dangaard Brouer

Hi Yonghong, I tried to run the XDP samples from the bpf-next tip
today, and was hit by a regression.

Commit f7010770fbac ("tools/bpf: move bpf/lib netlink related
functions into a new file") adds a while(1) around the recv call in
bpf_set_link_xdp_fd making that call getting stuck in an infinite
loop.

I simply removed the loop, and that solved my problem (patch below).

However, I don't know if removing the loop would break bpftool for
you. If not, I can submit the patch as a proper one for bpf-next.

Thanks!
Björn

From: =?UTF-8?q?Bj=C3=B6rn=20T=C3=B6pel?= <bjorn.topel@intel.com>
Date: Tue, 11 Sep 2018 12:35:44 +0200
Subject: [PATCH] tools/bpf: remove loop around netlink recv

Commit f7010770fbac ("tools/bpf: move bpf/lib netlink related
functions into a new file") moved the bpf_set_link_xdp_fd and split it
up into multiple functions. The added receive function
bpf_netlink_recv added a loop around the recv syscall leading to
multiple recv calls. This resulted in all XDP samples in the
samples/bpf/ to stop working, since they were stuck in a blocking
recv.

This commits removes the while (1)-statement.

Fixes: f7010770fbac ("tools/bpf: move bpf/lib netlink related
functions into a new file")
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
 tools/lib/bpf/netlink.c | 64 ++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 469e068dd0c5..0eae1fbf46c6 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -70,41 +70,39 @@ static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
     char buf[4096];
     int len, ret;

-    while (1) {
-        len = recv(sock, buf, sizeof(buf), 0);
-        if (len < 0) {
-            ret = -errno;
+    len = recv(sock, buf, sizeof(buf), 0);
+    if (len < 0) {
+        ret = -errno;
+        goto done;
+    }
+
+    for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+         nh = NLMSG_NEXT(nh, len)) {
+        if (nh->nlmsg_pid != nl_pid) {
+            ret = -LIBBPF_ERRNO__WRNGPID;
             goto done;
         }
-
-        for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
-             nh = NLMSG_NEXT(nh, len)) {
-            if (nh->nlmsg_pid != nl_pid) {
-                ret = -LIBBPF_ERRNO__WRNGPID;
-                goto done;
-            }
-            if (nh->nlmsg_seq != seq) {
-                ret = -LIBBPF_ERRNO__INVSEQ;
-                goto done;
-            }
-            switch (nh->nlmsg_type) {
-            case NLMSG_ERROR:
-                err = (struct nlmsgerr *)NLMSG_DATA(nh);
-                if (!err->error)
-                    continue;
-                ret = err->error;
-                nla_dump_errormsg(nh);
-                goto done;
-            case NLMSG_DONE:
-                return 0;
-            default:
-                break;
-            }
-            if (_fn) {
-                ret = _fn(nh, fn, cookie);
-                if (ret)
-                    return ret;
-            }
+        if (nh->nlmsg_seq != seq) {
+            ret = -LIBBPF_ERRNO__INVSEQ;
+            goto done;
+        }
+        switch (nh->nlmsg_type) {
+        case NLMSG_ERROR:
+            err = (struct nlmsgerr *)NLMSG_DATA(nh);
+            if (!err->error)
+                continue;
+            ret = err->error;
+            nla_dump_errormsg(nh);
+            goto done;
+        case NLMSG_DONE:
+            return 0;
+        default:
+            break;
+        }
+        if (_fn) {
+            ret = _fn(nh, fn, cookie);
+            if (ret)
+                return ret;
         }
     }
     ret = 0;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v3 00/15] soc: octeontx2: Add RVU admin function driver
From: Sunil Kovvuri @ 2018-09-11 16:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: LKML, olof, LAKML, linux-soc, Andrew Lunn, David S. Miller,
	Linux Netdev List, Sunil Goutham
In-Reply-To: <CAK8P3a32gpNX42NuzmbW1PbsHpq7v7LoQ6WDeKtt=QiYbuyM7w@mail.gmail.com>

On Tue, Sep 11, 2018 at 7:07 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Sep 11, 2018 at 2:37 PM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> >
> > Didn't receive any feedback for the v3 patch series over a week's time.
> > Can you please pick up these patches to merge into arm-soc ?
>
> I would still prefer to see the whole thing as part of drivers/net/
> instead of drivers/soc,
> and reviewed in full on the netdev side, including the parts that are
> not ethernet specific.
>
>        Arnd

Hmm.. I agree that there are many networking terms used in the driver
but it's not a
networking driver, it's just a HW configuration driver which includes
how HW should
parse the packet. This driver doesn't fit into drivers/net.

Let's say if netdev driver in drivers/net/ethernet doesn't make use of
crypto feature
then i guess netdev maintainers would reject any patches which configure crypto
block. Also as i have been saying there are other scenarios as well.
Future silicons may add support for other features into this resource
virtualization unit's domain.
An example would be compression. Any patches which do compression
related HW configuration
might be rejected by netdev maintainers, cause they are no way related
to networking.

I will keep netdev mailing list in all the patch submissions but
moving this driver into drivers/net
doesn't sound right, from it's functionality perspective.

Thanks,
Sunil.

^ permalink raw reply

* Re: [PATCH net-next 0/8] bnxt_en: devlink param updates
From: Jakub Kicinski @ 2018-09-11 11:33 UTC (permalink / raw)
  To: Vasundhara Volam; +Cc: davem, michael.chan, netdev, Alexander Duyck
In-Reply-To: <1536655505-14387-1-git-send-email-vasundhara-v.volam@broadcom.com>

On Tue, 11 Sep 2018 14:14:57 +0530, Vasundhara Volam wrote:
> This patchset adds support for 4 generic and 1 driver-specific devlink
> parameters.
> 
> Also, this patchset adds support to return proper error code if
> HWRM_NVM_GET/SET_VARIABLE commands return error code
> HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.
> 
> Vasundhara Volam (8):
>   devlink: Add generic parameter hw_tc_offload

Much like Jiri, I can't help but wonder why do you need this?

>   devlink: Add generic parameter ignore_ari
>   devlink: Add generic parameter msix_vec_per_pf_max
>   devlink: Add generic parameter msix_vec_per_pf_min

IMHO more structured API would be preferable if possible.  The string
keys won't scale if you want to set the parameters per PF, and
creating more structured API for PCIe which is a relatively slow
moving HW spec seems tractable.

Not to mention the question Alex posed before about where this knobs
should actually live.  I'm personally fine with devlink.

>   bnxt_en: Use hw_tc_offload and ignore_ari devlink parameters
>   bnxt_en: return proper error when FW returns
>     HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
>   bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink
>     params.
>   bnxt_en: Add a driver specific devlink parameter.

The details about the device specific devlink parameter are very much
lacking.  Why does the patch subject not mention any specifics?  What's
GRE in the first place?

You really need to provide more details and docs for all these
parameters.

^ permalink raw reply

* [PATCH net-next] qlcnic: Remove set but not used variables 'fw_mbx' and 'hdr_size'
From: YueHaibing @ 2018-09-11 11:51 UTC (permalink / raw)
  To: Harish Patil, Manish Chopra, David S. Miller
  Cc: Yue Haibing, Dept-GELinuxNICDev, netdev, kernel-janitors

From: Yue Haibing <yuehaibing@huawei.com>

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

drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c: In function 'qlcnic_sriov_pull_bc_msg':
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c:907:6: warning:
 variable 'fw_mbx' set but not used [-Wunused-but-set-variable]

drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c: In function 'qlcnic_sriov_issue_bc_post':
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c:939:16: warning:
 variable 'hdr_size' set but not used [-Wunused-but-set-variable]

Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c
index 77e386e..f7c2f32 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c
@@ -904,13 +904,11 @@ static void qlcnic_sriov_pull_bc_msg(struct qlcnic_adapter *adapter,
 				     u32 *hdr, u32 *pay, u32 size)
 {
 	struct qlcnic_hardware_context *ahw = adapter->ahw;
-	u32 fw_mbx;
 	u8 i, max = 2, hdr_size, j;
 
 	hdr_size = (sizeof(struct qlcnic_bc_hdr) / sizeof(u32));
 	max = (size / sizeof(u32)) + hdr_size;
 
-	fw_mbx = readl(QLCNIC_MBX_FW(ahw, 0));
 	for (i = 2, j = 0; j < hdr_size; i++, j++)
 		*(hdr++) = readl(QLCNIC_MBX_FW(ahw, i));
 	for (; j < max; i++, j++)
@@ -936,7 +934,7 @@ static int __qlcnic_sriov_issue_bc_post(struct qlcnic_vf_info *vf)
 static int qlcnic_sriov_issue_bc_post(struct qlcnic_bc_trans *trans, u8 type)
 {
 	struct qlcnic_vf_info *vf = trans->vf;
-	u32 pay_size, hdr_size;
+	u32 pay_size;
 	u32 *hdr, *pay;
 	int ret;
 	u8 pci_func = trans->func_id;
@@ -947,14 +945,12 @@ static int qlcnic_sriov_issue_bc_post(struct qlcnic_bc_trans *trans, u8 type)
 	if (type == QLC_BC_COMMAND) {
 		hdr = (u32 *)(trans->req_hdr + trans->curr_req_frag);
 		pay = (u32 *)(trans->req_pay + trans->curr_req_frag);
-		hdr_size = (sizeof(struct qlcnic_bc_hdr) / sizeof(u32));
 		pay_size = qlcnic_sriov_get_bc_paysize(trans->req_pay_size,
 						       trans->curr_req_frag);
 		pay_size = (pay_size / sizeof(u32));
 	} else {
 		hdr = (u32 *)(trans->rsp_hdr + trans->curr_rsp_frag);
 		pay = (u32 *)(trans->rsp_pay + trans->curr_rsp_frag);
-		hdr_size = (sizeof(struct qlcnic_bc_hdr) / sizeof(u32));
 		pay_size = qlcnic_sriov_get_bc_paysize(trans->rsp_pay_size,
 						       trans->curr_rsp_frag);
 		pay_size = (pay_size / sizeof(u32));

^ permalink raw reply related

* Re: unexpected GRO/veth behavior
From: Paolo Abeni @ 2018-09-11 11:44 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: Toshiaki Makita
In-Reply-To: <ecb079cc-033d-0726-75ce-00eada2ae765@gmail.com>

On Tue, 2018-09-11 at 03:27 -0700, Eric Dumazet wrote:
> On 09/10/2018 11:54 PM, Paolo Abeni wrote:
> > I *think* we can hit a similar condition with a tun device in IFF_NAPI
> > mode.
> 
> Why ?
> 
> tun_get_user() does not attach skb to a socket, that would be quite useless since skb
> 
	
I think we can hit this code path:

tun_get_user() -> tun_alloc_skb() -> sock_alloc_send_pskb() ->
skb_set_owner_w() 

Than later napi and GRO.

Cheers,

Paolo

^ permalink raw reply

* Re: [Intel-wired-lan] e1000e driver stuck at 10Mbps after reconnection
From: Camille Bordignon @ 2018-09-11 11:46 UTC (permalink / raw)
  To: Benjamin Poirier
  Cc: Neftin, Sasha, Alexander Duyck, Netdev, intel-wired-lan,
	David S. Miller, linux-kernel
In-Reply-To: <20180911083147.GA31642@f2>

Le mardi 11 sept. 2018 à 17:31:47 (+0900), Benjamin Poirier a écrit :
> On 2018/09/07 08:28, Camille Bordignon wrote:
> > Le mercredi 08 août 2018 à 18:00:28 (+0300), Neftin, Sasha a écrit :
> > > On 8/8/2018 17:24, Neftin, Sasha wrote:
> > > > On 8/7/2018 09:42, Camille Bordignon wrote:
> > > > > Le lundi 06 août 2018 à 15:45:29 (-0700), Alexander Duyck a écrit :
> > > > > > On Mon, Aug 6, 2018 at 4:59 AM, Camille Bordignon
> > > > > > <camille.bordignon@easymile.com> wrote:
> > > > > > > Hello,
> > > > > > > 
> > > > > > > Recently we experienced some issues with intel NIC (I219-LM
> > > > > > > and I219-V).
> > > > > > > It seems that after a wire reconnection, auto-negotation "fails" and
> > > > > > > link speed drips to 10 Mbps.
> > > > > > > 
> [...]
> > 
> > I recently figured out that neither the previous patch nor commit
> > 0b76aae741abb9d16d2c0e67f8b1e766576f897d fix this issue.
> > 
> > In these cases, after reproducing the issue, when ethernet wire is connected
> > kernel logs mention full speed (1000 Mbps) but actually it seems it is not.
> > The problem persists.
> > 
> 
> Hmm, so the newer (post 4110e02eb45e) kernels are actually "better", in
> that they accurately report that link speed is 10Mb/s.
> 
Exactly.

> In the end, do you know of a kernel version that doesn't exhibit the
> problem of slower actual link speed?
> 
No. I've tried older version but at some point API has changed and
it fails to compile.
> I had a look at the code and I tried to reproduce the problem on the
> hardware that I have (I217) but could not.
> 
> Also, out of curiosity, have you tried playing with the speed, autoneg
> and advertise settings via ethtool -s to force the link to 1000Mb/s?
Forcing the speed fixes the issue in any case (I mean even when speed is
wronly reported to 1000Mb/s).

Done this way:

#ethtool -s enp0s31f6 speed 1000 duplex full autoneg off

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/8] bnxt_en: devlink param updates
From: Jiri Pirko @ 2018-09-11 11:51 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Vasundhara Volam, davem, michael.chan, netdev, Alexander Duyck
In-Reply-To: <20180911133215.5798ed2a@cakuba>

Tue, Sep 11, 2018 at 01:33:51PM CEST, jakub.kicinski@netronome.com wrote:
>On Tue, 11 Sep 2018 14:14:57 +0530, Vasundhara Volam wrote:
>> This patchset adds support for 4 generic and 1 driver-specific devlink
>> parameters.
>> 
>> Also, this patchset adds support to return proper error code if
>> HWRM_NVM_GET/SET_VARIABLE commands return error code
>> HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.
>> 
>> Vasundhara Volam (8):
>>   devlink: Add generic parameter hw_tc_offload
>
>Much like Jiri, I can't help but wonder why do you need this?
>
>>   devlink: Add generic parameter ignore_ari
>>   devlink: Add generic parameter msix_vec_per_pf_max
>>   devlink: Add generic parameter msix_vec_per_pf_min
>
>IMHO more structured API would be preferable if possible.  The string
>keys won't scale if you want to set the parameters per PF, and
>creating more structured API for PCIe which is a relatively slow
>moving HW spec seems tractable.
>
>Not to mention the question Alex posed before about where this knobs
>should actually live.  I'm personally fine with devlink.
>
>>   bnxt_en: Use hw_tc_offload and ignore_ari devlink parameters
>>   bnxt_en: return proper error when FW returns
>>     HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
>>   bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink
>>     params.
>>   bnxt_en: Add a driver specific devlink parameter.
>
>The details about the device specific devlink parameter are very much
>lacking.  Why does the patch subject not mention any specifics?  What's
>GRE in the first place?
>
>You really need to provide more details and docs for all these
>parameters.

We really need Documentation/networking/devlink-params.txt to describe
these.

^ permalink raw reply

* [PATCH] qtnfmac: remove set but not used variable 'vif'
From: YueHaibing @ 2018-09-11 12:28 UTC (permalink / raw)
  To: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich, Kalle Valo,
	Sergei Maksimenko, Vasily Ulyanov, Andrey Shevchenko
  Cc: YueHaibing, linux-wireless, netdev, kernel-janitors

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

drivers/net/wireless/quantenna/qtnfmac/cfg80211.c: In function 'qtnf_dump_survey':
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c:694:19: warning:
 variable 'vif' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 4aa332f..452d4b7 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -691,11 +691,8 @@ static int qtnf_set_default_key(struct wiphy *wiphy, struct net_device *dev,
 	const struct cfg80211_chan_def *chandef = &wdev->chandef;
 	struct ieee80211_channel *chan;
 	struct qtnf_chan_stats stats;
-	struct qtnf_vif *vif;
 	int ret;
 
-	vif = qtnf_netdev_get_priv(dev);
-
 	sband = wiphy->bands[NL80211_BAND_2GHZ];
 	if (sband && idx >= sband->n_channels) {
 		idx -= sband->n_channels;

^ permalink raw reply related

* [PATCH] wil6210: remove set but not used variable 'start'
From: YueHaibing @ 2018-09-11 12:32 UTC (permalink / raw)
  To: Maya Erez, Kalle Valo
  Cc: YueHaibing, linux-wireless, wil6210, netdev, kernel-janitors

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

drivers/net/wireless/ath/wil6210/pm.c: In function 'wil_suspend_keep_radio_on':
drivers/net/wireless/ath/wil6210/pm.c:193:16: warning:
 variable 'start' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/wireless/ath/wil6210/pm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wil6210/pm.c b/drivers/net/wireless/ath/wil6210/pm.c
index 3a41947..75fe932 100644
--- a/drivers/net/wireless/ath/wil6210/pm.c
+++ b/drivers/net/wireless/ath/wil6210/pm.c
@@ -190,7 +190,7 @@ static int wil_resume_keep_radio_on(struct wil6210_priv *wil)
 static int wil_suspend_keep_radio_on(struct wil6210_priv *wil)
 {
 	int rc = 0;
-	unsigned long start, data_comp_to;
+	unsigned long data_comp_to;
 
 	wil_dbg_pm(wil, "suspend keep radio on\n");
 
@@ -232,7 +232,6 @@ static int wil_suspend_keep_radio_on(struct wil6210_priv *wil)
 	}
 
 	/* Wait for completion of the pending RX packets */
-	start = jiffies;
 	data_comp_to = jiffies + msecs_to_jiffies(WIL_DATA_COMPLETION_TO_MS);
 	if (test_bit(wil_status_napi_en, wil->status)) {
 		while (!wil->txrx_ops.is_rx_idle(wil)) {

^ permalink raw reply related

* [PATCH][net-dsa-next] net: dsa: b53: ensure variable pause is initialized
From: Colin King @ 2018-09-11 17:53 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S . Miller,
	netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Currenly integer pause is not initialized and may be left an
uninitialized state if flow control on BCM5301x's CPU port
is not enabled.  This will potentially feed garbage into
the TX or RX pause bits that are or'd into pause. Fix this
by ensuring pause is initialized to zero.

Detected by static analysis with smatch:
"error: uninitialized symbol 'pause'."

Fixes: 5e004460f874 ("net: dsa: b53: Add helper to set link parameters")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ea4256cd628b..dbf5b86a07fe 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1025,7 +1025,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
 	struct b53_device *dev = ds->priv;
 	struct ethtool_eee *p = &dev->ports[port].eee;
 	u8 rgmii_ctrl = 0, reg = 0, off;
-	int pause;
+	int pause = 0;
 
 	if (!phy_is_pseudo_fixed_link(phydev))
 		return;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] qtnfmac: remove set but not used variable 'vif'
From: Sergey Matyukevich @ 2018-09-11 12:58 UTC (permalink / raw)
  To: YueHaibing
  Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich, Kalle Valo,
	Sergei Maksimenko, Vasily Ulyanov, Andrey Shevchenko,
	linux-wireless, netdev, kernel-janitors
In-Reply-To: <1536668904-13110-1-git-send-email-yuehaibing@huawei.com>

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/wireless/quantenna/qtnfmac/cfg80211.c: In function 'qtnf_dump_survey':
> drivers/net/wireless/quantenna/qtnfmac/cfg80211.c:694:19: warning:
>  variable 'vif' set but not used [-Wunused-but-set-variable]
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Thanks !

Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

Regards,
Sergey

^ permalink raw reply

* [PATCH v2] neighbour: confirm neigh entries when ARP packet is received
From: Vasily Khoruzhick @ 2018-09-11 18:04 UTC (permalink / raw)
  To: David S. Miller, Roopa Prabhu, Alexey Dobriyan, Eric Dumazet,
	Stephen Hemminger, Jim Westfall, Wolfgang Bumiller,
	Vasily Khoruzhick, Kees Cook, Ihar Hrachyshka, netdev,
	linux-kernel
  Cc: Vasily Khoruzhick

Update 'confirmed' timestamp when ARP packet is received. It shouldn't
affect locktime logic and anyway entry can be confirmed by any higher-layer
protocol. Thus it makes to sense not to confirm it when ARP packet is
received.

Fixes: 77d7123342 ("neighbour: update neigh timestamps iff update is
effective")

Signed-off-by: Vasily Khoruzhick <vasilykh@arista.com>
---
v2: - update comment to match new code.

 net/core/neighbour.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index aa19d86937af..56a554597db5 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1180,6 +1180,12 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		lladdr = neigh->ha;
 	}
 
+	/* Update confirmed timestamp for neighbour entry after we
+	 * received ARP packet even if it doesn't change IP to MAC binding.
+	 */
+	if (new & NUD_CONNECTED)
+		neigh->confirmed = jiffies;
+
 	/* If entry was valid and address is not changed,
 	   do not change entry state, if new one is STALE.
 	 */
@@ -1201,15 +1207,12 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		}
 	}
 
-	/* Update timestamps only once we know we will make a change to the
+	/* Update timestamp only once we know we will make a change to the
 	 * neighbour entry. Otherwise we risk to move the locktime window with
 	 * noop updates and ignore relevant ARP updates.
 	 */
-	if (new != old || lladdr != neigh->ha) {
-		if (new & NUD_CONNECTED)
-			neigh->confirmed = jiffies;
+	if (new != old || lladdr != neigh->ha)
 		neigh->updated = jiffies;
-	}
 
 	if (new != old) {
 		neigh_del_timer(neigh);
-- 
2.18.0

^ permalink raw reply related

* Re: [PATCH v2 net-next 10/12] net: ethernet: Add helper for set_pauseparam for Asym Pause
From: kbuild test robot @ 2018-09-11 13:04 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: kbuild-all, David Miller, netdev, Florian Fainelli, Andrew Lunn
In-Reply-To: <1536616350-15442-11-git-send-email-andrew@lunn.ch>

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

Hi Andrew,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Andrew-Lunn/Preparing-for-phylib-limkmodes/20180911-204149
config: x86_64-randconfig-x011-201836 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/net/phy/phy_device.c: In function 'phy_set_asym_pause':
>> drivers/net/phy/phy_device.c:1838:11: warning: 'return' with a value, in function returning void
       return phy_start_aneg(phydev);
              ^~~~~~~~~~~~~~~~~~~~~~
   drivers/net/phy/phy_device.c:1824:6: note: declared here
    void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
         ^~~~~~~~~~~~~~~~~~

vim +/return +1838 drivers/net/phy/phy_device.c

  1812	
  1813	/**
  1814	 * phy_set_asym_pause - Configure Pause and Asym Pause
  1815	 * @phydev: target phy_device struct
  1816	 * @rx: Receiver Pause is supported
  1817	 * @tx: Transmit Pause is supported
  1818	 *
  1819	 * Description: Configure advertised Pause support depending on if
  1820	 * transmit and receiver pause is supported. If there has been a
  1821	 * change in adverting, trigger a new autoneg. Generally called from
  1822	 * the set_pauseparam .ndo.
  1823	 */
  1824	void phy_set_asym_pause(struct phy_device *phydev, bool rx, bool tx)
  1825	{
  1826		u16 oldadv = phydev->advertising;
  1827		u16 newadv = oldadv &= ~(SUPPORTED_Pause | SUPPORTED_Asym_Pause);
  1828	
  1829		if (rx)
  1830			newadv |= SUPPORTED_Pause | SUPPORTED_Asym_Pause;
  1831		if (tx)
  1832			newadv ^= SUPPORTED_Asym_Pause;
  1833	
  1834		if (oldadv != newadv) {
  1835			phydev->advertising = newadv;
  1836	
  1837			if (phydev->autoneg)
> 1838				return phy_start_aneg(phydev);
  1839		}
  1840	}
  1841	EXPORT_SYMBOL(phy_set_asym_pause);
  1842	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33600 bytes --]

^ permalink raw reply

* Re: [PATCH v3 net-next 5/6] dt-bindings: net: dsa: Add lantiq,xrx200-gswip DT bindings
From: Rob Herring @ 2018-09-11 13:06 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Hauke Mehrtens, David Miller, netdev, Vivien Didelot,
	Florian Fainelli, John Crispin, Linux-MIPS, dev, hauke.mehrtens,
	devicetree
In-Reply-To: <20180910220516.GA16024@lunn.ch>

On Mon, Sep 10, 2018 at 5:05 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > +See Documentation/devicetree/bindings/net/dsa/dsa.txt for a list of
> > > +additional required and optional properties.
> > > +
> > > +
>
> snip
>
> > > +   #address-cells = <1>;
> > > +   #size-cells = <0>;
> > > +   compatible = "lantiq,xrx200-gswip";
> > > +   reg = < 0xE108000 0x3000 /* switch */
> > > +           0xE10B100 0x70 /* mdio */
> > > +           0xE10B1D8 0x30 /* mii */
> > > +           >;
> > > +   dsa,member = <0 0>;
> >
> > Not documented.
>
> Hi Rob
>
> It is documented in Documentation/devicetree/bindings/net/dsa/dsa.txt
> referenced above.

Hum, okay. Not something I recall reviewing. Not that I tend to
remember, but 'dsa' is not a vendor and the property should be
'dsa-member'.

Rob

^ permalink raw reply

* Re: [PATCH v2] neighbour: confirm neigh entries when ARP packet is received
From: Stephen Hemminger @ 2018-09-11 18:12 UTC (permalink / raw)
  To: Vasily Khoruzhick
  Cc: David S. Miller, Roopa Prabhu, Alexey Dobriyan, Eric Dumazet,
	Jim Westfall, Wolfgang Bumiller, Vasily Khoruzhick, Kees Cook,
	Ihar Hrachyshka, netdev, linux-kernel
In-Reply-To: <20180911180406.31283-1-vasilykh@arista.com>

On Tue, 11 Sep 2018 11:04:06 -0700
Vasily Khoruzhick <vasilykh@arista.com> wrote:

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index aa19d86937af..56a554597db5 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1180,6 +1180,12 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
>  		lladdr = neigh->ha;
>  	}
>  
> +	/* Update confirmed timestamp for neighbour entry after we
> +	 * received ARP packet even if it doesn't change IP to MAC binding.
> +	 */
> +	if (new & NUD_CONNECTED)
> +		neigh->confirmed = jiffies;

You might want to do:
	if ((new & NUD_CONNECTED) && neigh->confirmed != jiffies)
		neigh->confirmed = jiffies;

This avoid poisoning the cacheline with unnecessary write.

^ permalink raw reply

* Re: [PATCH net-next v3 17/17] net: WireGuard secure network tunnel
From: kbuild test robot @ 2018-09-11 13:17 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: kbuild-all, linux-kernel, netdev, davem, gregkh,
	Jason A. Donenfeld
In-Reply-To: <20180911010838.8818-19-Jason@zx2c4.com>

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

Hi Jason,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jason-A-Donenfeld/WireGuard-Secure-Network-Tunnel/20180911-185037
config: arm-at91_dt_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arch/arm/vfp/vfphw.S: Assembler messages:
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c0' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c1' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c2' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c3' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c4' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c5' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c6' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c7' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c8' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c9' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c10' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c11' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c12' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c13' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c14' in ARM mode
>> arch/arm/vfp/vfphw.S:299: Error: selected processor does not support `mrrc p11,3,r0,r1,c15' in ARM mode
>> arch/arm/vfp/vfphw.S:321: Error: selected processor does not support `mcrr p11,3,r0,r1,c0' in ARM mode
>> arch/arm/vfp/vfphw.S:321: Error: selected processor does not support `mcrr p11,3,r0,r1,c1' in ARM mode
>> arch/arm/vfp/vfphw.S:321: Error: selected processor does not support `mcrr p11,3,r0,r1,c2' in ARM mode
>> arch/arm/vfp/vfphw.S:321: Error: selected processor does not support `mcrr p11,3,r0,r1,c3' in ARM mode

vim +299 arch/arm/vfp/vfphw.S

^1da177e Linus Torvalds  2005-04-16  285  
93ed3970 Catalin Marinas 2008-08-28  286  ENTRY(vfp_get_double)
07f33a03 Catalin Marinas 2009-07-24  287  	tbl_branch r0, r3, #3
^1da177e Linus Torvalds  2005-04-16  288  	.irp	dr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
07f33a03 Catalin Marinas 2009-07-24  289  1:	fmrrd	r0, r1, d\dr
6ebbf2ce Russell King    2014-06-30  290  	ret	lr
07f33a03 Catalin Marinas 2009-07-24  291  	.org	1b + 8
^1da177e Linus Torvalds  2005-04-16  292  	.endr
25ebee02 Catalin Marinas 2007-09-25  293  #ifdef CONFIG_VFPv3
25ebee02 Catalin Marinas 2007-09-25  294  	@ d16 - d31 registers
25ebee02 Catalin Marinas 2007-09-25  295  	.irp	dr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
07f33a03 Catalin Marinas 2009-07-24  296  1:	mrrc	p11, 3, r0, r1, c\dr	@ fmrrd	r0, r1, d\dr
6ebbf2ce Russell King    2014-06-30  297  	ret	lr
07f33a03 Catalin Marinas 2009-07-24  298  	.org	1b + 8
25ebee02 Catalin Marinas 2007-09-25 @299  	.endr
25ebee02 Catalin Marinas 2007-09-25  300  #endif
^1da177e Linus Torvalds  2005-04-16  301  
25ebee02 Catalin Marinas 2007-09-25  302  	@ virtual register 16 (or 32 if VFPv3) for compare with zero
^1da177e Linus Torvalds  2005-04-16  303  	mov	r0, #0
^1da177e Linus Torvalds  2005-04-16  304  	mov	r1, #0
6ebbf2ce Russell King    2014-06-30  305  	ret	lr
93ed3970 Catalin Marinas 2008-08-28  306  ENDPROC(vfp_get_double)
^1da177e Linus Torvalds  2005-04-16  307  
93ed3970 Catalin Marinas 2008-08-28  308  ENTRY(vfp_put_double)
07f33a03 Catalin Marinas 2009-07-24  309  	tbl_branch r2, r3, #3
^1da177e Linus Torvalds  2005-04-16  310  	.irp	dr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
07f33a03 Catalin Marinas 2009-07-24  311  1:	fmdrr	d\dr, r0, r1
6ebbf2ce Russell King    2014-06-30  312  	ret	lr
07f33a03 Catalin Marinas 2009-07-24  313  	.org	1b + 8
^1da177e Linus Torvalds  2005-04-16  314  	.endr
25ebee02 Catalin Marinas 2007-09-25  315  #ifdef CONFIG_VFPv3
25ebee02 Catalin Marinas 2007-09-25  316  	@ d16 - d31 registers
25ebee02 Catalin Marinas 2007-09-25  317  	.irp	dr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15
138de1c4 Russell King    2010-05-27  318  1:	mcrr	p11, 3, r0, r1, c\dr	@ fmdrr	r0, r1, d\dr
6ebbf2ce Russell King    2014-06-30  319  	ret	lr
07f33a03 Catalin Marinas 2009-07-24  320  	.org	1b + 8
25ebee02 Catalin Marinas 2007-09-25 @321  	.endr

:::::: The code at line 299 was first introduced by commit
:::::: 25ebee020bd34d1f4c5678538204f0b10bf9f6d5 [ARM] 4583/1: ARMv7: Add VFPv3 support

:::::: TO: Catalin Marinas <catalin.marinas@arm.com>
:::::: CC: Russell King <rmk+kernel@arm.linux.org.uk>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23776 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/3][can-next] can: rcar_can: Fix erroneous registration
From: Simon Horman @ 2018-09-11 13:31 UTC (permalink / raw)
  To: Fabrizio Castro
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Sergei Shtylyov, Chris Paterson, linux-can, netdev,
	Geert Uytterhoeven, Biju Das, linux-renesas-soc
In-Reply-To: <1536576195-11520-2-git-send-email-fabrizio.castro@bp.renesas.com>

On Mon, Sep 10, 2018 at 11:43:13AM +0100, Fabrizio Castro wrote:
> Assigning 2 to "renesas,can-clock-select" tricks the driver into
> registering the CAN interface, even though we don't want that.
> This patch improves one of the checks to prevent that from happening.
> 
> Fixes: 862e2b6af9413b43 ("can: rcar_can: support all input clocks")
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

^ permalink raw reply

* Re: [PATCH v2 2/3] dt-bindings: can: rcar_can: Add r8a774a1 support
From: Simon Horman @ 2018-09-11 13:31 UTC (permalink / raw)
  To: Fabrizio Castro
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland,
	David S. Miller, Sergei Shtylyov, linux-can, linux-kernel, netdev,
	devicetree, Geert Uytterhoeven, Chris Paterson, Biju Das,
	linux-renesas-soc
In-Reply-To: <1536576195-11520-3-git-send-email-fabrizio.castro@bp.renesas.com>

On Mon, Sep 10, 2018 at 11:43:14AM +0100, Fabrizio Castro wrote:
> Document RZ/G2M (r8a774a1) SoC specific bindings.
> 
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Chris Paterson <Chris.Paterson2@renesas.com>
> Reviewed-by: Biju Das <biju.das@bp.renesas.com>
> ---
> v1->v2:
> * dropped "renesas,rzg-gen2-can" and fixed "clocks" property description
>   as per Geert's comments.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

^ 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