From: David Miller <davem@davemloft.net>
To: Jason@zx2c4.com
Cc: linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, gregkh@linuxfoundation.org
Subject: Re: [PATCH net-next v9 19/19] net: WireGuard secure network tunnel
Date: Sun, 24 Mar 2019 20:02:54 -0400 (EDT) [thread overview]
Message-ID: <20190324.200254.1946143057733371048.davem@davemloft.net> (raw)
In-Reply-To: <20190322071122.6677-20-Jason@zx2c4.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Fri, 22 Mar 2019 01:11:22 -0600
> +static __always_inline void swap_endian(u8 *dst, const u8 *src, u8 bits)
> +{
Unless you have an absolutely requirement on inlining (if uninlined,
the compilation would break), you must not use the __always_inline
keyword and you must let the compiler decide what to do.
Said another way: "The code isn't optimal with my compiler on my computer
unless I force inline this" is not a valid reason to use __always_inline
And for this reason we never use __inline in foo.c files, always let the
compiler decide.
This applies to your entire submission.
> + ((u64 *)dst)[0] = be64_to_cpu(((const __be64 *)src)[0]);
> + ((u64 *)dst)[1] = be64_to_cpu(((const __be64 *)src)[1]);
Are 'dst' and 'src' both 64-bit aligned? If not you'll get traps on some cpus.
> + __skb_queue_head_init(&packets);
> + if (!skb_is_gso(skb)) {
> + skb->next = NULL;
Why? Direct ->next and ->prev pointer accesses should never be used,
along with anything that assumes what the implentation of skb lists
looks like.
Always use the helpers instead.
> diff --git a/drivers/net/wireguard/hashtables.c b/drivers/net/wireguard/hashtables.c
> new file mode 100644
> index 000000000000..8aedc17b85f9
> --- /dev/null
> +++ b/drivers/net/wireguard/hashtables.c
No way.
Do not invent your own hashtables, we have several generic versions in
tree and in particular rhashtable.
If the generic kernel facilities have a weakness, fix that instead of
rolling an entire new hashtable implementation.
next prev parent reply other threads:[~2019-03-25 0:02 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-22 7:11 [PATCH net-next v9 00/19] WireGuard: Secure Network Tunnel Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 01/19] asm: simd context helper API Jason A. Donenfeld
2019-03-22 11:21 ` Thomas Gleixner
2019-03-22 7:11 ` [PATCH net-next v9 02/19] zinc: introduce minimal cryptography library Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 03/19] zinc: ChaCha20 generic C implementation and selftest Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 04/19] zinc: ChaCha20 x86_64 implementation Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 05/19] zinc: ChaCha20 ARM and ARM64 implementations Jason A. Donenfeld
2019-03-22 23:17 ` Stefan Agner
2019-03-22 7:11 ` [PATCH net-next v9 06/19] zinc: ChaCha20 MIPS32r2 implementation Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 07/19] zinc: Poly1305 generic C implementations and selftest Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 08/19] zinc: Poly1305 x86_64 implementation Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 09/19] zinc: Poly1305 ARM and ARM64 implementations Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 10/19] zinc: Poly1305 MIPS64 and MIPS32r2 implementations Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 11/19] zinc: ChaCha20Poly1305 construction and selftest Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 12/19] zinc: BLAKE2s generic C implementation " Jason A. Donenfeld
2019-03-26 17:38 ` Eric Biggers
2019-03-22 7:11 ` [PATCH net-next v9 13/19] zinc: BLAKE2s x86_64 implementation Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 14/19] zinc: Curve25519 generic C implementations and selftest Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 15/19] zinc: Curve25519 x86_64 implementation Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 16/19] zinc: import Bernstein and Schwabe's Curve25519 ARM implementation Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 17/19] zinc: " Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 18/19] security/keys: rewrite big_key crypto to use Zinc Jason A. Donenfeld
2019-03-22 7:11 ` [PATCH net-next v9 19/19] net: WireGuard secure network tunnel Jason A. Donenfeld
2019-03-25 0:02 ` David Miller [this message]
2019-03-25 10:13 ` Jason A. Donenfeld
2019-03-25 11:51 ` [PATCH net-next v9 00/19] WireGuard: Secure Network Tunnel Herbert Xu
2019-03-25 11:57 ` Jason A. Donenfeld
2019-03-25 12:03 ` Herbert Xu
2019-03-30 5:53 ` Eric Biggers
2019-03-31 18:18 ` Jason A. Donenfeld
2019-03-31 18:42 ` Eric Biggers
2019-03-25 16:04 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190324.200254.1946143057733371048.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=Jason@zx2c4.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).